From 53d22a3b70023e9187d9a3d1b7af208ac508d9f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Malfait?= <felix.malfait@gmail.com>
Date: Sat, 18 Apr 2026 21:17:22 +0200
Subject: [PATCH] fix(server): require PKCE code_challenge for public OAuth
 clients (#19840)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

OAuth 2.1 and the MCP authorization spec mandate PKCE (S256) for public
clients — clients registered with \`token_endpoint_auth_method=none\`
(no client secret). We advertise \`code_challenge_methods_supported:
[\"S256\"]\` in \`/.well-known/oauth-authorization-server\` but our
\`/authorize\` flow accepted requests from public clients without
\`code_challenge\`.

## Why this was a soft failure today

\`oauth.service.ts:178\` already rejects token exchange when a client
presents neither \`client_secret\` nor \`code_verifier\`:

\`\`\`ts
if (!clientSecret && !storedCodeChallenge) {
return this.errorResponse('invalid_request', 'Either client_secret or
code_verifier (PKCE) is required');
}
\`\`\`

So a public client attempting to bypass PKCE would **eventually** fail —
but only after:
1. Getting a valid authorization code issued at \`/authorize\`
2. Round-tripping the user through consent
3. Trying to exchange the code at \`/token\` and finally getting
rejected

That's a wasted user interaction and a fuzzy spec boundary. This PR
rejects at \`/authorize\` instead, matching the spec's \"MUST require
PKCE for public clients\" expectation.

## Fix

Single check in \`AuthService.generateAuthorizationCode\`:

\`\`\`ts
const isPublicClient = !applicationRegistration.oAuthClientSecretHash;

if (isPublicClient && !codeChallenge) {
  throw new AuthException(
\`code_challenge is required for public clients (PKCE S256, per OAuth
2.1)\`,
    AuthExceptionCode.FORBIDDEN_EXCEPTION,
  );
}
\`\`\`

### Why \`!oAuthClientSecretHash\` is the right \"public\" predicate

- Dynamic registration (\`POST /oauth/register\`) hardcodes
\`oAuthClientSecretHash: null\` and rejects any
\`token_endpoint_auth_method != \"none\"\`
(oauth-registration.controller.ts:120-130).
- Confidential clients registered via the workspace settings UI have a
non-null bcrypt hash.
- The same field is already used as the public/confidential gate in
\`validateClient\` and \`validateClientSecret\`.

## Scope

- ✅ Dynamic-registration clients (Claude, other MCP connectors) — MUST
now supply code_challenge. They already do; no behavior change for
conformant clients.
- ✅ The seeded twenty-cli registration — public client, already uses
PKCE. No change.
- ➖ Confidential clients (workspace-admin-registered OAuth apps with a
client_secret) — unaffected, they authenticate at the token endpoint.

## Related

- #19836 — CORS exposes \`WWW-Authenticate\` / \`MCP-Protocol-Version\`
- #19838 — RFC 9728 PRM split + RFC 9207 iss param + \`scope\` in
WWW-Authenticate challenge

## Test plan

- [x] \`tsc --noEmit\` clean on modified file (pre-existing
\`twenty-shared\` dist errors unrelated)
- [ ] Integration-level smoke test after deploy:
  \`\`\`bash
  # Register a dynamic client (public)
  CLIENT_ID=$(curl -s -X POST -H 'Content-Type: application/json' \\
-d
'{\"client_name\":\"pkce-test\",\"redirect_uris\":[\"http://localhost/cb\"]}'
\\
    https://<host>/oauth/register | jq -r .client_id)

# Without code_challenge → should now 4xx at /authorize (cannot easily
test outside the React UI,
  # but the GraphQL authorizeApp mutation will throw AuthException)
  \`\`\`
- [ ] Claude MCP connector still completes OAuth end-to-end (it always
sends code_challenge, so no-op)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../core-modules/auth/services/auth.service.ts      | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts
index faf32ee250..53f0c1db45 100644
--- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts
+++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts
@@ -524,6 +524,19 @@ export class AuthService {
       );
     }
 
+    // OAuth 2.1 / MCP auth spec: PKCE is mandatory for public clients
+    // (clients registered with token_endpoint_auth_method=none, i.e. no
+    // client secret hash). Confidential clients are authenticated at the
+    // token endpoint instead.
+    const isPublicClient = !applicationRegistration.oAuthClientSecretHash;
+
+    if (isPublicClient && !codeChallenge) {
+      throw new AuthException(
+        `code_challenge is required for public clients (PKCE S256, per OAuth 2.1)`,
+        AuthExceptionCode.FORBIDDEN_EXCEPTION,
+      );
+    }
+
     // RFC 8252 §7.3: Native apps using loopback redirect URIs may use any port.
     // When a registration has no explicit redirect URIs (e.g. the seeded CLI registration),
     // allow any loopback redirect URI.