Repository navigation
fix: request the identity:handle OAuth scope - #600
Merged
Merged
Conversation
Changing an AT Protocol handle failed in production with "Missing required scope identity:handle". The scope was never added to the default OAuth scope list, so the PDS refused com.atproto.identity.updateHandle. This only bites users whose session comes from OAuth. A custodial session is built from stored credentials and has full account access, so the handle change worked for them. Once a user takes ownership of their own PDS account, the same call returns a 403. Taking ownership of your account should not cost you the ability to rename it. Existing sessions do not gain a newly added scope. The API already compares granted scopes against the configured list and reports the difference on /auth/me, and the profile page already shows a Reconnect banner, so affected users are prompted to re-authorize. The unit test on the default scope list is a guard. This was reported once before and closed against a change that fixed something else.
Adding identity:handle to the default scope list means every existing OAuth session is now missing a scope. The stale scope check measured every non-custodial identity against the same global list, so users hosted on an external PDS get the "New permissions are available, reconnect" banner too. That prompt is wrong for them. updateHandle rejects any identity that is not on our PDS, so identity:handle can never be used for those accounts. The banner also contradicts the one right next to it, which tells the same user that account changes must be made through their own PDS provider. Following the prompt grants us a handle rename permission we never use. The fix is the yardstick, not the result. getRequiredScopesForIdentity builds the scope list a given identity is expected to hold, and both places that report a mismatch now compare against that instead of the global list. Users on our PDS are unaffected, so people who take ownership of their account still get prompted to reconnect, which is the case the scope was added for. A scope that turns on some other condition adds a field and a case in one place. The scope itself is unchanged. It stays in the default list and in the OAuth client metadata. Narrowing what we request at authorization time is a separate change, and it can call the same function.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The problem
Changing an AT Protocol handle fails in production. The PDS returns:
DEFAULT_ATPROTO_OAUTH_SCOPESinsrc/utils/bluesky.tsnever included thatscope, so
com.atproto.identity.updateHandleis refused.Who it affects
Only users whose session comes from OAuth.
A custodial session is built from stored credentials and has full account
access, so the handle change works for them. After a user takes ownership of
their own PDS account, the same call returns a 403. Taking ownership of your
account should not cost you the ability to rename it.
This matters now because the take-ownership on-ramp is about to move a large
group of custodial users onto OAuth sessions.
The change
Two commits.
The first adds
identity:handleto the default scope list, plus a unit test.Nothing sets
ATPROTO_OAUTH_SCOPESin the infrastructure repo or in any envfile, so the code default is what every environment uses. There is no config
change to make alongside this.
The second stops us reporting that scope as missing for users who cannot use
it. See below.
Existing sessions
A token does not gain a scope that is added later, so a session authorized
before this ships is measured against the new list and reported as out of
date.
That is correct for users on our PDS. They reconnect, and the handle change
starts working.
It is wrong for users on an external PDS.
updateHandlerejects any identitythat is not on our PDS, so
identity:handlecan never be used for thoseaccounts. Those users would get the "New permissions are available, reconnect"
banner next to the banner that tells them account changes must be made through
their own PDS provider. Reconnecting would grant us a handle rename permission
we never use.
So the second commit measures each identity against the scopes it actually
needs, instead of one global list.
getRequiredScopesForIdentitybuilds thatlist, and both places that report a mismatch compare against it:
/auth/mecomputes the mismatch live from the session token.Users on our PDS still get the banner, which is the case this scope was added
for. Users on an external PDS do not, unless they are missing a scope that
does apply to them.
Still open
We request
identity:handlefrom everyone at authorization time, so anexternal user is still asked for it on the consent screen. Narrowing the
authorization request means resolving the PDS before
client.authorize, whichis a larger change. That is a separate PR, and it can call
getRequiredScopesForIdentityas well.Testing
identity:handleconditionremoved from
getRequiredScopesForIdentity, eight tests fail, and all ofthem are the external PDS cases.
tsc --noEmitreports the same 461 errors before and after this branch. Thediff is empty. This repo does not type check clean on
maintoday.Note for review
This was reported once before and closed against a change that fixed
something else, so the fix it asked for was never made. The unit test is
there to stop that happening a third time.