Repository navigation
Conversation
Bitbucket doesn't use invalid_grant when it rejects a refresh token. A revoked token comes back as a 400 invalid_request "Invalid refresh_token". The token of a deactivated Atlassian account comes back as a 401 "User is inactive", with a code that provider_error reads as a cancellation. So these raised AuthProviderError and AuthCanceled, and apps couldn't tell them apart from an outage or a cancelled login. BitbucketOAuth2.refresh_token now raises AuthCredentialError with reauthentication_required for exactly these two responses. Both codes are generic, so this lives in the Bitbucket backend at the refresh stage instead of in provider_error. A 401 without an error code still raises as before, since it can also mean the client credentials were rejected.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Can you please look into failing tests? |
Upstream now reads invalid_request as a configuration error, so the try/except around refresh_token no longer saw Bitbucket's 400 "Invalid refresh_token" and the refresh tests failed. Moving the check into the get_provider_error hook classifies both the revoked token and the inactive user at the point the HTTP error is built, whatever the base classification turns into.
|
@codex review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2172 +/- ##
==========================================
+ Coverage 97.16% 97.18% +0.01%
==========================================
Files 374 376 +2
Lines 23666 23808 +142
Branches 1448 1452 +4
==========================================
+ Hits 22996 23138 +142
Misses 466 466
Partials 204 204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Merged, thanks for your contribution! |
We hit this after upgrading to social-core 6. A background job that refreshes Bitbucket tokens started getting dead grants as provider errors and cancelled logins instead of credential errors, so it never asked those users to reconnect. We're keeping a small workaround in our app until this lands.
Bitbucket doesn't answer a dead refresh token with OAuth's
invalid_grant. A revoked token gets a 400 (Atlassian community thread):{"error_description": "Invalid refresh_token", "error": "invalid_request"}The token of a deactivated Atlassian account gets a 401 "User is inactive" (AX-684). Its error code is one that
provider_errortreats as a user cancellation. So today the first raisesAuthProviderError(http_error) and the second raisesAuthCanceled. Code that asks the user to reconnect onAuthCredentialErrorcan't tell either one from an outage or a cancelled login.This can't go into
provider_errorthe waybad_refresh_tokendid in #2124.invalid_requestand the cancellation codes are generic, and at login a cancellation code really does mean the user cancelled. SoBitbucketOAuth2.refresh_tokennow turns a 400invalid_request, or a cancellation with status 401, intoAuthCredentialErrorwithreauthentication_required. There's no user in the loop during a refresh, so a cancellation there means Bitbucket refused the account.Every other refresh error raises as before. That includes a 401 with no error code, which can also mean the client credentials were rejected. The new tests cover both rejections, and check that a plain-text 401, an
invalid_client400 and a 503 keep their current errors. The two rejection tests fail without the change. The full suite, prek, pyright, mypy, ty and pylint pass locally.Sources:
invalid_request"Invalid refresh_token" fromhttps://bitbucket.org/site/oauth2/access_token: https://community.atlassian.com/forums/discussion/1545002/how-do-i-refresh-a-token