fix(oauth-provider): resolve include: scopes at PAR time

.changeset/par-resolve-includes-eagerly.md

@@ -0,0 +1,5 @@
+---
+"@getcirrus/oauth-provider": patch
+---
+
+PAR (`/oauth/par`) now resolves every `include:<nsid>` permission-set scope eagerly and rejects with `invalid_scope` when an include points at a nonexistent or non-permission-set lexicon. Previously the resolver only ran at the authorize step, so clients with a typo in an include scope got a fresh `request_uri` from PAR and only learned about the bad scope at consent time. Matches reference oauth-provider behaviour (`request-manager.ts:297-313`).

packages/oauth-provider/src/par.ts

@@ -5,10 +5,16 @@
 
 import type { OAuthParResponse } from "@atproto/oauth-types";
 import type { ClientResolver } from "./client-resolver.js";
+import type { PermissionSetResolver } from "./permission-sets.js";
 import type { OAuthStorage, PARData } from "./storage.js";
 import { randomString } from "./encoding.js";
 import { parseRequestBody } from "./provider.js";
-import { ATPROTO_SCOPE, ScopeParseError, parseScope } from "./scopes.js";
+import {
+	ATPROTO_SCOPE,
+	ScopeParseError,
+	expandScope,
+	parseScope,
+} from "./scopes.js";
 
 export type { OAuthParResponse };
 
@@ -57,7 +63,7 @@ export class PARHandler {
 	private clientResolver: ClientResolver;
 	private issuer: string;
 	private expiresIn: number;
-	private allowIncludes: boolean;
+	private permissionSetResolver?: PermissionSetResolver;
 
 	/**
 	 * Create a PAR handler
@@ -67,23 +73,24 @@ export class PARHandler {
 	 *   RFC 6749 §3.1.2.4.
 	 * @param issuer The OAuth issuer URL
 	 * @param expiresIn PAR expiration time in seconds (default: 90)
-	 * @param allowIncludes Whether `include:` (permission set) scopes are
-	 *   accepted. Should mirror the parent provider's
-	 *   `permissionSetResolver` configuration so PAR rejects upfront rather
-	 *   than letting the request fail at consent.
+	 * @param permissionSetResolver Resolver for `include:<nsid>` permission
+	 *   set scopes. When provided, PAR resolves every requested include
+	 *   eagerly and rejects with `invalid_scope` on resolution failure,
+	 *   matching the reference oauth-provider. When omitted, `include:`
+	 *   scopes are rejected outright at the parse step.
 	 */
 	constructor(
 		storage: OAuthStorage,
 		clientResolver: ClientResolver,
 		issuer: string,
 		expiresIn: number = DEFAULT_EXPIRES_IN,
-		allowIncludes: boolean = false,
+		permissionSetResolver?: PermissionSetResolver,
 	) {
 		this.storage = storage;
 		this.clientResolver = clientResolver;
 		this.issuer = issuer;
 		this.expiresIn = expiresIn;
-		this.allowIncludes = allowIncludes;
+		this.permissionSetResolver = permissionSetResolver;
 	}
 
 	/**
@@ -178,12 +185,16 @@ export class PARHandler {
 
 		const scope = params.scope ?? ATPROTO_SCOPE;
 		params.scope = scope;
+		const allowIncludes = !!this.permissionSetResolver;
 		try {
-			// PAR is a deferred authorize: when permission sets are enabled,
-			// includes are accepted here and resolved at the authorize step.
-			// When disabled, reject upfront so the client gets a clear error
-			// rather than a dead-end at consent time.
-			parseScope(scope, { allowIncludes: this.allowIncludes });
+			parseScope(scope, { allowIncludes });
+			// Eagerly resolve every `include:<nsid>` against the lexicon so a
+			// nonexistent permission set fails fast with invalid_scope here
+			// rather than dead-ending at the consent UI. Matches reference
+			// oauth-provider behaviour (request-manager.ts:297).
+			if (allowIncludes && scope.includes("include:")) {
+				await expandScope(scope, this.permissionSetResolver);
+			}
 		} catch (e) {
 			if (e instanceof ScopeParseError) {
 				return this.errorResponse("invalid_scope", e.message, 400);

packages/oauth-provider/src/provider.ts

@@ -222,7 +222,7 @@ export class ATProtoOAuthProvider {
 			this.clientResolver,
 			config.issuer,
 			undefined,
-			!!config.permissionSetResolver,
+			config.permissionSetResolver,
 		);
 		this.verifyUser = config.verifyUser;
 		this.getCurrentUser = config.getCurrentUser;

packages/oauth-provider/test/par.test.ts

@@ -1,6 +1,10 @@
 import { describe, it, expect, beforeEach } from "vitest";
 import { ClientResolver } from "../src/client-resolver.js";
 import { PARHandler } from "../src/par.js";
+import type {
+	LexiconPermissionSet,
+	PermissionSetResolver,
+} from "../src/permission-sets.js";
 import { InMemoryOAuthStorage } from "../src/storage.js";
 import type { ClientMetadata } from "../src/storage.js";
 import { generateCodeChallenge, generateCodeVerifier } from "./helpers.js";
@@ -133,6 +137,81 @@ describe("PAR Handler", () => {
 			expect(json.error_description).toMatch(/not registered/i);
 		});
 
+		it("rejects include: for a permission set that fails to resolve", async () => {
+			const resolver: PermissionSetResolver = {
+				async resolve() {
+					return null;
+				},
+			};
+			const handlerWithResolver = new PARHandler(
+				storage,
+				new StubResolver(REGISTERED_REDIRECT),
+				"https://example.com",
+				undefined,
+				resolver,
+			);
+
+			const verifier = generateCodeVerifier();
+			const challenge = await generateCodeChallenge(verifier);
+			const request = createPARRequest({
+				client_id: "did:web:client.example.com",
+				redirect_uri: REGISTERED_REDIRECT,
+				response_type: "code",
+				code_challenge: challenge,
+				code_challenge_method: "S256",
+				state: "random-state",
+				scope: "atproto include:com.example.nonexistent",
+			});
+
+			const response = await handlerWithResolver.handlePushRequest(request);
+			expect(response.status).toBe(400);
+
+			const json = (await response.json()) as {
+				error: string;
+				error_description: string;
+			};
+			expect(json.error).toBe("invalid_scope");
+			expect(json.error_description).toMatch(/com\.example\.nonexistent/);
+		});
+
+		it("accepts include: that the resolver successfully resolves", async () => {
+			const permissionSet: LexiconPermissionSet = {
+				type: "permission-set",
+				title: "Test bundle",
+				detail: "for tests",
+				permissions: [],
+			};
+			const resolver: PermissionSetResolver = {
+				async resolve() {
+					return permissionSet;
+				},
+			};
+			const handlerWithResolver = new PARHandler(
+				storage,
+				new StubResolver(REGISTERED_REDIRECT),
+				"https://example.com",
+				undefined,
+				resolver,
+			);
+
+			const verifier = generateCodeVerifier();
+			const challenge = await generateCodeChallenge(verifier);
+			const request = createPARRequest({
+				client_id: "did:web:client.example.com",
+				redirect_uri: REGISTERED_REDIRECT,
+				response_type: "code",
+				code_challenge: challenge,
+				code_challenge_method: "S256",
+				state: "random-state",
+				scope: "atproto include:com.example.real",
+			});
+
+			const response = await handlerWithResolver.handlePushRequest(request);
+			expect(response.status).toBe(201);
+			const json = (await response.json()) as { request_uri: string };
+			expect(json.request_uri).toMatch(/^urn:ietf:params:oauth:request_uri:/);
+		});
+
 		it("rejects unsupported response_type", async () => {
 			const verifier = generateCodeVerifier();
 			const challenge = await generateCodeChallenge(verifier);