Skip to content

Commit

Permalink
review - round 3
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfc-gh-pcyrek
sfc-gh-pcyrek committed Dec 19, 2024
1 parent a52cb60 commit c3ef9e5
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 31 deletions.
38 changes: 18 additions & 20 deletions auth_with_external_browser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
}
Expand All @@ -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()
}
Expand All @@ -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))
Expand All @@ -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()
Expand All @@ -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()

Check failure on line 112 in auth_with_external_browser_test.go

View workflow job for this annotation

GitHub Actions / Check linter 

SA5001: should check error returned from db.Conn() before deferring conn.Close() (staticcheck)
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))
})
}

Expand All @@ -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")

Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions auth_with_keypair_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
10 changes: 5 additions & 5 deletions auth_with_oauth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ 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))
}

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))
Expand All @@ -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))
Expand Down Expand Up @@ -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)
}
Expand All @@ -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"`
Expand Down
8 changes: 4 additions & 4 deletions auth_with_okta_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))

Expand Down

0 comments on commit c3ef9e5

Please sign in to comment.