Recover OAuth credentials marked invalid instead of requiring a manual re-login - #1907
Conversation
…ing a manual re-login Once refreshAccessToken persists AuthInfoState.Invalid, softRefreshOAuth and refreshAccessToken both skip all future refresh attempts, so a credential invalidated by a single transient 401 from the token endpoint stays broken forever and ClientManager.createClient shows the 'There was an error connecting to... Please log in again.' modal in every new window — even though the stored refresh token still works. Fixes the roach motel by attempting a refresh for invalid OAuth credentials (still bounded by the existing failed-refresh backoff) and marking them Valid again when the refresh succeeds.
|
Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution. Already signed the CLA? To re-check, try refreshing the page. |
|
To enable Rovo Dev code reviews, link your GitHub account to your Atlassian account. This is a one-time task that takes less than a minute. Once your account is linked, resubmit the pull request to trigger a code review. |
|
I agree with the solution to stop considering AuthInfoState.Invalid as a final failed state. I note that OAuth 2 should return error responses with an HTTP 400 status code along with https://www.oauth.com/oauth2-servers/access-tokens/access-token-response/ If the Atlassian OAuth 2 server provides these parameters, the auth state can potentially be refined to contain more than simply Valid or Invalid. Unfortunately, the Atlassian documentation does not contain enough information about the various error/error_description that may be returned in their implementation so I agree that Invalid could be retried. |
What Is This Change?
Fixes #1827 (likely also #1818, and part of #1635).
Symptom: "There was an error connecting to Bitbucket Cloud. Please log in again." appears in every new VS Code window, even though the user is signed in and their stored refresh token is still valid. Bitbucket features silently stop working while the auth UI continues to show the user as logged in.
Root cause: once
refreshAccessTokenpersistsAuthInfoState.Invalidto secret storage, the credential can never recover:softRefreshOAuthreturns early forInvalidcredentials, so a refresh is never attempted again (ensure-no-refresh-for-invalidated-credentials #1719).refreshAccessTokenalso skipsInvalidcredentials and re-seeds the in-sessionpermanentFailurecache, so restarting VS Code doesn't help either (ensuring-invalidated-creds-are-skipped #1740).Validis a fresh login (loginManager.saveDetails).Meanwhile
ClientManager.createClientshows the modal once perClientManagerinstance (hasWarnedOfFailure), and a new instance is created per window — hence "every new window", indefinitely.The
Invalidflag is easy to acquire spuriously:OAuthRefesher.getNewTokensclassifies any 401 from the token endpoint as permanent (shouldInvalidate), including transient gateway/proxy 401s and RFC 6749invalid_clientresponses that say nothing about the user's refresh token. (Until #1747 any 403 did too.) On an affected install the credential had been stuckInvalidsince March; replaying the stored refresh token againsthttps://bitbucket.org/site/oauth2/access_tokenreturned HTTP 200 with fresh tokens — the invalidation was bogus, but the extension refused to retry for months.The change: treat persisted
Invalidas "needs re-validation" rather than "dead forever":softRefreshOAuthno longer skipsInvalidOAuth credentials; it proceeds to attempt a refresh.refreshAccessTokenno longer short-circuits onInvalid; the existing_failedRefreshCachebackoff still bounds retries (a refresh that comes backshouldInvalidatemarkspermanentFailure, so a genuinely dead token is attempted only once per session).state = AuthInfoState.Validbefore saving, so a spuriously-invalidated credential self-heals on the nextgetAuthInfocall — typically at window startup, before the modal would have been shown.Behavior for genuinely dead refresh tokens is unchanged: the recovery attempt fails, the credential stays
Invalid, and the existing modal still appears.How Has This Been Tested?
Invalidbehavior, and added coverage for: recovery success (state persisted back toValid), permanent failure (staysInvalid+permanentFailurecached), and no same-session retry after a permanent failure.authStore.test.tspasses (53/53); the rest of the unit suite is unaffected.state: 1) and replayed its refresh token against the Bitbucket token endpoint — HTTP 200, still valid — establishing that the persistedInvalidwas spurious.Basic checks:
npm run lintnpm run test(unit)Advanced checks:
Recommendations: