Files
twenty/packages
Félix Malfait 53d22a3b70 fix(server): require PKCE code_challenge for public OAuth clients (#19840)
## 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>
2026-04-18 21:17:22 +02:00
..
2026-04-16 09:47:44 +00:00