From c3ef9e5f8cd690f761f52ce1a045f80120361c12 Mon Sep 17 00:00:00 2001 From: Patryk Cyrek Date: Thu, 19 Dec 2024 09:10:55 +0100 Subject: [PATCH] review - round 3 --- auth_with_external_browser_test.go | 38 ++++++++++++++---------------- auth_with_keypair_test.go | 4 ++-- auth_with_oauth_test.go | 10 ++++---- auth_with_okta_test.go | 8 +++---- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/auth_with_external_browser_test.go b/auth_with_external_browser_test.go index 840922c3f..b9446fdc0 100644 --- a/auth_with_external_browser_test.go +++ b/auth_with_external_browser_test.go @@ -18,11 +18,11 @@ func TestExternalBrowserSuccessful(t *testing.T) { wg.Add(2) go func() { defer wg.Done() - provideCredentials(t, externalBrowserType.Success, cfg.User, cfg.Password) + provideExternalBrowserCredentials(t, externalBrowserType.Success, cfg.User, cfg.Password) }() go func() { defer wg.Done() - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) assertNilE(t, err, fmt.Sprintf("Connection failed due to %v", err)) }() wg.Wait() @@ -35,13 +35,12 @@ func TestExternalBrowserFailed(t *testing.T) { wg.Add(2) go func() { defer wg.Done() - provideCredentials(t, externalBrowserType.Fail, "FakeAccount", "NotARealPassword") + provideExternalBrowserCredentials(t, externalBrowserType.Fail, "FakeAccount", "NotARealPassword") }() go func() { defer wg.Done() - tOut := "authentication timed out" - err := connectToSnowflake(t, cfg) - assertEqualE(t, err.Error(), tOut, fmt.Sprintf("Expected %v, but got %v", tOut, err)) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) + assertEqualE(t, err.Error(), "authentication timed out") }() wg.Wait() } @@ -53,13 +52,12 @@ func TestExternalBrowserTimeout(t *testing.T) { wg.Add(2) go func() { defer wg.Done() - provideCredentials(t, externalBrowserType.Timeout, cfg.User, cfg.Password) + provideExternalBrowserCredentials(t, externalBrowserType.Timeout, cfg.User, cfg.Password) }() go func() { defer wg.Done() - tOut := "authentication timed out" - err := connectToSnowflake(t, cfg) - assertEqualE(t, err.Error(), tOut, fmt.Sprintf("Expected %v, but got %v", tOut, err)) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) + assertEqualE(t, err.Error(), "authentication timed out") }() wg.Wait() } @@ -73,11 +71,11 @@ func TestExternalBrowserMismatchUser(t *testing.T) { wg.Add(2) go func() { defer wg.Done() - provideCredentials(t, externalBrowserType.Success, correctUsername, cfg.Password) + provideExternalBrowserCredentials(t, externalBrowserType.Success, correctUsername, cfg.Password) }() go func() { defer wg.Done() - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) assertEqualE(t, snowflakeErr.Number, 390191, fmt.Sprintf("Expected 390191, but got %v", snowflakeErr.Number)) @@ -96,11 +94,11 @@ func TestClientStoreCredentials(t *testing.T) { wg.Add(2) go func() { defer wg.Done() - provideCredentials(t, externalBrowserType.Success, cfg.User, cfg.Password) + provideExternalBrowserCredentials(t, externalBrowserType.Success, cfg.User, cfg.Password) }() go func() { defer wg.Done() - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) assertNilE(t, err, fmt.Sprintf("Connection failed: err %v", err)) }() wg.Wait() @@ -111,18 +109,19 @@ func TestClientStoreCredentials(t *testing.T) { cfg.ClientStoreTemporaryCredential = 1 db := getDbHandlerFromConfig(t, cfg) conn, err := db.Conn(context.Background()) + defer conn.Close() assertNilE(t, err, fmt.Sprintf("Failed to connect to Snowflake. err: %v", err)) - _, err = conn.QueryContext(context.Background(), "SELECT 1") + rows, err := conn.QueryContext(context.Background(), "SELECT 1") + rows.Close() assertNilE(t, err, fmt.Sprintf("Failed to run a query. err: %v", err)) }) t.Run("Verify validation of IDToken if option disabled", func(t *testing.T) { cleanupBrowserProcesses(t) cfg.ClientStoreTemporaryCredential = 0 - tOut := "authentication timed out" db := getDbHandlerFromConfig(t, cfg) _, err := db.Conn(context.Background()) - assertEqualE(t, err.Error(), tOut, fmt.Sprintf("Expected %v, but got %v", tOut, err)) + assertEqualE(t, err.Error(), "authentication timed out", fmt.Sprintf("Expected timeout, but got %v", err)) }) } @@ -144,13 +143,13 @@ func cleanupBrowserProcesses(t *testing.T) { assertNilE(t, err, fmt.Sprintf("failed to execute command: %v", err)) } -func provideCredentials(t *testing.T, ExternalBrowserProcess string, user string, password string) { +func provideExternalBrowserCredentials(t *testing.T, ExternalBrowserProcess string, user string, password string) { const provideBrowserCredentialsPath = "/externalbrowser/provideBrowserCredentials.js" _, err := exec.Command("node", provideBrowserCredentialsPath, ExternalBrowserProcess, user, password).Output() assertNilE(t, err, fmt.Sprintf("failed to execute command: %v", err)) } -func connectToSnowflake(t *testing.T, cfg *Config) (err error) { +func verifyConnectionToSnowflakeAuthTests(t *testing.T, cfg *Config) (err error) { dsn, err := DSN(cfg) assertNilE(t, err, "failed to create DSN from Config") @@ -165,7 +164,6 @@ func connectToSnowflake(t *testing.T, cfg *Config) (err error) { } defer rows.Close() - assertTrueE(t, rows.Next(), "failed to get result", "There were no results for query: ") return err diff --git a/auth_with_keypair_test.go b/auth_with_keypair_test.go index 7b2b3fb74..3e9897b5e 100644 --- a/auth_with_keypair_test.go +++ b/auth_with_keypair_test.go @@ -13,14 +13,14 @@ func TestKeypairSuccessful(t *testing.T) { cfg := setupKeyPairTest(t) cfg.PrivateKey = loadRsaPrivateKeyForKeyPair(t, "SNOWFLAKE_AUTH_TEST_PRIVATE_KEY_PATH") - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) assertNilE(t, err, fmt.Sprintf("failed to connect. err: %v", err)) } func TestKeypairInvalidKey(t *testing.T) { cfg := setupKeyPairTest(t) cfg.PrivateKey = loadRsaPrivateKeyForKeyPair(t, "SNOWFLAKE_AUTH_TEST_INVALID_PRIVATE_KEY_PATH") - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) assertEqualE(t, snowflakeErr.Number, 390144, fmt.Sprintf("Expected 390144, but got %v", snowflakeErr.Number)) diff --git a/auth_with_oauth_test.go b/auth_with_oauth_test.go index 4c63b93bf..2c5f4056a 100644 --- a/auth_with_oauth_test.go +++ b/auth_with_oauth_test.go @@ -15,7 +15,7 @@ func TestOauthSuccessful(t *testing.T) { token, err := getOauthTestToken(t, cfg) assertNilE(t, err, fmt.Sprintf("failed to get token. err: %v", err)) cfg.Token = token - err = connectToSnowflake(t, cfg) + err = verifyConnectionToSnowflakeAuthTests(t, cfg) assertNilE(t, err, fmt.Sprintf("failed to connect. err: %v", err)) } @@ -23,7 +23,7 @@ func TestOauthInvalidToken(t *testing.T) { cfg := setupOauthTest(t) cfg.Token = "invalid_token" - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) @@ -37,7 +37,7 @@ func TestOauthMismatchedUser(t *testing.T) { cfg.Token = token cfg.User = "fakeaccount" - err = connectToSnowflake(t, cfg) + err = verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) @@ -82,7 +82,7 @@ func getOauthTestToken(t *testing.T, cfg *Config) (string, error) { defer resp.Body.Close() - var response Response + var response OAuthTokenResponse if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { return "", fmt.Errorf("failed to decode response: %v", err) } @@ -101,7 +101,7 @@ func formData(cfg *Config) url.Values { } -type Response struct { +type OAuthTokenResponse struct { Type string `json:"token_type"` Expiration int `json:"expires_in"` Token string `json:"access_token"` diff --git a/auth_with_okta_test.go b/auth_with_okta_test.go index b2663b3a8..31ae43b71 100644 --- a/auth_with_okta_test.go +++ b/auth_with_okta_test.go @@ -9,14 +9,14 @@ import ( func TestOktaSuccessful(t *testing.T) { cfg := setupOktaTest(t) - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) assertNilE(t, err, fmt.Sprintf("failed to connect. err: %v", err)) } func TestOktaWrongCredentials(t *testing.T) { cfg := setupOktaTest(t) cfg.Password = "fakePassword" - err := connectToSnowflake(t, cfg) + err := verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) @@ -29,7 +29,7 @@ func TestOktaWrongAuthenticator(t *testing.T) { assertNilF(t, err, fmt.Sprintf("failed to parse: %v", err)) cfg.OktaURL = invalidAddress - err = connectToSnowflake(t, cfg) + err = verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr)) @@ -45,7 +45,7 @@ func TestOktaWrongURL(t *testing.T) { cfg.OktaURL = invalidAddress - err = connectToSnowflake(t, cfg) + err = verifyConnectionToSnowflakeAuthTests(t, cfg) var snowflakeErr *SnowflakeError assertTrueF(t, errors.As(err, &snowflakeErr))