-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SNOW-715524: Add SSO token cache #921
base: master
Are you sure you want to change the base?
Changes from all commits
7de3ea6
4457077
15a58be
632a6b0
7f28fa8
a317158
625e04b
383fe5e
8ce9d10
a460e4b
e0b65d0
6606823
4e6869d
3788474
c597c64
39b90b2
b4ab4ed
13ba839
b334fb3
623ce6a
976bac2
feab579
61b2317
cb1c84f
7f0f801
780d213
575c0a4
aa00982
ecdcf37
6f31fe6
cdc9f80
9622f5c
a2f9b50
739c40c
c502e80
465da80
94bce01
83119f3
4e57147
61855f2
8b38fed
194eafa
8666194
403dbd2
33ebec5
512de5b
6ef9b35
d616dcc
045fc04
5c9d8d7
adb3218
da0cffb
44c746b
590e98b
2abcad4
dd24c76
f8b3230
7a384e8
7672901
2cdbae4
524b78f
6fdf045
2d2870d
f8f7336
8af5ba8
e56b086
2ba9aaf
7e9c098
e49ed64
c434184
67a8fbf
f8fcf2b
a46f1ad
52bfd3e
9edfd79
f6d8a0f
77a69b3
9038ba1
d800e6f
58b74d8
8f919e7
fbbac23
d53dd8f
98df45e
359cfa3
85d04ed
2d39171
f7be152
2f20edd
ac1d659
8c65f4d
330fca9
fc928fc
d53955a
d6ff05a
a1a6a31
74963c7
126e337
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
/* | ||
/* | ||
* Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. | ||
*/ | ||
|
||
|
@@ -21,6 +21,8 @@ namespace Snowflake.Data.Tests.IntegrationTests | |
using Snowflake.Data.Tests.Mock; | ||
using System.Runtime.InteropServices; | ||
using System.Net.Http; | ||
using Snowflake.Data.Core.CredentialManager; | ||
using Snowflake.Data.Core.CredentialManager.Infrastructure; | ||
|
||
[TestFixture] | ||
class SFConnectionIT : SFBaseTest | ||
|
@@ -1046,6 +1048,71 @@ public void TestSSOConnectionTimeoutAfter10s() | |
Assert.LessOrEqual(stopwatch.ElapsedMilliseconds, (waitSeconds + 5) * 1000); | ||
} | ||
|
||
[Test] | ||
[Ignore("This test requires manual interaction and therefore cannot be run in CI")] | ||
public void TestSSOConnectionWithTokenCaching() | ||
{ | ||
/* | ||
* This test checks that the connector successfully stores an SSO token and uses it for authentication if it exists | ||
* 1. Login normally using external browser with allow_sso_token_caching enabled | ||
* 2. Login again, this time without a browser, as the connector should be using the SSO token retrieved from step 1 | ||
*/ | ||
|
||
using (IDbConnection conn = new SnowflakeDbConnection()) | ||
{ | ||
// Set the allow_sso_token_caching property to true to enable token caching | ||
// The specified user should be configured for SSO | ||
conn.ConnectionString | ||
= ConnectionStringWithoutAuth | ||
+ $";authenticator=externalbrowser;user={testConfig.user};allow_sso_token_caching=true;"; | ||
|
||
// Authenticate to retrieve and store the token if doesn't exist or invalid | ||
conn.Open(); | ||
Assert.AreEqual(ConnectionState.Open, conn.State); | ||
|
||
// Authenticate using the SSO token (the connector will automatically use the token and a browser should not pop-up in this step) | ||
conn.Open(); | ||
Assert.AreEqual(ConnectionState.Open, conn.State); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the connection pool is enabled by default so when you open the connection for the second time you get exactly the same session - so you are not testing getting the token from the credential manager. You should open the second connection before closing the first one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
conn.Close(); | ||
Assert.AreEqual(ConnectionState.Closed, conn.State); | ||
} | ||
} | ||
|
||
[Test] | ||
[Ignore("This test requires manual interaction and therefore cannot be run in CI")] | ||
public void TestSSOConnectionWithInvalidCachedToken() | ||
{ | ||
/* | ||
* This test checks that the connector will attempt to re-authenticate using external browser if the token retrieved from the cache is invalid | ||
* 1. Create a credential manager and save credentials for the user with a wrong token | ||
* 2. Open a connection which initially should try to use the token and then switch to external browser when the token fails | ||
*/ | ||
|
||
using (IDbConnection conn = new SnowflakeDbConnection()) | ||
{ | ||
// Set the allow_sso_token_caching property to true to enable token caching | ||
conn.ConnectionString | ||
= ConnectionStringWithoutAuth | ||
+ $";authenticator=externalbrowser;user={testConfig.user};allow_sso_token_caching=true;"; | ||
|
||
// Create a credential manager and save a wrong token for the test user | ||
var key = SFCredentialManagerFactory.BuildCredentialKey(testConfig.host, testConfig.user, TokenType.IdToken); | ||
var credentialManager = SFCredentialManagerInMemoryImpl.Instance; | ||
credentialManager.SaveCredentials(key, "wrongToken"); | ||
|
||
// Use the credential manager with the wrong token | ||
SFCredentialManagerFactory.SetCredentialManager(credentialManager); | ||
|
||
// Open a connection which should switch to external browser after trying to connect using the wrong token | ||
conn.Open(); | ||
Assert.AreEqual(ConnectionState.Open, conn.State); | ||
|
||
// Switch back to the default credential manager | ||
SFCredentialManagerFactory.UseDefaultCredentialManager(); | ||
} | ||
} | ||
|
||
[Test] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to write a test which would not be ignored? Maybe with something mocked or something that simulates the browser action? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added tests for external browser authentication using a mock browser |
||
[Ignore("This test requires manual interaction and therefore cannot be run in CI")] | ||
public void TestSSOConnectionWithWrongUser() | ||
|
@@ -2311,6 +2378,40 @@ public void TestOpenAsyncThrowExceptionWhenOperationIsCancelled() | |
} | ||
} | ||
|
||
[Test] | ||
[Ignore("This test requires manual interaction and therefore cannot be run in CI")] | ||
public void TestSSOConnectionWithTokenCachingAsync() | ||
{ | ||
/* | ||
* This test checks that the connector successfully stores an SSO token and uses it for authentication if it exists | ||
* 1. Login normally using external browser with allow_sso_token_caching enabled | ||
* 2. Login again, this time without a browser, as the connector should be using the SSO token retrieved from step 1 | ||
*/ | ||
|
||
using (SnowflakeDbConnection conn = new SnowflakeDbConnection()) | ||
{ | ||
// Set the allow_sso_token_caching property to true to enable token caching | ||
// The specified user should be configured for SSO | ||
conn.ConnectionString | ||
= ConnectionStringWithoutAuth | ||
+ $";authenticator=externalbrowser;user={testConfig.user};allow_sso_token_caching=true;"; | ||
|
||
// Authenticate to retrieve and store the token if doesn't exist or invalid | ||
Task connectTask = conn.OpenAsync(CancellationToken.None); | ||
connectTask.Wait(); | ||
Assert.AreEqual(ConnectionState.Open, conn.State); | ||
|
||
// Authenticate using the SSO token (the connector will automatically use the token and a browser should not pop-up in this step) | ||
connectTask = conn.OpenAsync(CancellationToken.None); | ||
connectTask.Wait(); | ||
Assert.AreEqual(ConnectionState.Open, conn.State); | ||
|
||
connectTask = conn.CloseAsync(CancellationToken.None); | ||
connectTask.Wait(); | ||
Assert.AreEqual(ConnectionState.Closed, conn.State); | ||
} | ||
} | ||
|
||
[Test] | ||
public void TestCloseSessionWhenGarbageCollectorFinalizesConnection() | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
/* | ||
* Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. | ||
*/ | ||
|
||
using Snowflake.Data.Core; | ||
using System.Net.Http; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace Snowflake.Data.Tests.Mock | ||
{ | ||
|
||
class MockExternalBrowserRestRequester : IMockRestRequester | ||
{ | ||
public string ProofKey { get; set; } | ||
public string SSOUrl { get; set; } | ||
|
||
public T Get<T>(IRestRequest request) | ||
{ | ||
throw new System.NotImplementedException(); | ||
} | ||
|
||
public Task<T> GetAsync<T>(IRestRequest request, CancellationToken cancellationToken) | ||
{ | ||
throw new System.NotImplementedException(); | ||
} | ||
|
||
public T Post<T>(IRestRequest postRequest) | ||
{ | ||
return Task.Run(async () => await (PostAsync<T>(postRequest, CancellationToken.None)).ConfigureAwait(false)).Result; | ||
} | ||
|
||
public Task<T> PostAsync<T>(IRestRequest postRequest, CancellationToken cancellationToken) | ||
{ | ||
SFRestRequest sfRequest = (SFRestRequest)postRequest; | ||
if (sfRequest.jsonBody is AuthenticatorRequest) | ||
{ | ||
if (string.IsNullOrEmpty(SSOUrl)) | ||
{ | ||
var body = (AuthenticatorRequest)sfRequest.jsonBody; | ||
var port = body.Data.BrowserModeRedirectPort; | ||
SSOUrl = $"http://localhost:{port}/?token=mockToken"; | ||
} | ||
|
||
// authenticator | ||
var authnResponse = new AuthenticatorResponse | ||
{ | ||
success = true, | ||
data = new AuthenticatorResponseData | ||
{ | ||
proofKey = ProofKey, | ||
ssoUrl = SSOUrl, | ||
} | ||
}; | ||
|
||
return Task.FromResult<T>((T)(object)authnResponse); | ||
} | ||
else | ||
{ | ||
// login | ||
var loginResponse = new LoginResponse | ||
{ | ||
success = true, | ||
data = new LoginResponseData | ||
{ | ||
sessionId = "", | ||
token = "", | ||
masterToken = "", | ||
masterValidityInSeconds = 0, | ||
authResponseSessionInfo = new SessionInfo | ||
{ | ||
databaseName = "", | ||
schemaName = "", | ||
roleName = "", | ||
warehouseName = "", | ||
} | ||
} | ||
}; | ||
|
||
return Task.FromResult<T>((T)(object)loginResponse); | ||
} | ||
} | ||
|
||
public HttpResponseMessage Get(IRestRequest request) | ||
{ | ||
throw new System.NotImplementedException(); | ||
} | ||
|
||
public Task<HttpResponseMessage> GetAsync(IRestRequest request, CancellationToken cancellationToken) | ||
{ | ||
throw new System.NotImplementedException(); | ||
} | ||
|
||
public void setHttpClient(HttpClient httpClient) | ||
{ | ||
// Nothing to do | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with ignored tests is that they will not be executed so in the future something can break and we will not notice that.
Then if a developer wants to run them manually it takes him a lot of time to create a proper setup for the test.
Could you provide some context (for example in the comments or readme) how to run this test manually for someone who will want to run it for example in one year and would not know any context of this feature?
You provide here a specific user. Also maybe we would need information that we should change it when testing on particular environment. But we should not write about any connection details in this repo since it is publicly visible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, I'll put more details on the comments on how to do the test
As for the user, I'll replace it so the details are not exposed