fix(oauth): align scopes_supported and pdscheck probing with the spec#185
Merged
Conversation
…he spec The atproto OAuth spec requires `atproto` in scopes_supported and recommends the transitional scopes when supported. Everything else — granular resource scopes (`repo:<nsid>`, `rpc:<lxm>`, `blob:<mime>`, `account:<…>`, `identity:<…>`) and permission-set scopes (`include:<nsid>`) — is parameterised and can't be enumerated. The reference oauth-provider explicitly says so. Cirrus was advertising bare prefixes (`repo`, `rpc`, `blob`, `account`, `identity`, `include`) that aren't valid as standalone scope values, which only worked because pdscheck used the same non-spec convention to detect granular support. Changes: - Cirrus oauth-provider: trim scopes_supported to the four spec-defined values. - pdscheck flow.select-scope: replace the scopes_supported lookup with a probe PAR using GRANULAR_SCOPE; on accept use granular, on invalid_scope fall back to LEGACY_SCOPE. Move the step after PKCE/DPoP-key generation so the probe has the required inputs. - pdscheck flow.par-rejects-invalid-include: drop the advertising gate and always probe with a bogus include NSID. - pdscheck: remove flow.par-accepts-advertised-include — the par-accepts-known-permission-set probe covers include acceptance via a real published lexicon. - pdscheck: remove oauth-discovery.scope-resource-buckets and scope-permission-sets — both asserted on the non-spec advertising convention.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
pdscheck | a12dded | May 24 2026, 10:24 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
cirrusdocs | a12dded | Commit Preview URL Branch Preview URL |
May 24 2026, 10:24 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
atproto-pds | a12dded | May 24 2026, 10:24 PM |
commit: |
…sion set flow.select-scope was probing GRANULAR_SCOPE which combined repo: and include:site.standard.authFull. The include NSID isn't a real published permission set, so any AS with lexicon resolution wired up (the reference, bsky.social) rejected the whole probe with invalid_scope — and pdscheck mis-attributed this to "AS doesn't support granular". Split: GRANULAR_SCOPE is now just `atproto repo:earth.cirrus.check.testrecord`, testing granular repo scope acceptance. The known-permission-set probe moves to `include:app.bsky.authFullApp` — a real published permission set from the atproto reference lexicons.
…t probe site.standard.authFull is a real published permission set (_lexicon.standard.site TXT → did:plc:re3ebnp5v7ffagz6rb6xfei4), copied from leaflet.pub's OAuth metadata. Keeping it means the probe surfaces whether an AS has lexicon resolution wired up. The GRANULAR_SCOPE split from the previous commit stays — that isolates granular-repo support from include-resolution support.
clientId() was embedding `scope=<activeScope>` in the localhost client_id URL. activeScope starts as LEGACY_SCOPE, so when flow.select-scope probed with GRANULAR_SCOPE, the request asked for `repo:…` but the client metadata only declared `atproto transition:generic`. Reference rejects with \"Scope ... is not declared in the client metadata\". The activeScope is for what we *request*; the client_id metadata is for what we're *allowed* to request. Decouple them: the loopback client_id now declares the union of every scope any probe might use. Matches the deployed public/client-metadata.json which already does this.
… too Splitting GRANULAR_SCOPE into "granular only" surfaced the granular-repo support cleanly but stripped the include from the real PAR — the consent UI then only listed the repo: scope. Probe in tiers instead: 1. granular + include (GRANULAR_WITH_INCLUDE_SCOPE) — best case; consent shows both. 2. granular only — middle case; permission-set tests skip. 3. legacy — worst case; boundary tests skip. The probe still isolates failures (granular vs include) and the user sees every scope the AS will actually grant in their consent.
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
1 participant
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.
Background
The atproto OAuth spec on `scopes_supported`:
The reference oauth-provider lists exactly the four spec-defined values and explicitly comments "Other atproto scopes can't be enumerated as they are dynamic." Granular scopes (`repo:`, `rpc:`, `blob:`, `account:<…>`, `identity:<…>`) and permission-set scopes (`include:`) are parameterised — bare prefixes aren't valid scope values that can be granted.
Cirrus was advertising bare `repo`, `rpc`, `blob`, `account`, `identity`, and `include` alongside the four spec values. pdscheck used the presence of bare `repo` as its signal for "this AS supports granular" and only exercised the granular boundary tests against Cirrus — never against the reference. The whole feedback loop was internal to a non-spec convention we'd invented.
Changes
`packages/oauth-provider/src/provider.ts` — trim `scopes_supported` to the four spec-defined values. Drops `repo`, `rpc`, `blob`, `account`, `identity`, and the conditional `include` entry.
`apps/check/src/lib/oauth-flow.ts` — `flow.select-scope` no longer reads `scopes_supported`. It now sends a probe PAR with `GRANULAR_SCOPE` and:
Step reordered after `flow.generate-pkce` and `flow.generate-dpop-key` so the probe has the required inputs. Same flow now works identically against Cirrus, bsky.social, or any other spec-compliant AS that supports granular scopes.
`apps/check/src/lib/oauth-flow.ts` — `flow.par-rejects-invalid-include` drops its advertising gate; always probes with a bogus include NSID.
`apps/check/src/lib/oauth-flow.ts` — `flow.par-accepts-advertised-include` removed. Its purpose ("AS accepts the include: it advertised") only made sense under the non-spec convention. `flow.par-accepts-known-permission-set` still exercises include resolution via a real published lexicon.
`apps/check/src/checks/oauth-discovery.ts` — `oauth-discovery.scope-resource-buckets` and `oauth-discovery.scope-permission-sets` removed. Both asserted on non-spec advertising patterns.
Test plan