From 27cf82087635f6b1d9c86fe5c0ab24fd024ed84a Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 22:51:29 +0100 Subject: [PATCH 1/5] fix(oauth-provider, check): align scopes_supported and probing with the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atproto OAuth spec requires `atproto` in scopes_supported and recommends the transitional scopes when supported. Everything else — granular resource scopes (`repo:`, `rpc:`, `blob:`, `account:<…>`, `identity:<…>`) and permission-set scopes (`include:`) — 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. --- .changeset/scopes-supported-spec-compliant.md | 5 + apps/check/src/checks/oauth-discovery.ts | 66 ------ apps/check/src/lib/oauth-flow.ts | 208 +++++++----------- packages/oauth-provider/src/provider.ts | 10 +- 4 files changed, 88 insertions(+), 201 deletions(-) create mode 100644 .changeset/scopes-supported-spec-compliant.md diff --git a/.changeset/scopes-supported-spec-compliant.md b/.changeset/scopes-supported-spec-compliant.md new file mode 100644 index 00000000..7a2b1784 --- /dev/null +++ b/.changeset/scopes-supported-spec-compliant.md @@ -0,0 +1,5 @@ +--- +"@getcirrus/oauth-provider": patch +--- + +`scopes_supported` in the authorization-server metadata now lists only the values the spec calls out: `atproto`, `transition:generic`, `transition:email`, `transition:chat.bsky`. Granular resource scopes (`repo:`, `rpc:`, `blob:`, `account:<…>`, `identity:<…>`) and permission-set scopes (`include:`) are parameterised and aren't enumerable, so bare prefixes like `repo` or `include` are no longer advertised — clients discover support by attempting the scope and falling back on `invalid_scope`, matching the reference PDS. diff --git a/apps/check/src/checks/oauth-discovery.ts b/apps/check/src/checks/oauth-discovery.ts index c808a1be..222c0e1f 100644 --- a/apps/check/src/checks/oauth-discovery.ts +++ b/apps/check/src/checks/oauth-discovery.ts @@ -563,70 +563,6 @@ const scopeAdvertisesTransitionGeneric = scopeAdvertises( }, ); -const scopeAdvertisesResourceBuckets = scopeAdvertises( - "oauth-discovery.scope-resource-buckets", - "Auth server advertises granular resource scopes", - (scopes) => { - // Per atproto.com/specs/permission, granular scopes are bare resource- - // type tokens (repo, rpc, blob, account, identity) in scopes_supported. - // Clients construct fully-qualified scopes by appending parameters at - // request time (e.g. `repo:my.collection?action=create`). - const RESOURCES = ["repo", "rpc", "blob", "identity", "account"] as const; - const present = RESOURCES.filter((r) => scopes.includes(r)); - if (present.length === 0) { - return { - status: "warn", - message: - "no granular resource scopes (repo, rpc, blob, identity, account) advertised — AS supports only legacy transition:* bundles", - evidence: { - expected: - "scopes_supported to include resource-type tokens: repo, rpc, blob, account, identity", - actual: scopes, - }, - }; - } - const missing = RESOURCES.filter((r) => !scopes.includes(r)); - if (missing.length > 0) { - return { - status: "warn", - message: `partial: advertises ${present.join(", ")}; missing ${missing.join(", ")}`, - evidence: { actual: { present, missing } }, - }; - } - return { - status: "pass", - message: `all five granular resources advertised: ${present.join(", ")}`, - evidence: { actual: present }, - }; - }, -); - -const scopeAdvertisesPermissionSets = scopeAdvertises( - "oauth-discovery.scope-permission-sets", - "Auth server advertises permission set support", - (scopes) => { - // The AS advertises `include` as a resource type to signal it supports - // permission sets; specific include: scopes are dynamically - // resolved at PAR time via lexicon resolution, not enumerated here. - if (!scopes.includes("include")) { - return { - status: "warn", - message: - "`include` not in scopes_supported — AS does not advertise permission set resolution", - evidence: { - expected: "`include` token in scopes_supported", - actual: scopes, - }, - }; - } - return { - status: "pass", - message: "`include` advertised — AS resolves permission sets dynamically via lexicon resolution", - evidence: { actual: ["include"] }, - }; - }, -); - export const oauthDiscoveryChecks: Check[] = [ protectedResourceResponds, protectedResourceValidates, @@ -634,8 +570,6 @@ export const oauthDiscoveryChecks: Check[] = [ authServerValidates, scopeAdvertisesAtproto, scopeAdvertisesTransitionGeneric, - scopeAdvertisesResourceBuckets, - scopeAdvertisesPermissionSets, jwksResponds, jwksValidates, ]; diff --git a/apps/check/src/lib/oauth-flow.ts b/apps/check/src/lib/oauth-flow.ts index 57568259..263a0fe8 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -289,14 +289,13 @@ const PRE_REDIRECT_STEPS = [ "flow.discover-auth-server", "flow.validate-auth-server-metadata", "flow.atproto-conformance", - "flow.select-scope", "flow.generate-pkce", "flow.generate-dpop-key", + "flow.select-scope", "flow.send-par", "flow.par-response-shape", "flow.par-rejects-unregistered-redirect-uri", "flow.par-rejects-invalid-include", - "flow.par-accepts-advertised-include", "flow.par-accepts-known-permission-set", "flow.build-authorization-url", ] as const; @@ -330,7 +329,7 @@ function initialStepsFor(ids: readonly string[]): FlowStep[] { "Auth server metadata validates (oauth4webapi)", "flow.atproto-conformance": "AT Proto OAuth conformance", "flow.select-scope": - "Select scope (granular when AS advertises, legacy otherwise)", + "Select scope (granular when AS accepts, legacy otherwise)", "flow.generate-pkce": "Generate PKCE code verifier and challenge", "flow.generate-dpop-key": "Generate DPoP ES256 keypair", "flow.send-par": "Send pushed authorization request", @@ -339,8 +338,6 @@ function initialStepsFor(ids: readonly string[]): FlowStep[] { "PAR rejects unregistered redirect_uri (RFC 6749 §3.1.2.4)", "flow.par-rejects-invalid-include": "PAR rejects a nonexistent permission set include:", - "flow.par-accepts-advertised-include": - "PAR accepts an include: scope advertised in scopes_supported", "flow.par-accepts-known-permission-set": "PAR accepts include:site.standard.authFull (a published, lexicon-resolved permission set)", "flow.build-authorization-url": "Build authorization URL", @@ -645,46 +642,6 @@ export function startPreRedirectFlow(target: string): FlowRun { }); // 5b. Select scope based on what the AS advertises. - await runStep("flow.select-scope", async () => { - const supported = ( - (state.authServer as Record | undefined) - ?.scopes_supported as string[] | undefined - )?.filter((s) => typeof s === "string") ?? []; - const hasGranular = supported.some( - (s) => - s === "repo" || - s.startsWith("repo:") || - s.startsWith("repo "), - ); - activeScope = hasGranular ? GRANULAR_SCOPE : LEGACY_SCOPE; - if (hasGranular) { - return { - status: "pass", - message: `granular scope: ${activeScope}`, - evidence: { - actual: { - scopesSupported: supported, - selected: activeScope, - }, - }, - }; - } - return { - status: "warn", - message: `AS doesn't advertise repo:* scopes — falling back to legacy ${LEGACY_SCOPE}; granular boundary tests will skip`, - evidence: { - expected: - "scopes_supported to include at least one repo:* (Phase 2 granular) scope", - actual: { - scopesSupported: supported, - selected: activeScope, - }, - error: - "PDS doesn't support Phase 2 granular scopes — the verifier can't differentiate scope enforcement using this AS.", - }, - }; - }); - // 6. Generate PKCE const codeVerifier = oauth.generateRandomCodeVerifier(); const codeChallenge = @@ -717,6 +674,77 @@ export function startPreRedirectFlow(target: string): FlowRun { }, })); + // 7b. Select scope by *probing*, not by reading scopes_supported. The + // atproto OAuth spec only requires `atproto` (and transitional scopes + // when supported) in scopes_supported; granular scopes (`repo:`, + // `include:`, etc.) are parameterised and "cannot be enumerated + // as they are dynamic" (atproto reference oauth-provider comment). So + // we send a probe PAR with the granular scope; if the AS accepts, use + // it. If it rejects with invalid_scope, fall back to LEGACY_SCOPE. + const probeDpop = oauth.DPoP( + { [oauth.clockSkew]: 0 }, + dpopKeyPair, + ); + await runStep("flow.select-scope", async () => { + const probeParams = { + client_id: clientId(), + redirect_uri: redirectUri(), + response_type: "code", + scope: GRANULAR_SCOPE, + code_challenge: codeChallenge, + code_challenge_method: "S256", + state: oauth.generateRandomState(), + }; + const probe = async () => { + const res = await oauth.pushedAuthorizationRequest( + state.authServer!, + { client_id: clientId() }, + oauth.None(), + probeParams, + { DPoP: probeDpop }, + ); + return await oauth.processPushedAuthorizationResponse( + state.authServer!, + { client_id: clientId() }, + res, + ); + }; + try { + await withNonceRetry(probe); + activeScope = GRANULAR_SCOPE; + return { + status: "pass", + message: `granular scope accepted: ${activeScope}`, + evidence: { actual: { selected: activeScope } }, + }; + } catch (error) { + if ( + error instanceof oauth.ResponseBodyError && + (error.error === "invalid_scope" || + error.error === "invalid_request") + ) { + activeScope = LEGACY_SCOPE; + return { + status: "warn", + message: `AS rejected granular scope (${error.error}) — falling back to ${LEGACY_SCOPE}; granular boundary tests will skip`, + evidence: { + response: { status: error.status, body: error.cause }, + actual: { selected: activeScope }, + }, + }; + } + activeScope = LEGACY_SCOPE; + return { + status: "warn", + message: `probe inconclusive: ${error instanceof Error ? error.message : String(error)} — falling back to ${LEGACY_SCOPE}`, + evidence: { + error: String(error), + actual: { selected: activeScope }, + }, + }; + } + }); + // 8. Send PAR (with DPoP-nonce retry built in — RFC 9449 §8 allows the // AS to require a nonce; the first request fails with use_dpop_nonce // and the captured nonce is used automatically on retry.) @@ -917,26 +945,13 @@ export function startPreRedirectFlow(target: string): FlowRun { }, ); - // 9c. Permission-set probes: only meaningful if the AS advertises any - // `include:*` scope in scopes_supported. Skip otherwise. - const advertisedScopes = ( - (state.authServer as Record | undefined) - ?.scopes_supported as string[] | undefined - )?.filter((s) => typeof s === "string") ?? []; - const advertisedIncludes = advertisedScopes.filter((s) => - s.startsWith("include:"), - ); + // 9c. Permission-set probes. We don't gate on scopes_supported — the + // spec doesn't require ASes to enumerate include:* scopes since they + // resolve dynamically via lexicon resolution. // 9c.i — request a clearly-nonexistent permission set. The AS should // reject with invalid_scope (or similar) once it tries to resolve. await runStep("flow.par-rejects-invalid-include", async () => { - if (advertisedIncludes.length === 0) { - return { - status: "skip", - message: - "AS doesn't advertise any include:* scopes — permission set support not exercisable", - }; - } const bogusInclude = "include:earth.cirrus.check.invalidnonexistentpermissionset"; const probeParams = { @@ -999,74 +1014,9 @@ export function startPreRedirectFlow(target: string): FlowRun { } }); - // 9c.ii — request the AS's OWN advertised include: scope. If the AS - // advertises it in scopes_supported, it must be able to accept and - // resolve it. Rejecting your own advertised scope is a real bug. - await runStep("flow.par-accepts-advertised-include", async () => { - if (advertisedIncludes.length === 0) { - return { - status: "skip", - message: - "AS doesn't advertise any include:* scopes — nothing to probe", - }; - } - const advertised = advertisedIncludes[0]!; - const probeParams = { - client_id: clientId(), - redirect_uri: redirectUri(), - response_type: "code", - scope: `atproto ${advertised}`, - code_challenge: codeChallenge, - code_challenge_method: "S256", - state: oauth.generateRandomState(), - }; - const attempt = async () => { - const res = await oauth.pushedAuthorizationRequest( - state.authServer!, - { client_id: clientId() }, - oauth.None(), - probeParams, - { DPoP: dpop }, - ); - return await oauth.processPushedAuthorizationResponse( - state.authServer!, - { client_id: clientId() }, - res, - ); - }; - try { - const accepted = await withNonceRetry(attempt); - return { - status: "pass", - message: `AS accepted its own advertised ${advertised} (request_uri expires in ${accepted.expires_in}s)`, - evidence: { - response: { body: accepted }, - actual: { probed: advertised }, - }, - }; - } catch (error) { - if (error instanceof oauth.ResponseBodyError) { - return { - status: "fail", - message: `AS rejected ${advertised} (its own advertised scope): ${error.error}${error.cause?.error_description ? ` — ${error.cause.error_description}` : ""}`, - evidence: { - response: { status: error.status, body: error.cause }, - error: `Permission set ${advertised} is listed in scopes_supported but PAR rejects it — the AS is advertising a scope it can't actually honor.`, - }, - }; - } - return { - status: "warn", - message: `probe inconclusive: ${error instanceof Error ? error.message : String(error)}`, - evidence: { error: String(error) }, - }; - } - }); - - // 9c.iii — request a published permission set (`site.standard.authFull`) - // that does NOT need to appear in scopes_supported. This tests whether - // the AS can dynamically resolve `include:*` NSIDs via lexicon resolution. - // An AS that only supports its own pre-advertised includes will fail here. + // 9c.ii — request a published permission set (`site.standard.authFull`) + // to test whether the AS can dynamically resolve `include:*` NSIDs via + // lexicon resolution. await runStep("flow.par-accepts-known-permission-set", async () => { const knownInclude = "include:site.standard.authFull"; const probeParams = { diff --git a/packages/oauth-provider/src/provider.ts b/packages/oauth-provider/src/provider.ts index 5491c63a..725c0144 100644 --- a/packages/oauth-provider/src/provider.ts +++ b/packages/oauth-provider/src/provider.ts @@ -907,17 +907,15 @@ export class ATProtoOAuthProvider { grant_types_supported: ["authorization_code", "refresh_token"], code_challenge_methods_supported: ["S256"], token_endpoint_auth_methods_supported: ["none", "private_key_jwt"], + // Per atproto OAuth spec: must include "atproto"; transitional scopes + // included when supported. Granular resource scopes (repo:, rpc:, blob:, + // account:, identity:) and permission-set include: scopes are + // parameterized and aren't enumerable, so they aren't listed. scopes_supported: [ "atproto", "transition:generic", "transition:email", "transition:chat.bsky", - "repo", - "rpc", - "blob", - "account", - "identity", - ...(this.permissionSetResolver ? ["include"] : []), ], subject_types_supported: ["public"], authorization_response_iss_parameter_supported: true, From f43863eb60a18f209de00271a7869e0d5c3219ea Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 22:58:00 +0100 Subject: [PATCH 2/5] fix(check): probe granular without include; use real published permission set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/check/src/lib/oauth-flow.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/apps/check/src/lib/oauth-flow.ts b/apps/check/src/lib/oauth-flow.ts index 263a0fe8..3f807880 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -72,8 +72,7 @@ interface PersistedState { // the granular scope at PAR ("invalid_scope: not declared in client metadata"), // so we don't even attempt it on those servers. const LEGACY_SCOPE = "atproto transition:generic"; -const GRANULAR_SCOPE = - "atproto repo:earth.cirrus.check.testrecord include:site.standard.authFull"; +const GRANULAR_SCOPE = "atproto repo:earth.cirrus.check.testrecord"; const OUT_OF_SCOPE_COLLECTION = "earth.cirrus.check.othertestrecord"; const CALLBACK_PATH = "/oauth/flow-callback"; @@ -339,7 +338,7 @@ function initialStepsFor(ids: readonly string[]): FlowStep[] { "flow.par-rejects-invalid-include": "PAR rejects a nonexistent permission set include:", "flow.par-accepts-known-permission-set": - "PAR accepts include:site.standard.authFull (a published, lexicon-resolved permission set)", + "PAR accepts include:app.bsky.authFullApp (a published, lexicon-resolved permission set)", "flow.build-authorization-url": "Build authorization URL", "flow.callback-params-present": "Callback has code, state, iss", "flow.iss-matches": "iss parameter matches auth server (RFC 9207)", @@ -1014,11 +1013,11 @@ export function startPreRedirectFlow(target: string): FlowRun { } }); - // 9c.ii — request a published permission set (`site.standard.authFull`) + // 9c.ii — request a published permission set (`app.bsky.authFullApp`) // to test whether the AS can dynamically resolve `include:*` NSIDs via // lexicon resolution. await runStep("flow.par-accepts-known-permission-set", async () => { - const knownInclude = "include:site.standard.authFull"; + const knownInclude = "include:app.bsky.authFullApp"; const probeParams = { client_id: clientId(), redirect_uri: redirectUri(), @@ -1046,7 +1045,7 @@ export function startPreRedirectFlow(target: string): FlowRun { const accepted = await withNonceRetry(attempt); return { status: "pass", - message: `AS resolved site.standard.authFull (request_uri expires in ${accepted.expires_in}s)`, + message: `AS resolved app.bsky.authFullApp (request_uri expires in ${accepted.expires_in}s)`, evidence: { response: { body: accepted }, actual: { probed: knownInclude }, @@ -1063,7 +1062,7 @@ export function startPreRedirectFlow(target: string): FlowRun { "AS resolves the published lexicon and accepts the include:", actual: error.error, error: - "site.standard.authFull is a published permission set lexicon. Rejecting it means this AS doesn't support dynamic lexicon-based permission-set resolution.", + "app.bsky.authFullApp is a published permission set lexicon. Rejecting it means this AS doesn't support dynamic lexicon-based permission-set resolution.", }, }; } From 40d33ca70eaece390447b803e209f3614e39719f Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 22:59:51 +0100 Subject: [PATCH 3/5] revert(check): keep site.standard.authFull as the known-permission-set probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/check/src/lib/oauth-flow.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/check/src/lib/oauth-flow.ts b/apps/check/src/lib/oauth-flow.ts index 3f807880..10b193ba 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -338,7 +338,7 @@ function initialStepsFor(ids: readonly string[]): FlowStep[] { "flow.par-rejects-invalid-include": "PAR rejects a nonexistent permission set include:", "flow.par-accepts-known-permission-set": - "PAR accepts include:app.bsky.authFullApp (a published, lexicon-resolved permission set)", + "PAR accepts include:site.standard.authFull (a published, lexicon-resolved permission set)", "flow.build-authorization-url": "Build authorization URL", "flow.callback-params-present": "Callback has code, state, iss", "flow.iss-matches": "iss parameter matches auth server (RFC 9207)", @@ -1013,11 +1013,11 @@ export function startPreRedirectFlow(target: string): FlowRun { } }); - // 9c.ii — request a published permission set (`app.bsky.authFullApp`) + // 9c.ii — request a published permission set (`site.standard.authFull`) // to test whether the AS can dynamically resolve `include:*` NSIDs via // lexicon resolution. await runStep("flow.par-accepts-known-permission-set", async () => { - const knownInclude = "include:app.bsky.authFullApp"; + const knownInclude = "include:site.standard.authFull"; const probeParams = { client_id: clientId(), redirect_uri: redirectUri(), @@ -1045,7 +1045,7 @@ export function startPreRedirectFlow(target: string): FlowRun { const accepted = await withNonceRetry(attempt); return { status: "pass", - message: `AS resolved app.bsky.authFullApp (request_uri expires in ${accepted.expires_in}s)`, + message: `AS resolved site.standard.authFull (request_uri expires in ${accepted.expires_in}s)`, evidence: { response: { body: accepted }, actual: { probed: knownInclude }, @@ -1062,7 +1062,7 @@ export function startPreRedirectFlow(target: string): FlowRun { "AS resolves the published lexicon and accepts the include:", actual: error.error, error: - "app.bsky.authFullApp is a published permission set lexicon. Rejecting it means this AS doesn't support dynamic lexicon-based permission-set resolution.", + "site.standard.authFull is a published permission set lexicon. Rejecting it means this AS doesn't support dynamic lexicon-based permission-set resolution.", }, }; } From 9ef4fd09bf04da496aa81bc39a1c9dfd8f4f72b0 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 23:02:13 +0100 Subject: [PATCH 4/5] fix(check): declare every probe-able scope in the loopback client_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clientId() was embedding `scope=` 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. --- apps/check/src/lib/oauth-flow.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/check/src/lib/oauth-flow.ts b/apps/check/src/lib/oauth-flow.ts index 10b193ba..1e52e7cb 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -81,6 +81,19 @@ const CALLBACK_PATH = "/oauth/flow-callback"; // flow at a time so this is safe. let activeScope: string = LEGACY_SCOPE; +// Superset of every scope the flow might request — used in the client_id +// metadata so an AS that enforces "requested scope must be declared" (RFC +// 9101 / atproto OAuth) accepts both the legacy and granular probes. The +// actual scope sent at PAR time is `activeScope`, selected by probe. +const CLIENT_METADATA_SCOPE = [ + "atproto", + "transition:generic", + GRANULAR_SCOPE.split(" ").filter((s) => s !== "atproto").join(" "), + "include:site.standard.authFull", +] + .join(" ") + .trim(); + function clientId(): string { const isLoopback = location.hostname === "localhost" || location.hostname === "127.0.0.1"; @@ -88,7 +101,7 @@ function clientId(): string { if (isLoopback) { const params = new URLSearchParams({ redirect_uri: redirectUri, - scope: activeScope, + scope: CLIENT_METADATA_SCOPE, }); return `http://localhost?${params.toString()}`; } From bdbb214753e5a429b502c895494bc56065d54fd9 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Sun, 24 May 2026 23:13:26 +0100 Subject: [PATCH 5/5] fix(check): three-tier probe so the consent UI gets the include scope too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/check/src/lib/oauth-flow.ts | 91 ++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/apps/check/src/lib/oauth-flow.ts b/apps/check/src/lib/oauth-flow.ts index 1e52e7cb..bb61f4b1 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -73,6 +73,8 @@ interface PersistedState { // so we don't even attempt it on those servers. const LEGACY_SCOPE = "atproto transition:generic"; const GRANULAR_SCOPE = "atproto repo:earth.cirrus.check.testrecord"; +const GRANULAR_WITH_INCLUDE_SCOPE = + "atproto repo:earth.cirrus.check.testrecord include:site.standard.authFull"; const OUT_OF_SCOPE_COLLECTION = "earth.cirrus.check.othertestrecord"; const CALLBACK_PATH = "/oauth/flow-callback"; @@ -690,71 +692,78 @@ export function startPreRedirectFlow(target: string): FlowRun { // atproto OAuth spec only requires `atproto` (and transitional scopes // when supported) in scopes_supported; granular scopes (`repo:`, // `include:`, etc.) are parameterised and "cannot be enumerated - // as they are dynamic" (atproto reference oauth-provider comment). So - // we send a probe PAR with the granular scope; if the AS accepts, use - // it. If it rejects with invalid_scope, fall back to LEGACY_SCOPE. + // as they are dynamic" (atproto reference oauth-provider comment). + // + // Three-tier probe: try granular + include, fall back to granular only, + // then to legacy. The user sees the richest scope set the AS will accept + // on the consent UI, and the boundary tests still run whenever granular + // works. const probeDpop = oauth.DPoP( { [oauth.clockSkew]: 0 }, dpopKeyPair, ); - await runStep("flow.select-scope", async () => { + const probeScope = async (scope: string): Promise => { const probeParams = { client_id: clientId(), redirect_uri: redirectUri(), response_type: "code", - scope: GRANULAR_SCOPE, + scope, code_challenge: codeChallenge, code_challenge_method: "S256", state: oauth.generateRandomState(), }; - const probe = async () => { - const res = await oauth.pushedAuthorizationRequest( - state.authServer!, - { client_id: clientId() }, - oauth.None(), - probeParams, - { DPoP: probeDpop }, - ); - return await oauth.processPushedAuthorizationResponse( - state.authServer!, - { client_id: clientId() }, - res, - ); - }; try { - await withNonceRetry(probe); - activeScope = GRANULAR_SCOPE; + await withNonceRetry(async () => { + const res = await oauth.pushedAuthorizationRequest( + state.authServer!, + { client_id: clientId() }, + oauth.None(), + probeParams, + { DPoP: probeDpop }, + ); + return await oauth.processPushedAuthorizationResponse( + state.authServer!, + { client_id: clientId() }, + res, + ); + }); + return null; + } catch (error) { + if (error instanceof oauth.ResponseBodyError) return error; + throw error; + } + }; + await runStep("flow.select-scope", async () => { + const fullErr = await probeScope(GRANULAR_WITH_INCLUDE_SCOPE); + if (!fullErr) { + activeScope = GRANULAR_WITH_INCLUDE_SCOPE; return { status: "pass", - message: `granular scope accepted: ${activeScope}`, + message: `granular + include scope accepted: ${activeScope}`, evidence: { actual: { selected: activeScope } }, }; - } catch (error) { - if ( - error instanceof oauth.ResponseBodyError && - (error.error === "invalid_scope" || - error.error === "invalid_request") - ) { - activeScope = LEGACY_SCOPE; - return { - status: "warn", - message: `AS rejected granular scope (${error.error}) — falling back to ${LEGACY_SCOPE}; granular boundary tests will skip`, - evidence: { - response: { status: error.status, body: error.cause }, - actual: { selected: activeScope }, - }, - }; - } - activeScope = LEGACY_SCOPE; + } + const granularErr = await probeScope(GRANULAR_SCOPE); + if (!granularErr) { + activeScope = GRANULAR_SCOPE; return { status: "warn", - message: `probe inconclusive: ${error instanceof Error ? error.message : String(error)} — falling back to ${LEGACY_SCOPE}`, + message: `AS rejected include: (${fullErr.error}) — using granular without include; permission-set tests will skip`, evidence: { - error: String(error), + response: { status: fullErr.status, body: fullErr.cause }, actual: { selected: activeScope }, }, }; } + activeScope = LEGACY_SCOPE; + return { + status: "warn", + message: `AS rejected granular scope (${granularErr.error}) — falling back to ${LEGACY_SCOPE}; granular boundary tests will skip`, + evidence: { + response: { status: granularErr.status, body: granularErr.cause }, + actual: { selected: activeScope }, + }, + }; }); // 8. Send PAR (with DPoP-nonce retry built in — RFC 9449 §8 allows the