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 eb6f0347..dc9f2662 100644 --- a/apps/check/src/lib/oauth-flow.ts +++ b/apps/check/src/lib/oauth-flow.ts @@ -73,7 +73,8 @@ 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 = +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"; @@ -83,6 +84,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"; @@ -90,7 +104,7 @@ function clientId(): string { if (isLoopback) { const params = new URLSearchParams({ redirect_uri: redirectUri, - scope: activeScope, + scope: CLIENT_METADATA_SCOPE, }); return `http://localhost?${params.toString()}`; } @@ -290,14 +304,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; @@ -331,7 +344,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", @@ -340,8 +353,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", @@ -646,46 +657,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 = @@ -718,6 +689,84 @@ 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). + // + // 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, + ); + const probeScope = async (scope: string): Promise => { + const probeParams = { + client_id: clientId(), + redirect_uri: redirectUri(), + response_type: "code", + scope, + code_challenge: codeChallenge, + code_challenge_method: "S256", + state: oauth.generateRandomState(), + }; + try { + 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 + include scope accepted: ${activeScope}`, + evidence: { actual: { selected: activeScope } }, + }; + } + const granularErr = await probeScope(GRANULAR_SCOPE); + if (!granularErr) { + activeScope = GRANULAR_SCOPE; + return { + status: "warn", + message: `AS rejected include: (${fullErr.error}) — using granular without include; permission-set tests will skip`, + evidence: { + 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 // AS to require a nonce; the first request fails with use_dpop_nonce // and the captured nonce is used automatically on retry.) @@ -918,26 +967,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 = { @@ -1000,74 +1036,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 91bc9b72..e6f55c4a 100644 --- a/packages/oauth-provider/src/provider.ts +++ b/packages/oauth-provider/src/provider.ts @@ -908,17 +908,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,