diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e085df107..96614c8b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Snowflake SPCS (Snowpark Container Services) authentication now properly handles OIDC + by requiring both a Snowflake connection name and a Connect API key. The Connect API + key is sent via the X-RSC-Authorization header while the Snowflake token is sent via + the Authorization header for proxied authentication. - The "R Packages" section no longer shows you an alert if you aren't using renv. (#3095) - When `renv.lock` contains packages installed from GitHub or Bitbucket the deploy process should respect `RemoteHost`, `RemoteRepo`, `RemoteUsername` and diff --git a/extensions/vscode/CHANGELOG.md b/extensions/vscode/CHANGELOG.md index 2d6a390532..24af22df39 100644 --- a/extensions/vscode/CHANGELOG.md +++ b/extensions/vscode/CHANGELOG.md @@ -31,6 +31,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - When opening the full deployment log record, it opens as a new unsaved file. (#2996) +### Fixed + +- Snowflake SPCS (Snowpark Container Services) authentication now properly handles OIDC + by requiring both a Snowflake connection name and a Connect API key. This aligns with + changes in Snowflake SPCS where proxied authentication headers no longer carry sufficient + user identification information. + ## [1.22.0] ### Fixed diff --git a/extensions/vscode/src/api/resources/SnowflakeConnections.ts b/extensions/vscode/src/api/resources/SnowflakeConnections.ts index 2cc0178356..2c123c1a50 100644 --- a/extensions/vscode/src/api/resources/SnowflakeConnections.ts +++ b/extensions/vscode/src/api/resources/SnowflakeConnections.ts @@ -15,13 +15,17 @@ export class SnowflakeConnections { // Returned connections include the validated server URL they were // successfully tested against. // + // If apiKey is provided, connections will be tested with both the + // Snowflake token and the Connect API key for SPCS OIDC authentication. + // // Returns: // 200 - ok // 500 - internal server error - list(serverUrl: string) { + list(serverUrl: string, apiKey?: string) { return this.client.get(`snowflake-connections`, { params: { serverUrl, + apiKey, }, }); } diff --git a/extensions/vscode/src/multiStepInputs/common.ts b/extensions/vscode/src/multiStepInputs/common.ts index 3ff9c24140..0119f102c7 100644 --- a/extensions/vscode/src/multiStepInputs/common.ts +++ b/extensions/vscode/src/multiStepInputs/common.ts @@ -65,13 +65,19 @@ export const platformList: QuickPickItem[] = [ ]; // Fetch the list of all available snowflake connections -export const fetchSnowflakeConnections = async (serverUrl: string) => { +export const fetchSnowflakeConnections = async ( + serverUrl: string, + apiKey?: string, +) => { let connections: SnowflakeConnection[] = []; let connectionQuickPicks: QuickPickItemWithIndex[]; try { const api = await useApi(); - const connsResponse = await api.snowflakeConnections.list(serverUrl); + const connsResponse = await api.snowflakeConnections.list( + serverUrl, + apiKey, + ); connections = connsResponse.data; connectionQuickPicks = connections.map((connection, i) => ({ label: connection.name, diff --git a/extensions/vscode/src/multiStepInputs/newConnectCredential.ts b/extensions/vscode/src/multiStepInputs/newConnectCredential.ts index 2b3e98a251..ff6bb99ff9 100644 --- a/extensions/vscode/src/multiStepInputs/newConnectCredential.ts +++ b/extensions/vscode/src/multiStepInputs/newConnectCredential.ts @@ -81,6 +81,7 @@ export async function newConnectCredential( INPUT_SERVER_URL = "inputServerUrl", INPUT_API_KEY = "inputAPIKey", INPUT_SNOWFLAKE_CONN = "inputSnowflakeConnection", + INPUT_SNOWFLAKE_API_KEY = "inputSnowflakeAPIKey", INPUT_CRED_NAME = "inputCredentialName", INPUT_AUTH_METHOD = "inputAuthMethod", INPUT_TOKEN = "inputToken", @@ -93,6 +94,7 @@ export async function newConnectCredential( [step.INPUT_SERVER_URL]: inputServerUrl, [step.INPUT_API_KEY]: inputAPIKey, [step.INPUT_SNOWFLAKE_CONN]: inputSnowflakeConnection, + [step.INPUT_SNOWFLAKE_API_KEY]: inputSnowflakeAPIKey, [step.INPUT_CRED_NAME]: inputCredentialName, [step.INPUT_AUTH_METHOD]: inputAuthMethod, [step.INPUT_TOKEN]: inputToken, @@ -126,8 +128,12 @@ export async function newConnectCredential( }; const isValidSnowflakeAuth = () => { - // for Snowflake, require snowflakeConnection - return isSnowflake(serverType) && isString(state.data.snowflakeConnection); + // for Snowflake SPCS with OIDC, require both snowflakeConnection and apiKey + return ( + isSnowflake(serverType) && + isString(state.data.snowflakeConnection) && + isString(state.data.apiKey) + ); }; // *************************************************************** @@ -254,8 +260,21 @@ export async function newConnectCredential( severity: InputBoxValidationSeverity.Error, }); } + // Check and set serverType first, even if there's an error + // This is important for Snowflake URLs where auth will fail + // without credentials, but we still need to detect the server type + if (testResult.data.serverType) { + serverType = testResult.data.serverType; + } + const err = testResult.data.error; if (err) { + // For Snowflake, we expect auth to fail without credentials + // So we allow the flow to continue to prompt for credentials + if (isSnowflake(serverType)) { + return Promise.resolve(undefined); + } + if (err.code === "errorCertificateVerification") { return Promise.resolve({ message: `Error: URL Not Accessible - ${err.msg}. If applicable, consider disabling [Verify TLS Certificates](${openConfigurationCommand}).`, @@ -267,11 +286,6 @@ export async function newConnectCredential( severity: InputBoxValidationSeverity.Error, }); } - - if (testResult.data.serverType) { - // serverType will be overwritten if it is snowflake - serverType = testResult.data.serverType; - } } catch (e) { return Promise.resolve({ message: `Error: Invalid URL (unable to validate connectivity with Server URL - ${getMessageFromError(e)}).`, @@ -287,10 +301,12 @@ export async function newConnectCredential( state.data.url = formatURL(resp.trim()); if (isSnowflake(serverType)) { + // For Snowflake SPCS with OIDC, ask for API key first + // Then we can test connections with both credentials return { - name: step.INPUT_SNOWFLAKE_CONN, + name: step.INPUT_SNOWFLAKE_API_KEY, step: (input: MultiStepInput) => - steps[step.INPUT_SNOWFLAKE_CONN](input, state), + steps[step.INPUT_SNOWFLAKE_API_KEY](input, state), }; } @@ -490,15 +506,17 @@ export async function newConnectCredential( input: MultiStepInput, state: MultiStepState, ) { - // url should always be defined by the time we get to this step - // but we have to type guard it for the API + // url and apiKey should always be defined by the time we get to this step + // but we have to type guard them for the API const serverUrl = typeof state.data.url === "string" ? state.data.url : ""; + const apiKey = + typeof state.data.apiKey === "string" ? state.data.apiKey : ""; let connections: SnowflakeConnection[] = []; let connectionQuickPicks: QuickPickItemWithIndex[] = []; try { - await showProgress("Reading Snowflake connections", viewId, async () => { - const resp = await fetchSnowflakeConnections(serverUrl); + await showProgress("Testing Snowflake connections", viewId, async () => { + const resp = await fetchSnowflakeConnections(serverUrl, apiKey); connections = resp.connections; connectionQuickPicks = resp.connectionQuickPicks; }); @@ -532,6 +550,59 @@ export async function newConnectCredential( }; } + // *************************************************************** + // Step: Enter the API Key for Snowflake SPCS (Snowflake only) + // *************************************************************** + async function inputSnowflakeAPIKey( + input: MultiStepInput, + state: MultiStepState, + ) { + const currentAPIKey = + typeof state.data.apiKey === "string" ? state.data.apiKey : ""; + + const resp = await input.showInputBox({ + title: state.title, + step: 0, + totalSteps: 0, + password: true, + value: currentAPIKey, + prompt: `The Posit Connect API key for Snowflake SPCS OIDC authentication. + This is required in addition to the Snowflake connection for authentication with Connect deployed in Snowflake SPCS.`, + validate: (input: string) => { + if (input.includes(" ")) { + return Promise.resolve({ + message: "Error: Invalid API Key (spaces are not allowed).", + severity: InputBoxValidationSeverity.Error, + }); + } + return Promise.resolve(undefined); + }, + finalValidation: (input: string) => { + // validate that the API key is formed correctly + const errorMsg = checkSyntaxApiKey(input); + if (errorMsg) { + return Promise.resolve({ + message: `Error: Invalid API Key (${errorMsg}).`, + severity: InputBoxValidationSeverity.Error, + }); + } + return Promise.resolve(undefined); + }, + shouldResume: () => Promise.resolve(false), + ignoreFocusOut: true, + }); + + state.data.apiKey = resp.trim(); + + // Now that we have the API key, fetch Snowflake connections + // and test them with both the Snowflake token and API key + return { + name: step.INPUT_SNOWFLAKE_CONN, + step: (input: MultiStepInput) => + steps[step.INPUT_SNOWFLAKE_CONN](input, state), + }; + } + // *************************************************************** // Step: Name the credential (used for all platforms) // *************************************************************** diff --git a/internal/accounts/account.go b/internal/accounts/account.go index 27f5d09c6f..744dbcd862 100644 --- a/internal/accounts/account.go +++ b/internal/accounts/account.go @@ -29,11 +29,12 @@ type Account struct { // AuthType returns the detected AccountAuthType based on the properties of the // Account. func (acct *Account) AuthType() AccountAuthType { - // An account should have one of: API key, Snowflake connection name, or token+private key - if acct.ApiKey != "" { - return AuthTypeAPIKey - } else if acct.SnowflakeConnection != "" { + // Snowflake SPCS with OIDC requires both SnowflakeConnection AND ApiKey + // Check for Snowflake first since it's the most specific case + if acct.SnowflakeConnection != "" { return AuthTypeSnowflake + } else if acct.ApiKey != "" { + return AuthTypeAPIKey } else if acct.Token != "" && acct.PrivateKey != "" { return AuthTypeToken } diff --git a/internal/api_client/auth/auth.go b/internal/api_client/auth/auth.go index 18e8432380..89602da622 100644 --- a/internal/api_client/auth/auth.go +++ b/internal/api_client/auth/auth.go @@ -28,9 +28,11 @@ func (af AuthFactory) NewClientAuth(acct *accounts.Account) (AuthMethod, error) case accounts.AuthTypeAPIKey: return NewApiKeyAuthenticator(acct.ApiKey, ""), nil case accounts.AuthTypeSnowflake: + // Snowflake SPCS with OIDC requires both Snowflake token and Connect API key auth, err := NewSnowflakeAuthenticator( af.connections, acct.SnowflakeConnection, + acct.ApiKey, ) if err != nil { return nil, err diff --git a/internal/api_client/auth/snowflake.go b/internal/api_client/auth/snowflake.go index 2245ec8f31..eb9b38f65b 100644 --- a/internal/api_client/auth/snowflake.go +++ b/internal/api_client/auth/snowflake.go @@ -14,6 +14,7 @@ const headerName = "Authorization" type snowflakeAuthenticator struct { tokenProvider snowflake.TokenProvider + apiKey string } var _ AuthMethod = &snowflakeAuthenticator{} @@ -22,6 +23,10 @@ var _ AuthMethod = &snowflakeAuthenticator{} // from the system Snowflake configuration and returns an authenticator that // will add auth headers to requests. // +// For Snowflake SPCS with OIDC, this sets both: +// - Authorization header with the Snowflake token (for proxied auth) +// - X-RSC-Authorization header with the Connect API key (for Connect authentication) +// // Only supports keypair authentication. // // Errs if the named connection cannot be found, or if the connection does not @@ -29,6 +34,7 @@ var _ AuthMethod = &snowflakeAuthenticator{} func NewSnowflakeAuthenticator( connections snowflake.Connections, connectionName string, + apiKey string, ) (AuthMethod, error) { conn, err := connections.Get(connectionName) if err != nil { @@ -59,6 +65,7 @@ func NewSnowflakeAuthenticator( return &snowflakeAuthenticator{ tokenProvider: tokenProvider, + apiKey: apiKey, }, nil } @@ -68,7 +75,14 @@ func (a *snowflakeAuthenticator) AddAuthHeaders(req *http.Request) error { if err != nil { return err } + // Set Authorization header with Snowflake token for proxied authentication header := fmt.Sprintf(`Snowflake Token="%s"`, token) req.Header.Set(headerName, header) + + // Set X-RSC-Authorization header with Connect API key for OIDC authentication + if a.apiKey != "" { + apiKeyHeader := fmt.Sprintf("Key %s", a.apiKey) + req.Header.Set("X-RSC-Authorization", apiKeyHeader) + } return nil } diff --git a/internal/api_client/auth/snowflake_test.go b/internal/api_client/auth/snowflake_test.go index 60ad2445b5..c16537b664 100644 --- a/internal/api_client/auth/snowflake_test.go +++ b/internal/api_client/auth/snowflake_test.go @@ -33,7 +33,7 @@ func (s *SnowflakeAuthSuite) TestNewSnowflakeAuthenticator() { connections := &snowflake.MockConnections{} connections.On("Get", ":name:").Return(&snowflake.Connection{}, errors.New("connection error")).Once() - _, err := NewSnowflakeAuthenticator(connections, ":name:") + _, err := NewSnowflakeAuthenticator(connections, ":name:", "test-api-key") s.ErrorContains(err, "connection error") // unsupported authenticator type @@ -41,7 +41,7 @@ func (s *SnowflakeAuthSuite) TestNewSnowflakeAuthenticator() { Authenticator: "fake", }, nil).Once() - _, err = NewSnowflakeAuthenticator(connections, ":name:") + _, err = NewSnowflakeAuthenticator(connections, ":name:", "test-api-key") s.EqualError(err, "unsupported authenticator type: fake") // errors from implementation are bubbled up @@ -50,7 +50,7 @@ func (s *SnowflakeAuthSuite) TestNewSnowflakeAuthenticator() { Authenticator: "snowflake_jwt", }, nil).Once() - _, err = NewSnowflakeAuthenticator(connections, ":name:") + _, err = NewSnowflakeAuthenticator(connections, ":name:", "test-api-key") s.ErrorContains(err, "error loading private key file: ") // JWT token provider @@ -61,12 +61,13 @@ func (s *SnowflakeAuthSuite) TestNewSnowflakeAuthenticator() { Authenticator: "snowflake_jwt", }, nil).Once() - auth, err := NewSnowflakeAuthenticator(connections, ":name:") + auth, err := NewSnowflakeAuthenticator(connections, ":name:", "test-api-key") s.NoError(err) sfauth, ok := auth.(*snowflakeAuthenticator) s.True(ok) s.NotNil(sfauth.tokenProvider) s.IsType(&snowflake.JWTTokenProvider{}, sfauth.tokenProvider) + s.Equal("test-api-key", sfauth.apiKey) // oauth token provider connections.On("Get", ":name:").Return(&snowflake.Connection{ @@ -75,12 +76,13 @@ func (s *SnowflakeAuthSuite) TestNewSnowflakeAuthenticator() { Authenticator: "oauth", }, nil).Once() - auth, err = NewSnowflakeAuthenticator(connections, ":name:") + auth, err = NewSnowflakeAuthenticator(connections, ":name:", "test-api-key") s.NoError(err) sfauth, ok = auth.(*snowflakeAuthenticator) s.True(ok) s.NotNil(sfauth.tokenProvider) s.IsType(&snowflake.OAuthTokenProvider{}, sfauth.tokenProvider) + s.Equal("test-api-key", sfauth.apiKey) } func (s *SnowflakeAuthSuite) TestAddAuthHeaders() { @@ -89,6 +91,7 @@ func (s *SnowflakeAuthSuite) TestAddAuthHeaders() { auth := &snowflakeAuthenticator{ tokenProvider: tokenProvider, + apiKey: "test-api-key", } req, err := http.NewRequest("GET", "https://example.snowflakecomputing.app/connect/#/content", nil) @@ -103,5 +106,27 @@ func (s *SnowflakeAuthSuite) TestAddAuthHeaders() { "Authorization": []string{ "Snowflake Token=\":atoken:\"", }, + "X-Rsc-Authorization": []string{ + "Key test-api-key", + }, }, req.Header) + + // Test without API key + tokenProvider.On("GetToken", "example.snowflakecomputing.app").Return(":atoken:", nil).Once() + authNoKey := &snowflakeAuthenticator{ + tokenProvider: tokenProvider, + apiKey: "", + } + + req2, err := http.NewRequest("GET", "https://example.snowflakecomputing.app/connect/#/content", nil) + s.NoError(err) + + err = authNoKey.AddAuthHeaders(req2) + s.NoError(err) + + s.Equal(http.Header{ + "Authorization": []string{ + "Snowflake Token=\":atoken:\"", + }, + }, req2.Header) } diff --git a/internal/credentials/credentials.go b/internal/credentials/credentials.go index 93e4223ae6..1ffec53d75 100644 --- a/internal/credentials/credentials.go +++ b/internal/credentials/credentials.go @@ -272,7 +272,8 @@ func (details CreateCredentialDetails) ToCredential() (*Credential, error) { return nil, NewIncompleteCredentialError() } case server_type.ServerTypeSnowflake: - if !snowflakePresent || connectPresent || connectCloudPresent || tokenAuthPresent { + // Snowflake SPCS now requires both SnowflakeConnection AND ApiKey for OIDC authentication + if !snowflakePresent || !connectPresent || connectCloudPresent || tokenAuthPresent { return nil, NewIncompleteCredentialError() } case server_type.ServerTypeConnectCloud: diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go index c1a5bea406..2ee4dd1f77 100644 --- a/internal/credentials/file_test.go +++ b/internal/credentials/file_test.go @@ -486,13 +486,13 @@ func (s *FileCredentialsServiceSuite) TestSet() { ServerType: server_type.ServerTypeSnowflake, Name: "snowcred", URL: "https://example.snowflakecomputing.app/connect", - ApiKey: "", + ApiKey: "test-snowflake-api-key", SnowflakeConnection: "snowy"}) s.NoError(err) s.Equal(newcred3.Name, "snowcred") s.Equal(newcred3.URL, "https://example.snowflakecomputing.app/connect") - s.Equal(newcred3.ApiKey, "") + s.Equal(newcred3.ApiKey, "test-snowflake-api-key") s.Equal(newcred3.SnowflakeConnection, "snowy") creds, err = cs.load() @@ -529,7 +529,7 @@ func (s *FileCredentialsServiceSuite) TestSet() { Version: 3, ServerType: server_type.ServerTypeSnowflake, URL: "https://example.snowflakecomputing.app/connect", - ApiKey: "", + ApiKey: "test-snowflake-api-key", SnowflakeConnection: "snowy", }, }, @@ -587,7 +587,7 @@ func (s *FileCredentialsServiceSuite) TestSet() { Version: 3, ServerType: server_type.ServerTypeSnowflake, URL: "https://example.snowflakecomputing.app/connect", - ApiKey: "", + ApiKey: "test-snowflake-api-key", SnowflakeConnection: "snowy", }, "cloudy": { diff --git a/internal/credentials/keyring_test.go b/internal/credentials/keyring_test.go index 1e4084f412..c4481213f5 100644 --- a/internal/credentials/keyring_test.go +++ b/internal/credentials/keyring_test.go @@ -171,13 +171,13 @@ func (s *KeyringCredentialsTestSuite) TestSet() { ServerType: server_type.ServerTypeSnowflake, Name: "sfexample", URL: "https://example.snowflakecomputing.app", - ApiKey: "", + ApiKey: "test-snowflake-api-key", SnowflakeConnection: "snow"}) s.NoError(err) s.NotNil(cred.GUID) s.Equal(cred.Name, "sfexample") s.Equal(cred.URL, "https://example.snowflakecomputing.app") - s.Equal(cred.ApiKey, "") + s.Equal(cred.ApiKey, "test-snowflake-api-key") s.Equal(cred.SnowflakeConnection, "snow") cred, err = cs.Set(CreateCredentialDetails{ diff --git a/internal/services/api/get_snowflake_connections.go b/internal/services/api/get_snowflake_connections.go index d16819c28b..d48333b0b6 100644 --- a/internal/services/api/get_snowflake_connections.go +++ b/internal/services/api/get_snowflake_connections.go @@ -27,6 +27,7 @@ type getSnowflakeConnectionsResponseBody struct { func GetSnowflakeConnectionsHandlerFunc(log logging.Logger, connections snowflake.Connections) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { serverUrl := req.URL.Query().Get("serverUrl") + apiKey := req.URL.Query().Get("apiKey") serverType, err := server_type.ServerTypeFromURL(serverUrl) if err != nil { @@ -60,6 +61,7 @@ func GetSnowflakeConnectionsHandlerFunc(log logging.Logger, connections snowflak URL: url, Insecure: false, // TODO: do we need to support insecure snowflake URLs? SnowflakeConnection: name, + ApiKey: apiKey, } timeout := time.Second * 30