fix(labeler): read Access group claims from the verified custom object

Cloudflare Access places IdP group membership inside the verified
application token's `custom` object (a `groups` claim configured on the
SAML/OIDC integration), never as a top-level `groups` claim. The role
mapper read `payload.groups`, which Access never populates, so the
group-only reviewer/admin allowlist granted no roles to human operators
whose access is conferred by group.

Read the `groups` array from the verified `payload.custom` object,
guarding the `custom` record and the array shape strictly. Restrict group
extraction to human identities: a service token is authorized solely by
common_name and carries no IdP identity, so a `custom.groups` on a
service token must never confer a role (moving the group claim under
`custom` would otherwise open a fail-open path). Email/common_name
resolution is otherwise unchanged.

Author: ascorbic

apps/labeler/src/access-auth.ts

@@ -129,15 +129,26 @@ export async function verifyAccessRequest(
 	}
 
 	const principal = identity.kind === "service" ? identity.commonName : identity.email;
-	const principals = new Set<string>([principal, ...groupPrincipals(payload.groups)]);
+	// Group membership authorizes humans only. A service token carries no IdP
+	// identity — it is authorized solely by common_name — so a `custom.groups`
+	// on a service token must never confer a role.
+	const principals = new Set<string>([
+		principal,
+		...(identity.kind === "human" ? groupPrincipals(payload.custom) : []),
+	]);
 	const roles: OperatorRole[] = [];
 	if (config.admins.some((admin) => principals.has(admin))) roles.push("admin");
 	if (config.reviewers.some((reviewer) => principals.has(reviewer))) roles.push("reviewer");
 
 	return { ...identity, roles };
 }
 
-function groupPrincipals(groups: unknown): readonly string[] {
+// Cloudflare Access surfaces IdP group membership inside the verified token's
+// `custom` object (as a `groups` claim configured on the SAML/OIDC integration),
+// never as a top-level claim. Read the group principals from there.
+function groupPrincipals(custom: unknown): readonly string[] {
+	if (!isRecord(custom)) return [];
+	const groups = custom.groups;
 	if (!Array.isArray(groups)) return [];
 	return groups.filter((entry): entry is string => typeof entry === "string" && entry.length > 0);
 }

apps/labeler/test/access-auth.test.ts

@@ -28,6 +28,7 @@ interface MintOverrides {
 	email?: string;
 	common_name?: string;
 	groups?: unknown;
+	custom?: unknown;
 	issuer?: string;
 	audience?: string;
 	expiresInSeconds?: number;
@@ -102,9 +103,9 @@ describe("verifyAccessRequest", () => {
 		});
 	});
 
-	it("maps roles from a groups claim", async () => {
+	it("maps roles from a groups claim nested under the verified custom object", async () => {
 		const token = await mintToken(
-			{ email: "someone@example.com", groups: ["emdash-labeler-reviewers"] },
+			{ email: "someone@example.com", custom: { groups: ["emdash-labeler-reviewers"] } },
 			signKey,
 		);
 		const identity = await verifyAccessRequest(
@@ -115,6 +116,35 @@ describe("verifyAccessRequest", () => {
 		expect(identity.roles).toEqual(["reviewer"]);
 	});
 
+	it("ignores a top-level groups claim (Access places IdP groups under custom)", async () => {
+		const token = await mintToken(
+			{ email: "someone@example.com", groups: ["emdash-labeler-admins"] },
+			signKey,
+		);
+		const identity = await verifyAccessRequest(
+			requestWith({ "Cf-Access-Jwt-Assertion": token }),
+			baseConfig(),
+			resolver,
+		);
+		expect(identity.roles).toEqual([]);
+	});
+
+	it("maps both admin and reviewer when custom.groups lists both", async () => {
+		const token = await mintToken(
+			{
+				email: "someone@example.com",
+				custom: { groups: ["emdash-labeler-admins", "emdash-labeler-reviewers"] },
+			},
+			signKey,
+		);
+		const identity = await verifyAccessRequest(
+			requestWith({ "Cf-Access-Jwt-Assertion": token }),
+			baseConfig(),
+			resolver,
+		);
+		expect(identity.roles).toEqual(["admin", "reviewer"]);
+	});
+
 	it("grants only reviewer role for a reviewer-only principal", async () => {
 		const token = await mintToken({ email: "reviewer@example.com" }, signKey);
 		const identity = await verifyAccessRequest(
@@ -330,33 +360,39 @@ describe("verifyAccessRequest", () => {
 		).rejects.toMatchObject({ reason: "invalid-token" });
 	});
 
-	it("ignores a groups claim of the wrong shape without crashing", async () => {
-		const stringShape = await mintToken(
-			{ email: "reviewer@example.com", groups: "emdash-labeler-reviewers" },
-			signKey,
-		);
-		const numberArrayShape = await mintToken(
-			{ email: "reviewer@example.com", groups: [1, 2, 3] },
-			signKey,
-		);
+	it("ignores malformed custom / groups shapes without crashing", async () => {
+		// custom itself is not an object; custom.groups is a string not an array;
+		// custom.groups is a non-string array; custom is absent. None should throw
+		// and none should leak a group principal — the reviewer role here comes
+		// only from the email allowlist.
+		const shapes: unknown[] = [
+			"custom-is-a-string",
+			{ groups: "emdash-labeler-reviewers" },
+			{ groups: [1, 2, 3] },
+			{ groups: { nested: true } },
+		];
+		for (const custom of shapes) {
+			const token = await mintToken({ email: "reviewer@example.com", custom }, signKey);
+			const identity = await verifyAccessRequest(
+				requestWith({ "Cf-Access-Jwt-Assertion": token }),
+				baseConfig(),
+				resolver,
+			);
+			expect(identity.roles).toEqual(["reviewer"]);
+		}
 
-		const identityA = await verifyAccessRequest(
-			requestWith({ "Cf-Access-Jwt-Assertion": stringShape }),
-			baseConfig(),
-			resolver,
-		);
-		const identityB = await verifyAccessRequest(
-			requestWith({ "Cf-Access-Jwt-Assertion": numberArrayShape }),
+		const noCustom = await mintToken({ email: "reviewer@example.com" }, signKey);
+		const identity = await verifyAccessRequest(
+			requestWith({ "Cf-Access-Jwt-Assertion": noCustom }),
 			baseConfig(),
 			resolver,
 		);
-		expect(identityA.roles).toEqual(["reviewer"]);
-		expect(identityB.roles).toEqual(["reviewer"]);
+		expect(identity.roles).toEqual(["reviewer"]);
 	});
 
-	it("does not treat an empty-string group entry as a principal", async () => {
+	it("does not treat an empty-string custom group entry as a principal", async () => {
 		const token = await mintToken(
-			{ email: "nobody@example.com", groups: ["", "not-a-configured-group"] },
+			{ email: "nobody@example.com", custom: { groups: ["", "not-a-configured-group"] } },
 			signKey,
 		);
 		// An empty-string allowlist entry can only match if an empty group leaks in as a principal.
@@ -368,6 +404,40 @@ describe("verifyAccessRequest", () => {
 		expect(identity.roles).toEqual([]);
 	});
 
+	it("never lets a service token gain a role from custom.groups (service = common_name only)", async () => {
+		// A service token carries no IdP identity, so a `custom.groups` that
+		// happens to match an allowlist entry must NOT confer a role — otherwise
+		// moving the group claim under `custom` opens a fail-open path.
+		const unauthorized = await mintToken(
+			{ common_name: "unknown-service", custom: { groups: ["emdash-labeler-admins"] } },
+			signKey,
+		);
+		const identityA = await verifyAccessRequest(
+			requestWith({ "Cf-Access-Jwt-Assertion": unauthorized }),
+			baseConfig(),
+			resolver,
+		);
+		expect(identityA.roles).toEqual([]);
+
+		// A service token authorized by common_name keeps exactly that role; a
+		// reviewer group in its custom object adds nothing.
+		const authorized = await mintToken(
+			{ common_name: "ci-automation", custom: { groups: ["emdash-labeler-reviewers"] } },
+			signKey,
+		);
+		const identityB = await verifyAccessRequest(
+			requestWith({ "Cf-Access-Jwt-Assertion": authorized }),
+			baseConfig({ admins: ["ci-automation"] }),
+			resolver,
+		);
+		expect(identityB).toEqual<OperatorIdentity>({
+			kind: "service",
+			commonName: "ci-automation",
+			sub: "user-sub-1",
+			roles: ["admin"],
+		});
+	});
+
 	it("never includes the raw token in a thrown error message", async () => {
 		const token = await mintToken({ email: "admin@example.com" }, otherKey);
 		try {