-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Support Snowflake SPCS OIDC authentication with dual credentials #3215 #3268
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
base: main
Are you sure you want to change the base?
Conversation
Snowflake SPCS deployments with OIDC now require both a Snowflake connection name and a Connect API key for authentication. This change updates the credential validation logic and account authentication type detection to support this new requirement. Changes: - credentials.go: Updated validation to require both SnowflakeConnection and ApiKey for ServerTypeSnowflake credentials - account.go: Modified AuthType() to prioritize Snowflake connection detection since it's the most specific case, and added documentation about the dual authentication requirement This aligns with changes in Snowflake SPCS where proxied authentication headers no longer carry sufficient user identification information, necessitating the use of Connect API keys in addition to Snowflake tokens. Related: posit-dev/rsconnect-python#715 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implements the authentication mechanism for Snowflake SPCS with OIDC support
by sending both Snowflake tokens and Connect API keys in separate headers.
Changes:
- snowflake.go:
- Added apiKey field to snowflakeAuthenticator struct
- Updated NewSnowflakeAuthenticator to accept apiKey parameter
- Modified AddAuthHeaders to set both Authorization (Snowflake token) and
X-RSC-Authorization (Connect API key) headers
- Enhanced documentation to explain the dual-header OIDC authentication
- auth.go:
- Updated NewClientAuth to pass the API key when creating Snowflake
authenticators
The Authorization header contains the Snowflake token for proxied authentication,
while the X-RSC-Authorization header contains the Connect API key for OIDC
authentication. This dual-header approach ensures proper authentication with
Connect servers deployed in Snowflake SPCS.
Related: posit-dev/rsconnect-python#715
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Updates all tests to reflect the new dual-credential requirement for Snowflake
SPCS authentication with OIDC support.
Changes:
- snowflake_test.go:
- Updated all NewSnowflakeAuthenticator calls to include API key parameter
- Added assertions to verify API key is properly stored in authenticator
- Enhanced TestAddAuthHeaders to verify both Authorization and
X-RSC-Authorization headers are set correctly
- Added test case for authenticator without API key to ensure the header
is only set when an API key is provided
- file_test.go & keyring_test.go:
- Updated Snowflake credential creation tests to include API key
- Changed expected API key assertions from empty string to test API key
All tests pass, confirming that the OIDC authentication changes work correctly
while maintaining backward compatibility.
Related: posit-dev/rsconnect-python#715
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
… extension Adds a new input step in the VSCode extension credential creation flow to prompt users for a Connect API key when creating Snowflake SPCS credentials. Changes: - Added INPUT_SNOWFLAKE_API_KEY step to the credential creation flow - Implemented inputSnowflakeAPIKey() function that: - Prompts users for the Connect API key with password masking - Validates API key syntax using existing validation logic - Provides clear messaging about OIDC authentication requirements - Updated isValidSnowflakeAuth() to require both snowflakeConnection and apiKey - Modified inputSnowflakeConnection() to navigate to the API key input step before proceeding to credential naming The new flow for Snowflake SPCS credentials is: 1. Enter server URL 2. Select Snowflake connection 3. Enter Connect API key (NEW) 4. Name the credential This ensures users provide both authentication components needed for Snowflake SPCS deployments with OIDC authentication. Related: posit-dev/rsconnect-python#715 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Documents the Snowflake SPCS OIDC authentication changes in both the main repository and VSCode extension changelogs. Changes: - Added entries to "Unreleased > Fixed" sections explaining that Snowflake SPCS authentication now requires both a Snowflake connection name and a Connect API key - Documented the dual-header authentication approach (Authorization for Snowflake token, X-RSC-Authorization for Connect API key) - Explained the reason for the change: proxied authentication headers in Snowflake SPCS no longer carry sufficient user identification information with the move to OIDC Related: posit-dev/rsconnect-python#715 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Pinging for review |
|
Also related: This rsconnect PR adds API key alongside Snowflake auth. rstudio/rsconnect#1248 |
| } 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 != "" { |
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.
given the constructor change in auth.go - do we need to also check in the guard here that acct.ApiKey is configured properly? It seems like AuthTypeSnowflake should only be valid if both are configured.
| } | ||
| case server_type.ServerTypeSnowflake: | ||
| if !snowflakePresent || connectPresent || connectCloudPresent || tokenAuthPresent { | ||
| // Snowflake SPCS now requires both SnowflakeConnection AND ApiKey for OIDC authentication |
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.
| // Snowflake SPCS now requires both SnowflakeConnection AND ApiKey for OIDC authentication | |
| // Snowflake SPCS requires both SnowflakeConnection AND ApiKey for OIDC authentication |
(very minor suggestion, obviously)
| // Returned connections include the validated server URL they were | ||
| // successfully tested against. | ||
| // | ||
| // If apiKey is provided, connections will be tested with both the |
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.
if I'm understanding this PR correctly, apiKey is now strictly required, right?
Does treating the apiKey like an optional parameter here still make sense?
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.
It doesn't look it should be optional, agreed.
How will they know to do this? What is the mechanism for updating the existing credential? |
dotNomad
left a comment
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.
Noticed a few things to adjust, some questions, and a few suggestions to improve clarity.
| // 200 - ok | ||
| // 500 - internal server error | ||
| list(serverUrl: string) { | ||
| list(serverUrl: string, apiKey?: string) { |
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.
Based on the description of the PR the apiKey here is in fact not optional and a requirement of this API. Looking at get_snowflake_connections.go the apiKey is required.
| list(serverUrl: string, apiKey?: string) { | |
| list(serverUrl: string, apiKey: string) { |
| // Returned connections include the validated server URL they were | ||
| // successfully tested against. | ||
| // | ||
| // If apiKey is provided, connections will be tested with both the |
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.
It doesn't look it should be optional, agreed.
| export const fetchSnowflakeConnections = async (serverUrl: string) => { | ||
| export const fetchSnowflakeConnections = async ( | ||
| serverUrl: string, | ||
| apiKey?: string, |
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.
Similar note here - this doesn't look like it can be optional.
| apiKey?: string, | |
| apiKey: string, |
| - 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. |
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.
This needs to be placed in the [Unreleased] section. It looks like there isn't an issue to link so all good there.
| ### 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. |
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.
Can leave this out. We fill this file on release.
| 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); |
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.
Looks like we always send up an apiKey so no worries removing the optional in the other spots noted.
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.
Wait how does the testing of the snowflake connection work when the INPUT_SNOWFLAKE_API_KEY step happens afterwords? Wouldn't the apiKey always be "" here since we have not gotten to the inputSnowflakeAPIKey step?
Oh nevermind I see the that we go from INPUT_SNOWFLAKE_API_KEY to INPUT_SNOWFLAKE_CONN. It could be helpful to re-order those in the steps Record to mimic the order, similar suggestion with the order of the functions in the file.
| // *************************************************************** | ||
| // Step: Enter the API Key for Snowflake SPCS (Snowflake only) | ||
| // *************************************************************** | ||
| async function inputSnowflakeAPIKey( |
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.
Just for my own clarification - is this a Connect API Key when going through Snowflake SPCS? Could be good to clarify a bit here.
Support Snowflake SPCS OIDC authentication with dual credentials
Intent
Adapts the Snowflake SPCS OIDC authentication changes from
rsconnect-python#715 to the
publisher repository.
Prior to recent changes on the Snowflake side, proxied authentication headers carried
enough information for Connect running in Snowflake SPCS to identify users. With the move
to OIDC, Connect servers no longer trust Snowflake headers for username identification.
This requires users to provide both a Snowflake connection (for proxied authentication)
and a Connect API key (for OIDC authentication).
I have tested and validated this works with publishing internally in workbench to connect and externally from positron on my PC to the dogfood instance https://bf2oiaib-duloftf-posit-software-pbc-dev.snowflakecomputing.app/.
This needs a review since I do not know how VS code extensions work and this PR was largely written by Claude with extensive prompts.
Type of Change
User Impact
Breaking Change for Snowflake SPCS Users:
Connect API key
Automated Tests
snowflake_test.goto verify dual-header authenticationfile_test.goandkeyring_test.goto test credential validation with bothfields
The tests verify:
AuthorizationandX-RSC-Authorizationheaders are set correctlyTesting:
Run the authentication and credential tests:
go test ./internal/credentials/... ./internal/api_client/auth/... ./internal/accounts/... -vCompare with rsconnect-python:
Checklist