fix(oauth-provider): PAR rejects unregistered redirect_uri

.changeset/par-validate-redirect-uri.md

@@ -0,0 +1,5 @@
+---
+"@getcirrus/oauth-provider": patch
+---
+
+PAR (`/oauth/par`) now validates `redirect_uri` against the client's registered redirect_uris at push time. Previously the check only ran at the authorize step, which let a malicious caller obtain a `request_uri` for an unregistered redirect even though the subsequent authorize would have rejected it. Reject early per RFC 6749 §3.1.2.4.

packages/oauth-provider/src/par.ts

@@ -4,6 +4,7 @@
  */
 
 import type { OAuthParResponse } from "@atproto/oauth-types";
+import type { ClientResolver } from "./client-resolver.js";
 import type { OAuthStorage, PARData } from "./storage.js";
 import { randomString } from "./encoding.js";
 import { parseRequestBody } from "./provider.js";
@@ -53,13 +54,17 @@ const REQUIRED_PARAMS = [
  */
 export class PARHandler {
 	private storage: OAuthStorage;
+	private clientResolver: ClientResolver;
 	private issuer: string;
 	private expiresIn: number;
 	private allowIncludes: boolean;
 
 	/**
 	 * Create a PAR handler
 	 * @param storage OAuth storage implementation
+	 * @param clientResolver Client metadata resolver. Used to validate
+	 *   redirect_uri against the registered list at push time per
+	 *   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
@@ -69,11 +74,13 @@ export class PARHandler {
 	 */
 	constructor(
 		storage: OAuthStorage,
+		clientResolver: ClientResolver,
 		issuer: string,
 		expiresIn: number = DEFAULT_EXPIRES_IN,
 		allowIncludes: boolean = false,
 	) {
 		this.storage = storage;
+		this.clientResolver = clientResolver;
 		this.issuer = issuer;
 		this.expiresIn = expiresIn;
 		this.allowIncludes = allowIncludes;
@@ -147,6 +154,28 @@ export class PARHandler {
 			return this.errorResponse("invalid_request", "Invalid redirect_uri", 400);
 		}
 
+		// RFC 6749 §3.1.2.4: the AS MUST reject any redirect_uri that wasn't
+		// registered in the client metadata. Authorize re-validates, but PAR
+		// must also reject here — otherwise a request_uri gets issued for a
+		// redirect the client never registered.
+		let client;
+		try {
+			client = await this.clientResolver.resolveClient(clientId);
+		} catch (e) {
+			return this.errorResponse(
+				"invalid_client",
+				`Failed to resolve client: ${e instanceof Error ? e.message : String(e)}`,
+				400,
+			);
+		}
+		if (!client.redirectUris.includes(params.redirect_uri!)) {
+			return this.errorResponse(
+				"invalid_request",
+				"redirect_uri is not registered for this client",
+				400,
+			);
+		}
+
 		const scope = params.scope ?? ATPROTO_SCOPE;
 		params.scope = scope;
 		try {

packages/oauth-provider/src/provider.ts

@@ -215,14 +215,15 @@ export class ATProtoOAuthProvider {
 		this.issuer = config.issuer;
 		this.dpopRequired = config.dpopRequired ?? true;
 		this.enablePAR = config.enablePAR ?? true;
+		this.clientResolver =
+			config.clientResolver ?? new ClientResolver({ storage: config.storage });
 		this.parHandler = new PARHandler(
 			config.storage,
+			this.clientResolver,
 			config.issuer,
 			undefined,
 			!!config.permissionSetResolver,
 		);
-		this.clientResolver =
-			config.clientResolver ?? new ClientResolver({ storage: config.storage });
 		this.verifyUser = config.verifyUser;
 		this.getCurrentUser = config.getCurrentUser;
 		this.getPasskeyOptions = config.getPasskeyOptions;

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

@@ -1,15 +1,44 @@
 import { describe, it, expect, beforeEach } from "vitest";
+import { ClientResolver } from "../src/client-resolver.js";
 import { PARHandler } from "../src/par.js";
 import { InMemoryOAuthStorage } from "../src/storage.js";
+import type { ClientMetadata } from "../src/storage.js";
 import { generateCodeChallenge, generateCodeVerifier } from "./helpers.js";
 
+/**
+ * Stub resolver for PAR tests: returns metadata with a fixed registered
+ * redirect_uri so PAR's RFC 6749 §3.1.2.4 check passes for the canonical
+ * fixture. Tests that probe rejection pass a different redirect_uri.
+ */
+class StubResolver extends ClientResolver {
+	private fixed: ClientMetadata;
+	constructor(redirectUri: string) {
+		super();
+		this.fixed = {
+			clientId: "did:web:client.example.com",
+			clientName: "Test Client",
+			redirectUris: [redirectUri],
+			tokenEndpointAuthMethod: "none",
+			cachedAt: Date.now(),
+		};
+	}
+	override async resolveClient(): Promise<ClientMetadata> {
+		return this.fixed;
+	}
+}
+
 describe("PAR Handler", () => {
 	let storage: InMemoryOAuthStorage;
 	let handler: PARHandler;
+	const REGISTERED_REDIRECT = "https://client.example.com/callback";
 
 	beforeEach(() => {
 		storage = new InMemoryOAuthStorage();
-		handler = new PARHandler(storage, "https://example.com");
+		handler = new PARHandler(
+			storage,
+			new StubResolver(REGISTERED_REDIRECT),
+			"https://example.com",
+		);
 	});
 
 	function createPARRequest(params: Record<string, string>): Request {
@@ -79,6 +108,31 @@ describe("PAR Handler", () => {
 			expect(json.error).toBe("invalid_request");
 		});
 
+		it("rejects redirect_uri not registered in client metadata (RFC 6749 §3.1.2.4)", async () => {
+			const verifier = generateCodeVerifier();
+			const challenge = await generateCodeChallenge(verifier);
+
+			const request = createPARRequest({
+				client_id: "did:web:client.example.com",
+				redirect_uri: "https://attacker.invalid/steal-code",
+				response_type: "code",
+				code_challenge: challenge,
+				code_challenge_method: "S256",
+				state: "random-state",
+				scope: "atproto",
+			});
+
+			const response = await handler.handlePushRequest(request);
+			expect(response.status).toBe(400);
+
+			const json = (await response.json()) as {
+				error: string;
+				error_description: string;
+			};
+			expect(json.error).toBe("invalid_request");
+			expect(json.error_description).toMatch(/not registered/i);
+		});
+
 		it("rejects unsupported response_type", async () => {
 			const verifier = generateCodeVerifier();
 			const challenge = await generateCodeChallenge(verifier);