fix(oauth): align scopes_supported and pdscheck probing with the spec

.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:<nsid>`, `rpc:<lxm>`, `blob:<mime>`, `account:<…>`, `identity:<…>`) and permission-set scopes (`include:<nsid>`) 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.

apps/check/src/checks/oauth-discovery.ts

@@ -563,79 +563,13 @@ 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:<nsid> 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,
 	authServerResponds,
 	authServerValidates,
 	scopeAdvertisesAtproto,
 	scopeAdvertisesTransitionGeneric,
-	scopeAdvertisesResourceBuckets,
-	scopeAdvertisesPermissionSets,
 	jwksResponds,
 	jwksValidates,
 ];

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,14 +84,27 @@ 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";
 	const redirectUri = `${location.origin}${CALLBACK_PATH}`;
 	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<string, unknown> | 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:<nsid>`,
+			// `include:<nsid>`, 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<oauth.ResponseBodyError | null> => {
+				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<string, unknown> | 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 = {