SNOW-715524: Add SSO token cache #921

sfc-gh-ext-simba-lf · 2024-04-18T17:01:00Z

Description

Add SSO token cache

Checklist

Code compiles correctly
Code is formatted according to Coding Conventions
Created tests which fail without the change (if possible)
All tests passing (dotnet test)
Extended the README / documentation, if necessary
Provide JIRA issue id (if possible) or GitHub issue id in PR name

codecov · 2024-04-18T18:31:34Z

Codecov Report

Attention: Patch coverage is 87.11656% with 21 lines in your changes missing coverage. Please review.

Project coverage is 87.49%. Comparing base (444a2c1) to head (b4fdf7e).

Files with missing lines	Patch %	Lines
Snowflake.Data/Core/Tools/BrowserOperations.cs	5.55%	17 Missing ⚠️
...Core/Authenticator/ExternalBrowserAuthenticator.cs	97.08%	3 Missing ⚠️
Snowflake.Data/Core/Session/SFSession.cs	96.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #921      +/-   ##
==========================================
+ Coverage   86.23%   87.49%   +1.25%     
==========================================
  Files         132      133       +1     
  Lines       12664    12721      +57     
  Branches     1299     1305       +6     
==========================================
+ Hits        10921    11130     +209     
+ Misses       1417     1267     -150     
+ Partials      326      324       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…nector-net into SNOW-715524-SSO-Token-Cache

Snowflake.Data/Snowflake.Data.csproj

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerFactory.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerAdysTechImpl.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerFileImpl.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerInMemoryImpl.cs

Snowflake.Data/Core/Session/SFSession.cs

rafael-sotelo · 2024-05-16T15:14:49Z

Any updates on this PR ? Can I help ? I been waiting for this SSO Token Cache for long time

sfc-gh-ext-simba-lf · 2024-05-16T17:02:32Z

Any updates on this PR ? Can I help ? I been waiting for this SSO Token Cache for long time

Currently we're trying out different libraries/packages for the credential manager. Then it'll be reviewed if it meets the security requirements

Snowflake.Data/Client/SnowflakeCredentialManager/ISnowflakeCredentialManager.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerFactory.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerInMemoryImpl.cs

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerFactory.cs

sfc-gh-knozderko · 2024-06-05T15:16:07Z

Snowflake.Data/Client/SnowflakeCredentialManager/SnowflakeCredentialManagerNativeImpl.cs

+
+namespace Snowflake.Data.Client
+{
+    public class SnowflakeCredentialManagerNativeImpl : ISnowflakeCredentialManager


You could move all the implementations into subpackage Infrastructure

Snowflake.Data/Core/Authenticator/ExternalBrowserAuthenticator.cs

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs

…nector-net into SNOW-715524-SSO-Token-Cache

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data/Core/SFError.cs

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs # Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs # Snowflake.Data/Core/Authenticator/ExternalBrowserAuthenticator.cs # Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerFileImpl.cs # Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerInMemoryImpl.cs # Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerWindowsNativeImpl.cs # Snowflake.Data/Core/RestResponse.cs # Snowflake.Data/Core/SFError.cs # Snowflake.Data/Core/Session/SFSession.cs # Snowflake.Data/Core/Session/SFSessionParameter.cs # Snowflake.Data/Core/Session/SFSessionProperty.cs # Snowflake.Data/Core/Tools/FileOperations.cs # Snowflake.Data/Core/Tools/UnixOperations.cs # doc/Connecting.md

…nector-net into SNOW-715524-SSO-Token-Cache

…flakedb/snowflake-connector-net into SNOW-715524-SSO-Token-Cache

sfc-gh-knozderko · 2025-02-05T16:38:44Z

Snowflake.Data/Core/Authenticator/ExternalBrowserAuthenticator.cs

-
-            int localPort = GetRandomUnusedPort();
-            using (var httpListener = GetHttpListener(localPort))
+            var idToken = new NetworkCredential(string.Empty, session._idToken).Password;


you could use SecureStringHelper.Decode()

sfc-gh-knozderko · 2025-02-05T16:44:29Z

Snowflake.Data/Core/Authenticator/ExternalBrowserAuthenticator.cs

-            data.Token = _samlResponseToken;
-            data.ProofKey = _proofKey;
-            SetSecondaryAuthenticationData(ref data);
+            var idToken = new NetworkCredential(string.Empty, session._idToken).Password;


you could use SecureStringHelper.Decode()

sfc-gh-knozderko · 2025-02-05T16:46:19Z

Snowflake.Data/Core/Authenticator/ExternalBrowserAuthenticator.cs

+            else
+            {
+                data.Token = idToken;
+                data.Authenticator = TokenType.IdToken.GetAttribute<StringAttr>().value;


it will be id_token not externalbrowser?

Yes, it's for the server response to contain an ID token

It's similar to how other connectors handle it: https://github.com/snowflakedb/snowflake-connector-nodejs/blob/9cae04323ef4f76d91342e80cfef39952fe652c3/lib/authentication/auth_idtoken.js#L28

sfc-gh-knozderko · 2025-02-05T16:47:03Z

Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerInMemoryImpl.cs

-                }
-                s_logger.Info("Unable to get credentials for the specified key");
-                return "";
+            s_logger.Debug($"Getting credentials from memory for key: {key}");


this extra tab is not needed

Sorry, I'm not able to see an extra tab. I checked with the formatter and it doesn't find anything either

sfc-gh-knozderko · 2025-02-05T16:49:47Z

Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerWindowsNativeImpl.cs

@@ -12,7 +12,6 @@

 namespace Snowflake.Data.Core.CredentialManager.Infrastructure
 {
-


I would revert this change

sfc-gh-ext-simba-lf and others added 5 commits April 18, 2024 09:54

SNOW-715524: Add SSO token cache

7de3ea6

Add tests

4457077

Remove unused namespace

15a58be

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

632a6b0

Fix unit test

7f28fa8

sfc-gh-ext-simba-lf added 10 commits April 18, 2024 14:09

Refactor constructor and tests

a317158

Refactor test

625e04b

Refactor test and file impl

383fe5e

Refactor file name and fix test

8ce9d10

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

a460e4b

…nector-net into SNOW-715524-SSO-Token-Cache

Revert removed lines

e0b65d0

Refactor code and remove unnecessary check

6606823

Add session test

4e6869d

Fix test

3788474

Refactor test and rename file

c597c64

sfc-gh-ext-simba-lf marked this pull request as ready for review April 20, 2024 00:42

sfc-gh-ext-simba-lf requested a review from a team as a code owner April 20, 2024 00:42

sfc-gh-ext-simba-lf requested a review from sfc-gh-knozderko April 20, 2024 00:42

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

39b90b2

…nector-net into SNOW-715524-SSO-Token-Cache

sfc-gh-pbulawa requested changes Apr 30, 2024

View reviewed changes

sfc-gh-ext-simba-lf added 4 commits May 1, 2024 13:05

Add more logging

b4ab4ed

Change log from error to info

13ba839

Add file path to logs

b334fb3

Add Meziantou package for credential manager implementation

623ce6a

sfc-gh-ext-simba-lf added 2 commits May 17, 2024 09:50

Add native implementation of credential cache

976bac2

Add impl for ReleaseHandle

feab579

sfc-gh-knozderko requested changes Jun 5, 2024

View reviewed changes

sfc-gh-ext-simba-lf and others added 26 commits July 9, 2024 09:35

Close HttpListener before throwing exception

d53dd8f

Close HttpListener before throwing exception

98df45e

Modify browser timeout test

359cfa3

Abort HttpListener before throwing exception

85d04ed

Check if browser timeout already reached before getting context

2d39171

Use SecureString for tokens

f7be152

Add security checks when reading the credential cache json

2f20edd

Add error logs when reading the json cache file

ac1d659

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

8c65f4d

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

330fca9

…nector-net into SNOW-715524-SSO-Token-Cache

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

fc928fc

…nector-net into SNOW-715524-SSO-Token-Cache

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

d53955a

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

d6ff05a

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

a1a6a31

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

74963c7

…nector-net into SNOW-715524-SSO-Token-Cache # Conflicts: # Snowflake.Data/Core/SFError.cs

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

126e337

Modify external browser test after rebase

17877b8

Fix session test

f6a536f

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

d0c33e7

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

06020dd

…nector-net into SNOW-715524-SSO-Token-Cache

Merge branch 'SNOW-715524-SSO-Token-Cache' of https://github.com/snow…

41cadc3

…flakedb/snowflake-connector-net into SNOW-715524-SSO-Token-Cache

Retrigger checks

03fd55c

Merge branch 'master' into SNOW-715524-SSO-Token-Cache

0fc3252

Replace and remove SFCredentialManagerFactory class

af2fb8c

Revert external browser tests

b4fdf7e

sfc-gh-pbulawa approved these changes Feb 3, 2025

View reviewed changes

sfc-gh-knozderko reviewed Feb 5, 2025

View reviewed changes

sfc-gh-ext-simba-lf added 2 commits February 5, 2025 12:20

Use SecureStringHelper.Decode

a941a25

Revert newline

8003c05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNOW-715524: Add SSO token cache #921

SNOW-715524: Add SSO token cache #921

sfc-gh-ext-simba-lf commented Apr 18, 2024

codecov bot commented Apr 18, 2024 •

edited

Loading

rafael-sotelo commented May 16, 2024

sfc-gh-ext-simba-lf commented May 16, 2024

sfc-gh-knozderko Jun 5, 2024

sfc-gh-ext-simba-lf Jun 6, 2024

sfc-gh-knozderko Feb 5, 2025

sfc-gh-ext-simba-lf Feb 5, 2025

sfc-gh-knozderko Feb 5, 2025

sfc-gh-ext-simba-lf Feb 5, 2025

sfc-gh-knozderko Feb 5, 2025

sfc-gh-ext-simba-lf Feb 5, 2025

sfc-gh-knozderko Feb 5, 2025

sfc-gh-ext-simba-lf Feb 5, 2025

sfc-gh-knozderko Feb 5, 2025

sfc-gh-ext-simba-lf Feb 5, 2025

		@@ -12,7 +12,6 @@

		namespace Snowflake.Data.Core.CredentialManager.Infrastructure
		{

SNOW-715524: Add SSO token cache #921

Are you sure you want to change the base?

SNOW-715524: Add SSO token cache #921

Conversation

sfc-gh-ext-simba-lf commented Apr 18, 2024

Description

Checklist

codecov bot commented Apr 18, 2024 • edited Loading

Codecov Report

rafael-sotelo commented May 16, 2024

sfc-gh-ext-simba-lf commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 18, 2024 •

edited

Loading