Repository navigation
Recover from stale cached OAuth access tokens without a browser - #2979
Merged
sfc-gh-turbaszek merged 5 commits intoAug 6, 2026
Merged
Conversation
OAuth authorization-code (and client-credentials) connections cache the access/refresh tokens when client_store_temporary_credential=True. Two gaps made a cached token fail hard instead of recovering: Gap A - prepare() only short-circuited on a cached access token; a fresh process holding only a valid refresh token went straight to the browser. It now attempts a silent refresh with the cached refresh token before falling back to interactive auth (gated on refresh_token_enabled, so behavior is unchanged when refresh tokens are off). Async inherits this via the sync prepare(). Gap B - the connector only reauthenticated on GS code 390318 (access token *expired*). A token rejected as *invalid* (390303, "Invalid OAuth access token") fell through to a hard 250001 failure and the poisoned token stayed cached, so every subsequent run failed until the credential cache was disabled. 390303 now triggers reauthentication in both the sync and async auth paths (excluding OAuth v1.0, which cannot renew a user-supplied token), and reauthenticate() evicts the stale access token from the cache on terminal failure via a lone remove() (never a remove-then-store, which would destroy the macOS Keychain ACL). Tests: unit coverage for proactive refresh, invalid-token error handling, and access-token eviction; wiremock integration tests for the no-access-token silent refresh and the 390303 -> refresh -> login recovery flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2979 +/- ##
==========================================
+ Coverage 65.02% 65.06% +0.04%
==========================================
Files 123 123
Lines 17410 17430 +20
Branches 2387 2392 +5
==========================================
+ Hits 11321 11341 +20
+ Misses 5649 5648 -1
- Partials 440 441 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ve OAuth guard Follow-up to the OAuth stale-token recovery work, addressing PR review: - Remove the async-tier changes (aio/auth/_auth.py, aio/_connection.py). Async recovery is intentionally out of scope for this PR (NO_ASYNC_CHANGES). - prepare(): wrap the proactive silent refresh so any failure - including a transient network error surfaced through _handle_failure as DatabaseError - degrades to interactive auth instead of hard-failing the connection. This differs intentionally from reauthenticate(), where raising mid-session is correct. Adds a unit test for the raising path. - Reauth guard: replace the negative `not isinstance(auth_instance, AuthByOAuth)` with a positive `isinstance(auth_instance, AuthByOAuthBase)`. Both 390318 (expired) and 390303 (invalid) are OAuth-scoped in GS, and only OAuth v2 authenticators can renew a token; the positive check keeps OAuth v1.0 and any non-OAuth authenticator out of the reauth retry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sfc-gh-fpawlowski
approved these changes
Aug 4, 2026
sfc-gh-fpawlowski
left a comment
sfc-gh-fpawlowski
left a comment
Contributor
There was a problem hiding this comment.
LGTM, left similar UD - related note
|
|
||
| @pytest.mark.skipolddriver | ||
| @patch("snowflake.connector.auth._http_server.AuthHttpServer.DEFAULT_TIMEOUT", 30) | ||
| def test_oauth_code_invalid_access_token_triggers_refresh( |
Contributor
There was a problem hiding this comment.
Is the same scenario tested / confirmed not-occurring in the UD?
Contributor
Author
There was a problem hiding this comment.
UD backport label added to check in future
![snowflake-security-bot[bot]](https://avatars.githubusercontent.com/u/140554735?s=60&v=4){kind=small}
There was a problem hiding this comment.
Snowflake Security Review
Security grade: A — Passed ✅
No security findings after adjudication. This PR passes the Snowflake Security Review.
📊 4 of 9 files (5 tests skipped) · 5,376 lines reviewed · 5 candidates → 1 kept · 1 duplicate collapsed · retrieval: on
…oken reauth Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sfc-gh-turbaszek
deleted the
oauth-code-silent-refresh-and-invalid-token-reauth
branch
August 6, 2026 07:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OAuth authorization-code (and client-credentials) connections cache access/refresh tokens when
client_store_temporary_credential=True. Two gaps made a cached token fail hard instead of recovering — the observable symptom was250001 (08001) ... Invalid OAuth access token, only worked around by removingclient_store_temporary_credential=True.Gap A — proactive silent refresh in
prepare()prepare()only short-circuited on a cached access token. A fresh process holding only a valid refresh token went straight to the interactive browser flow. It now attempts a silent refresh with the cached refresh token before falling back to interactive auth. Gated onrefresh_token_enabled, so behavior is unchanged when refresh tokens are off. The async path inherits this via the syncprepare().Gap B — reauthenticate on invalid (not just expired) tokens
The connector only reauthenticated on GS code
390318(access token expired). A token rejected as invalid — GS code390303,"Invalid OAuth access token"(confirmed ings_error_messages.properties) — fell through to a hard250001failure, and the poisoned token stayed cached so every subsequent run failed.390303now triggers reauthentication in both sync and async auth paths, excluding OAuth v1.0 (AuthByOAuth), which cannot renew a user-supplied token.reauthenticate()now evicts the stale access token from the cache on terminal failure via a loneremove()— never a remove-then-store, which would destroy the macOS Keychain ACL. On success,_store_tokens()overwrites the stale token ACL-safely.Tests
prepare()), invalid-token error handling (390303->ReauthenticationRequest), access-token eviction on reauth failure / no-eviction on success.390303-> refresh -> retry-login recovery.All OAuth/auth suites green locally.
Notes for reviewers
prepare()/refresh runs blocking on the event loop — pre-existing (SNOW-2324426), not addressed here.🤖 Generated with Claude Code