pkce dev ed (#1694)

* feat: dev branch for dev testing (#1692)

* feat: pkce from a couple days ago

* fix: dev

* fix: better

.github/workflows/server.yml

@@ -176,9 +176,18 @@ jobs:
         id: floating_tags
         run: |
           if [ "$GITHUB_REF" = "refs/heads/main" ]; then
-            echo "value=ghcr.io/zkldi/tachi:main\nghcr.io/zkldi/tachi:latest" >> "$GITHUB_OUTPUT"
+            {
+              echo "value<<TAGS_EOF"
+              echo "ghcr.io/zkldi/tachi:main"
+              echo "ghcr.io/zkldi/tachi:latest"
+              echo "TAGS_EOF"
+            } >> "$GITHUB_OUTPUT"
           else
-            echo "value=ghcr.io/zkldi/tachi:main-dev" >> "$GITHUB_OUTPUT"
+            {
+              echo "value<<TAGS_EOF"
+              echo "ghcr.io/zkldi/tachi:main-dev"
+              echo "TAGS_EOF"
+            } >> "$GITHUB_OUTPUT"
           fi
 
       - name: Build and push unified server image

db/migrations/20260619120000_oauth2_pkce.sql

@@ -0,0 +1,6 @@
+-- Add PKCE (RFC 7636) support to OAuth2 authorization codes.
+-- code_challenge: BASE64URL(SHA256(code_verifier)), stored at code creation time.
+-- code_challenge_method: only "S256" is supported; NULL means client_secret flow.
+ALTER TABLE "priv_oauth2_auth_token"
+    ADD COLUMN code_challenge TEXT,
+    ADD COLUMN code_challenge_method TEXT;

typescript/client/src/app/pages/OAuthRequestAuthPage.tsx

@@ -65,6 +65,8 @@ function OAuthRequestAuthMain({
 	const params = useQueryString();
 
 	const context = params.get("context");
+	const codeChallenge = params.get("code_challenge");
+	const codeChallengeMethod = params.get("code_challenge_method");
 
 	return (
 		<div className="row">
@@ -88,6 +90,16 @@ function OAuthRequestAuthMain({
 					<button
 						className="btn btn-success"
 						onClick={async () => {
+							const body: Record<string, string> = {};
+
+							if (codeChallenge) {
+								body.code_challenge = codeChallenge;
+							}
+
+							if (codeChallengeMethod) {
+								body.code_challenge_method = codeChallengeMethod;
+							}
+
 							const tokenRes = await APIFetchV1<{
 								code: string;
 								createdOn: number;
@@ -96,6 +108,8 @@ function OAuthRequestAuthMain({
 								"/oauth/create-code",
 								{
 									method: "POST",
+									headers: { "Content-Type": "application/json" },
+									body: JSON.stringify(body),
 								},
 								false,
 								true,

typescript/db/src/generated/public/PrivOauth2AuthToken.ts

@@ -14,6 +14,10 @@ export default interface PrivOauth2AuthTokenTable {
   created_on: ColumnType<string, string, string>;
 
   user_id: ColumnType<account_id, account_id, account_id>;
+
+  code_challenge: ColumnType<string | null, string | null, string | null>;
+
+  code_challenge_method: ColumnType<string | null, string | null, string | null>;
 }
 
 export type PrivOauth2AuthToken = Selectable<PrivOauth2AuthTokenTable>;

typescript/server/src/actions/create-oauth2-auth-code.ts

@@ -1,22 +1,35 @@
 import { MakeAction } from "#lib/actions/actions";
 import DB from "#services/pg/db";
 import { Random20Hex } from "#utils/misc";
+import { ExpectedErr } from "bliss";
 
-export const ACTION_CreateOAuth2AuthCode = MakeAction("CREATE_OAUTH2_AUTH_CODE", async (taker) => {
-	const code = Random20Hex();
-	const createdOn = Date.now();
+export const ACTION_CreateOAuth2AuthCode = MakeAction(
+	"CREATE_OAUTH2_AUTH_CODE",
+	async (taker, input) => {
+		if ((input.codeChallenge === undefined) !== (input.codeChallengeMethod === undefined)) {
+			throw new ExpectedErr(
+				400,
+				"Both code_challenge and code_challenge_method must be provided together.",
+			);
+		}
 
-	await DB.insertInto("priv_oauth2_auth_token")
-		.values({
-			token: code,
-			user_id: taker.acct.id,
-			created_on: new Date(createdOn).toISOString(),
-		})
-		.execute();
+		const code = Random20Hex();
+		const createdOn = Date.now();
 
-	return {
-		code,
-		userID: taker.acct.id,
-		createdOn,
-	};
-});
+		await DB.insertInto("priv_oauth2_auth_token")
+			.values({
+				token: code,
+				user_id: taker.acct.id,
+				created_on: new Date(createdOn).toISOString(),
+				code_challenge: input.codeChallenge ?? null,
+				code_challenge_method: input.codeChallengeMethod ?? null,
+			})
+			.execute();
+
+		return {
+			code,
+			userID: taker.acct.id,
+			createdOn,
+		};
+	},
+);

typescript/server/src/anon-actions/oauth-token-exchange.ts

@@ -20,28 +20,39 @@ const SELECT_OAUTH_CLIENT = [
 	"priv_api_client.pm_manage_challenges",
 ] as const;
 
+/**
+ * Compute BASE64URL(SHA256(verifier)) and compare it in constant time to the
+ * stored code_challenge. Returns true if they match.
+ */
+function verifyPkceChallenge(codeVerifier: string, codeChallenge: string): boolean {
+	const digest = crypto.createHash("sha256").update(codeVerifier, "ascii").digest();
+	const computed = digest
+		.toString("base64")
+		.replace(/\+/gu, "-")
+		.replace(/\//gu, "_")
+		.replace(/=/gu, "");
+
+	const expectedBuf = Buffer.from(codeChallenge, "utf8");
+	const receivedBuf = Buffer.from(computed, "utf8");
+
+	return (
+		expectedBuf.length === receivedBuf.length &&
+		crypto.timingSafeEqual(expectedBuf, receivedBuf)
+	);
+}
+
 export const ANON_ACTION_OAuthTokenExchange = MakeAnonAction(
 	"OAUTH_TOKEN_EXCHANGE",
 	async (_taker, input) => {
 		const client = await DB.selectFrom("priv_api_client")
 			.select(SELECT_OAUTH_CLIENT)
-			.where("client_id", "=", input.client_id)
+			.where("priv_api_client.client_id", "=", input.client_id)
 			.executeTakeFirst();
 
 		if (!client) {
 			throw new ExpectedErr(404, `This client does not exist.`);
 		}
 
-		const expectedBuf = Buffer.from(client.client_secret, "utf8");
-		const receivedBuf = Buffer.from(String(input.client_secret), "utf8");
-
-		if (
-			expectedBuf.length !== receivedBuf.length ||
-			!crypto.timingSafeEqual(expectedBuf, receivedBuf)
-		) {
-			throw new ExpectedErr(403, `Invalid secret.`);
-		}
-
 		if (client.redirect_uri !== input.redirect_uri) {
 			throw new ExpectedErr(
 				400,
@@ -52,39 +63,77 @@ export const ANON_ACTION_OAuthTokenExchange = MakeAnonAction(
 		const apiToken = Random20Hex();
 		const identifier = `${client.name} Token`;
 
-		await DB.transaction().execute(async (txn) => {
-			const deleted = await txn
-				.deleteFrom("priv_oauth2_auth_token")
-				.where("token", "=", input.code)
-				.returning("user_id")
-				.executeTakeFirst();
+		// Consume the authorization code up front, in its own committed
+		// statement, *before* verifying the verifier/secret. A failed
+		// verification below must still burn the code; otherwise a rolled-back
+		// deletion would leave the code redeemable and turn this endpoint into
+		// an unbounded verification oracle (unlimited guessing against a
+		// single-use, non-expiring code).
+		const deleted = await DB.deleteFrom("priv_oauth2_auth_token")
+			.where("priv_oauth2_auth_token.token", "=", input.code)
+			.returning([
+				"priv_oauth2_auth_token.user_id",
+				"priv_oauth2_auth_token.code_challenge",
+				"priv_oauth2_auth_token.code_challenge_method",
+			])
+			.executeTakeFirst();
 
-			if (!deleted) {
-				throw new ExpectedErr(404, `This code does not exist.`);
+		if (!deleted) {
+			throw new ExpectedErr(404, `This code does not exist.`);
+		}
+
+		if (deleted.code_challenge !== null) {
+			// PKCE flow: verify code_verifier, client_secret is not required.
+			if (!input.code_verifier) {
+				throw new ExpectedErr(
+					400,
+					"This authorization code requires PKCE. Provide code_verifier.",
+				);
 			}
 
-			await txn
-				.insertInto("priv_api_token")
-				.values({
-					token: apiToken,
-					user_id: deleted.user_id,
-					identifier,
-					from_oauth2_client: client.client_id,
-					pm_customise_profile: client.pm_customise_profile,
-					pm_customise_score: client.pm_customise_score,
-					pm_customise_session: client.pm_customise_session,
-					pm_delete_score: client.pm_delete_score,
-					pm_manage_rivals: client.pm_manage_rivals,
-					pm_manage_targets: client.pm_manage_targets,
-					pm_submit_score: client.pm_submit_score,
-					pm_manage_challenges: client.pm_manage_challenges,
-				})
-				.execute();
-		});
+			if (!verifyPkceChallenge(input.code_verifier, deleted.code_challenge)) {
+				throw new ExpectedErr(403, "Invalid code_verifier.");
+			}
+		} else {
+			// Legacy flow: client_secret is required.
+			if (!input.client_secret) {
+				throw new ExpectedErr(
+					400,
+					"client_secret is required for non-PKCE authorization codes.",
+				);
+			}
+
+			const expectedBuf = Buffer.from(client.client_secret, "utf8");
+			const receivedBuf = Buffer.from(String(input.client_secret), "utf8");
+
+			if (
+				expectedBuf.length !== receivedBuf.length ||
+				!crypto.timingSafeEqual(expectedBuf, receivedBuf)
+			) {
+				throw new ExpectedErr(403, `Invalid secret.`);
+			}
+		}
+
+		await DB.insertInto("priv_api_token")
+			.values({
+				token: apiToken,
+				user_id: deleted.user_id,
+				identifier,
+				from_oauth2_client: client.client_id,
+				pm_customise_profile: client.pm_customise_profile,
+				pm_customise_score: client.pm_customise_score,
+				pm_customise_session: client.pm_customise_session,
+				pm_delete_score: client.pm_delete_score,
+				pm_manage_rivals: client.pm_manage_rivals,
+				pm_manage_targets: client.pm_manage_targets,
+				pm_submit_score: client.pm_submit_score,
+				pm_manage_challenges: client.pm_manage_challenges,
+			})
+			.execute();
 
 		const row = await DB.selectFrom("priv_api_token")
 			.select(SELECT_API_TOKEN)
-			.where("token", "=", apiToken)
+			.where("priv_api_token.token", "=", apiToken)
 			.executeTakeFirstOrThrow();
 
 		const doc = ToAPITokenDocument(row);

typescript/server/src/lib/actions/actions.ts

@@ -282,7 +282,17 @@ export const ActionSignatures = {
 		output: z.object({}),
 	},
 	CREATE_OAUTH2_AUTH_CODE: {
-		input: z.object({}),
+		input: z.object({
+			// BASE64URL(SHA256(verifier)) is always exactly 43 unpadded base64url chars (RFC 7636 §4.2).
+			codeChallenge: z
+				.string()
+				.regex(
+					/^[A-Za-z0-9_-]{43}$/u,
+					"code_challenge must be a 43-character base64url SHA-256 digest.",
+				)
+				.optional(),
+			codeChallengeMethod: z.literal("S256").optional(),
+		}),
 		output: z.object({
 			code: z.string(),
 			userID: z.number().int(),
@@ -499,10 +509,18 @@ export const AnonActionSignatures = {
 	OAUTH_TOKEN_EXCHANGE: {
 		input: z.object({
 			client_id: z.string(),
-			client_secret: z.string(),
+			client_secret: z.string().optional(),
 			grant_type: z.literal("authorization_code"),
 			redirect_uri: z.string(),
 			code: z.string(),
+			// RFC 7636 §4.1: code_verifier is 43-128 chars from the unreserved set.
+			code_verifier: z
+				.string()
+				.regex(
+					/^[A-Za-z0-9._~-]{43,128}$/u,
+					"code_verifier must be 43-128 unreserved characters.",
+				)
+				.optional(),
 		}),
 		output: z.object({
 			userID: z.number().int(),

typescript/server/src/server/router/api/v1/oauth/router.test.ts

@@ -2,6 +2,7 @@ import { seedApiClient } from "#actions/test-utils/api-tokens";
 import DB from "#services/pg/db";
 import mockApi, { CloseServerConnection } from "#test-utils/mock-api";
 import { seedUser } from "#test-utils/pg-fixtures";
+import crypto from "node:crypto";
 import { afterAll, beforeEach, describe, expect, it } from "vitest";
 
 async function loginAs(username: string, password = "password123") {
@@ -14,6 +15,17 @@ async function loginAs(username: string, password = "password123") {
 	return res.headers["set-cookie"] as unknown as string[];
 }
 
+function makeS256Challenge(verifier: string): string {
+	return crypto
+		.createHash("sha256")
+		.update(verifier, "ascii")
+		.digest()
+		.toString("base64")
+		.replace(/\+/gu, "-")
+		.replace(/\//gu, "_")
+		.replace(/=/gu, "");
+}
+
 afterAll(() => CloseServerConnection());
 
 describe("POST /api/v1/oauth/token", () => {
@@ -44,6 +56,8 @@ describe("POST /api/v1/oauth/token", () => {
 				token: authCode,
 				user_id: userId,
 				created_on: new Date().toISOString(),
+				code_challenge: null,
+				code_challenge_method: null,
 			})
 			.execute();
 	});
@@ -129,6 +143,147 @@ describe("POST /api/v1/oauth/token", () => {
 
 		expect(res.status).toBe(400);
 	});
+
+	it("returns 400 when client_secret is omitted for a non-PKCE code", async () => {
+		const res = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+		});
+
+		expect(res.status).toBe(400);
+	});
+});
+
+describe("POST /api/v1/oauth/token (PKCE)", () => {
+	let userId: number;
+	const clientId = "PKCE_CLIENT_ID";
+	const clientSecret = "PKCE_CLIENT_SECRET";
+	const redirectUri = "https://example.com/pkce-callback";
+	const codeVerifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk";
+	// Correctly formatted (43-char unreserved) but not the real verifier.
+	const wrongCodeVerifier = "wrongVerifierValueButValidLength0123456789a";
+	const authCode = "PKCE_AUTH_CODE";
+
+	let codeChallenge: string;
+
+	beforeEach(async () => {
+		codeChallenge = makeS256Challenge(codeVerifier);
+
+		({ id: userId } = await seedUser({
+			username: "pkce_oauth_user",
+			withCredential: true,
+			withSettings: true,
+		}));
+
+		await seedApiClient({
+			clientId,
+			authorId: userId,
+			name: "PKCE_Service",
+			clientSecret,
+			customiseProfile: true,
+			redirectUri,
+		});
+
+		await DB.insertInto("priv_oauth2_auth_token")
+			.values({
+				token: authCode,
+				user_id: userId,
+				created_on: new Date().toISOString(),
+				code_challenge: codeChallenge,
+				code_challenge_method: "S256",
+			})
+			.execute();
+	});
+
+	it("grants a token with a valid code_verifier, no client_secret needed", async () => {
+		const res = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+			code_verifier: codeVerifier,
+		});
+
+		expect(res.status).toBe(200);
+		expect(res.body.success).toBe(true);
+		expect(res.body.body.userID).toBe(userId);
+		expect(res.body.body.fromAPIClient).toBe(clientId);
+
+		const codeRow = await DB.selectFrom("priv_oauth2_auth_token")
+			.selectAll()
+			.where("token", "=", authCode)
+			.executeTakeFirst();
+
+		expect(codeRow).toBeUndefined();
+	});
+
+	it("returns 403 for an incorrect code_verifier", async () => {
+		const res = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+			code_verifier: wrongCodeVerifier,
+		});
+
+		expect(res.status).toBe(403);
+	});
+
+	it("consumes the code on a failed code_verifier so it cannot be retried", async () => {
+		const firstRes = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+			code_verifier: wrongCodeVerifier,
+		});
+
+		expect(firstRes.status).toBe(403);
+
+		const codeRow = await DB.selectFrom("priv_oauth2_auth_token")
+			.selectAll()
+			.where("token", "=", authCode)
+			.executeTakeFirst();
+
+		expect(codeRow).toBeUndefined();
+
+		const retryRes = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+			code_verifier: codeVerifier,
+		});
+
+		expect(retryRes.status).toBe(404);
+	});
+
+	it("returns 400 when code_verifier is missing for a PKCE code", async () => {
+		const res = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			client_secret: clientSecret,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+		});
+
+		expect(res.status).toBe(400);
+	});
+
+	it("also accepts a valid code_verifier when client_secret is additionally provided", async () => {
+		const res = await mockApi.post(`/api/v1/oauth/token`).send({
+			client_id: clientId,
+			client_secret: clientSecret,
+			grant_type: "authorization_code",
+			redirect_uri: redirectUri,
+			code: authCode,
+			code_verifier: codeVerifier,
+		});
+
+		expect(res.status).toBe(200);
+	});
 });
 
 describe("POST /api/v1/oauth/create-code", () => {
@@ -161,5 +316,53 @@ describe("POST /api/v1/oauth/create-code", () => {
 
 		expect(row).toBeDefined();
 		expect(Number(row?.user_id)).toBe(uid);
+		expect(row?.code_challenge).toBeNull();
+		expect(row?.code_challenge_method).toBeNull();
+	});
+
+	it("stores code_challenge and code_challenge_method for PKCE codes", async () => {
+		const { id: uid } = await seedUser({
+			username: "pkce_code_user",
+			withCredential: true,
+			withSettings: true,
+		});
+		const cookie = await loginAs("pkce_code_user");
+
+		const verifier = "test-verifier-value-for-pkce-flow";
+		const challenge = makeS256Challenge(verifier);
+
+		const res = await mockApi
+			.post("/api/v1/oauth/create-code")
+			.set("Cookie", cookie)
+			.send({ code_challenge: challenge, code_challenge_method: "S256" });
+
+		expect(res.status).toBe(200);
+		expect(res.body.body.code).toBeTruthy();
+
+		const row = await DB.selectFrom("priv_oauth2_auth_token")
+			.selectAll()
+			.where("token", "=", res.body.body.code)
+			.executeTakeFirst();
+
+		expect(row).toBeDefined();
+		expect(Number(row?.user_id)).toBe(uid);
+		expect(row?.code_challenge).toBe(challenge);
+		expect(row?.code_challenge_method).toBe("S256");
+	});
+
+	it("rejects a malformed (wrong-length) code_challenge", async () => {
+		await seedUser({
+			username: "bad_pkce_code_user",
+			withCredential: true,
+			withSettings: true,
+		});
+		const cookie = await loginAs("bad_pkce_code_user");
+
+		const res = await mockApi
+			.post("/api/v1/oauth/create-code")
+			.set("Cookie", cookie)
+			.send({ code_challenge: "too-short", code_challenge_method: "S256" });
+
+		expect(res.status).toBe(400);
 	});
 });

typescript/server/src/server/router/api/v1/oauth/router.ts

@@ -13,10 +13,11 @@ import { API_V1_ROUTER } from "../_singleton";
  * part of the spec or not, but it probably is.
  *
  * @param client_id - The id for the client requesting a token.
- * @param client_secret - The secret for the client.
+ * @param client_secret - The secret for the client. Required if not doing pkce
  * @param grant_type - Only exactly "authorization_code" is supported at the moment.
  * @param redirect_uri - Must be the exact redirectUri registered with this client.
  * @param code - The code to convert into an API token.
+ * @param code_verifier - PKCE verifier (RFC 7636). Required when the code was created with a code_challenge.
  *
  * @name POST /api/v1/oauth/token
  */
@@ -29,6 +30,7 @@ API_V1_ROUTER.add("POST /oauth/token", async ({ input, req }) => {
 			code: input.code,
 			grant_type: input.grant_type,
 			redirect_uri: input.redirect_uri,
+			code_verifier: input.code_verifier,
 		},
 	);
 
@@ -38,9 +40,12 @@ API_V1_ROUTER.add("POST /oauth/token", async ({ input, req }) => {
 /**
  * Creates an authorization code for this user (inferred from session).
  *
+ * @param code_challenge - PKCE code challenge (BASE64URL(SHA256(code_verifier))). Optional.
+ * @param code_challenge_method - Must be "S256" if provided. Optional.
+ *
  * @name POST /api/v1/oauth/create-code
  */
-API_V1_ROUTER.add("POST /oauth/create-code", async ({ req }) => {
+API_V1_ROUTER.add("POST /oauth/create-code", async ({ input, req }) => {
 	if (!req.session.tachi?.user) {
 		throw new ExpectedErr(401, "You are not authenticated.");
 	}
@@ -48,7 +53,10 @@ API_V1_ROUTER.add("POST /oauth/create-code", async ({ req }) => {
 	const user = req.session.tachi.user;
 	const taker = { acct: { id: user.id, username: user.username }, ip: req.ip };
 
-	const doc = await ACTION_CreateOAuth2AuthCode(taker, {});
+	const doc = await ACTION_CreateOAuth2AuthCode(taker, {
+		codeChallenge: input.code_challenge,
+		codeChallengeMethod: input.code_challenge_method,
+	});
 
 	return success("Successfully created code.", doc);
 });

typescript/server/src/server/router/api/v1/spec.ts

@@ -1333,13 +1333,22 @@ export const API_V1_SPEC = {
 	// ────────────────────────────────────────────────
 
 	"POST /oauth/token": {
-		description: "Exchange an OAuth2 authorization code for an API token.",
+		description:
+			"Exchange an OAuth2 authorization code for an API token. Supports both the standard client_secret flow and the PKCE flow (RFC 7636). For PKCE, omit client_secret and provide code_verifier instead.",
 		input: z.object({
 			client_id: z.string(),
-			client_secret: z.string(),
+			client_secret: z.string().optional(),
 			grant_type: z.literal("authorization_code"),
 			redirect_uri: z.string(),
 			code: z.string(),
+			// RFC 7636 §4.1: code_verifier is 43-128 chars from the unreserved set.
+			code_verifier: z
+				.string()
+				.regex(
+					/^[A-Za-z0-9._~-]{43,128}$/u,
+					"code_verifier must be 43-128 unreserved characters.",
+				)
+				.optional(),
 		}),
 		output: z.strictObject({
 			userID: z.number(),
@@ -1351,8 +1360,19 @@ export const API_V1_SPEC = {
 	},
 
 	"POST /oauth/create-code": {
-		description: "Create an OAuth2 authorization code (session auth required).",
-		input: z.object({}),
+		description:
+			"Create an OAuth2 authorization code (session auth required). Pass code_challenge and code_challenge_method to use the PKCE flow (RFC 7636).",
+		input: z.object({
+			// BASE64URL(SHA256(verifier)) is always exactly 43 unpadded base64url chars (RFC 7636 §4.2).
+			code_challenge: z
+				.string()
+				.regex(
+					/^[A-Za-z0-9_-]{43}$/u,
+					"code_challenge must be a 43-character base64url SHA-256 digest.",
+				)
+				.optional(),
+			code_challenge_method: z.literal("S256").optional(),
+		}),
 		output: z.strictObject({
 			code: z.string(),
 			userID: z.number(),