fix(jwt-auth.guard): refuse and clear tokenPair when proxy identity differs#8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens SSO authentication by detecting stale tokenPair sessions when the upstream oauth2-proxy identity no longer matches the JWT user, and clearing the cookie to force a clean re-auth flow.
Changes:
- Added SSO-only identity consistency check in
JwtAuthGuardcomparingX-Auth-Request-Email(normalized) to the JWT user email; on mismatch, clearstokenPairand refuses the request. - Added unit tests covering match/mismatch, normalization, bare-username synthesis via
DEFAULT_EMAIL_DOMAIN, non-SSO modes, API-key tokens, and missingclearCookie.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/twenty-server/src/engine/guards/jwt-auth.guard.ts | Adds SSO stale-session detection and best-effort tokenPair cookie clearing on identity mismatch. |
| packages/twenty-server/src/engine/guards/tests/jwt-auth.guard.spec.ts | Introduces a new spec suite validating the new guard behavior across key scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private matchesProxyIdentity(request: any, jwtEmail: string): boolean { | ||
| const rawHeader = this.firstHeaderValue( | ||
| request.headers?.['x-auth-request-email'], | ||
| ); | ||
|
|
There was a problem hiding this comment.
Addressed in commit 4a3acb157d — matchesProxyIdentity now calls a new resolveProxyIdentity helper that prefers x-auth-request-email and falls back to x-auth-request-user, mirroring SsoProxyLoginController.resolveEmail(). Tests at jwt-auth.guard.spec.ts:143 (falls back to X-Auth-Request-User) and :293 (prefers X-Auth-Request-Email over X-Auth-Request-User when both are present) pin the behaviour.
| describe('JwtAuthGuard', () => { | ||
| it('returns true when the proxy header matches the JWT user email', async () => { | ||
| const { guard } = buildGuard(); | ||
| const { context } = buildContext({ | ||
| headers: { 'x-auth-request-email': 'alice@example.com' }, | ||
| }); | ||
|
|
||
| expect(await guard.canActivate(context)).toBe(true); | ||
| }); | ||
|
|
||
| it('returns true when no proxy header is present (e.g. internal calls)', async () => { | ||
| const { guard } = buildGuard(); | ||
| const { context, clearCookie } = buildContext(); | ||
|
|
||
| expect(await guard.canActivate(context)).toBe(true); | ||
| expect(clearCookie).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('refuses and clears tokenPair when proxy email differs from JWT user', async () => { |
There was a problem hiding this comment.
Addressed — fallback behaviour is now covered by:
falls back to X-Auth-Request-User when X-Auth-Request-Email is absent(line 143)prefers X-Auth-Request-Email over X-Auth-Request-User when both are present(line 293)synthesises bare X-Auth-Request-User against DEFAULT_EMAIL_DOMAIN before comparing(line 364)refuses and clears tokenPair when bare proxy identity arrives without DEFAULT_EMAIL_DOMAIN configured(line 392)
…iffers
Closes the stale-session-on-user-switch class of bug in Twenty. In SSO
mode, JwtAuthGuard previously validated the Bearer JWT from the
tokenPair cookie without ever consulting the upstream identity that
oauth2-proxy was asserting. Once Twenty issued its tokenPair cookie via
/auth/sso/proxy-login, the cookie was the sole source of identity on
every subsequent request.
After a portal "Logout all" + login as a different user — which clears
the shared _oauth2_proxy cookie and Cognito SSO session but NOT
Twenty's tokenPair cookie on its own subdomain — refreshing the Twenty
tab kept serving the previous user from the stale Bearer.
Implements proxy-auth-middleware Rule 2 ("Identity mismatch SHALL flush
the existing session immediately") per the cross-app contract in
awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md.
Behaviour:
- AUTH_TYPE != SSO: bypass entirely (Twenty's native auth path).
- No `data.user` on the auth context (API key, application token):
bypass (programmatic identity, no upstream user to compare).
- SSO + user present + proxy header matches JWT email (normalised on
both sides): pass through, no DB writes.
- SSO + user present + proxy header absent: pass through. Header
absence is NOT a logout signal per the spec — internal calls,
OPTIONS preflight, and direct backend hits legitimately arrive
without it.
- SSO + user present + proxy header asserts a DIFFERENT identity:
response.clearCookie('tokenPair', { path: '/' }), return false.
The frontend bootstraps without a tokenPair, routes through
/auth/sso/proxy-login under the new upstream identity, and gets
fresh tokens.
normalizeProxyEmail uses indexOf-based email-shape detection rather
than regex per the spec's polynomial-backtracking avoidance rule.
Bare-username Cognito pools (user_id_claim=cognito:username) get
synthesised against DEFAULT_EMAIL_DOMAIN so the comparison matches the
shape the SSO proxy-login flow uses to provision the user.
Tests (jwt-auth.guard.spec.ts, 11 cases):
- SSO match → passes through, no clearCookie
- SSO header absent → passes through, no clearCookie
- SSO whitespace-only header → passes through (treats as absent)
- SSO mismatch → refuse + clearCookie('tokenPair', { path: '/' })
- SSO case- and whitespace-variant match — bidirectional norm guard
- SSO bare-username synthesis against DEFAULT_EMAIL_DOMAIN
- AUTH_TYPE unset → mismatch check NOT run
- API-key auth (no user) → mismatch check NOT run
- Application auth (no user) → mismatch check NOT run
- clearCookie unavailable (defensive) → still refuses cleanly
- validateTokenByRequest throws → returns false
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
awais786
force-pushed
the
fix/proxy-auth-stale-session-on-user-switch
branch
from
0cf3292 to
d8b3e2b
Compare
|
Pressingly/plane#29 @copilot review with this rules. |
Agent-Logs-Url: https://github.com/Pressingly/twenty/sessions/1a2157d8-8957-4e36-bd85-54886713ef06 Co-authored-by: awais786 <445320+awais786@users.noreply.github.com>
Applied the analogous follow-up here in 4a3acb1: |
| * - If it isn't email-shaped (e.g. oauth2-proxy is forwarding a bare | ||
| * Cognito username via user_id_claim=cognito:username), synthesise | ||
| * `<local>@${DEFAULT_EMAIL_DOMAIN}` so the resulting key matches the | ||
| * one Twenty's SSO proxy-login flow uses to provision the user. | ||
| * | ||
| * The email-shape check uses indexOf rather than a regex to avoid | ||
| * polynomial-backtracking complexity on adversarial input — per | ||
| * openspec proxy-auth-middleware §"email-shape detection on header | ||
| * values SHALL avoid polynomial-backtracking regex". | ||
| */ | ||
| private normalizeProxyEmail(raw: string): string { | ||
| const trimmed = raw.toLowerCase().trim(); | ||
| const atIdx = trimmed.indexOf('@'); | ||
| const dotIdx = trimmed.indexOf('.', atIdx + 1); | ||
| const isEmailShaped = atIdx > 0 && dotIdx > atIdx + 1; | ||
|
|
||
| if (isEmailShaped) { |
There was a problem hiding this comment.
Addressed in commit 095250b3f7. normalizeProxyIdentity now uses .includes('@') rather than a stricter email-shape regex — any value containing @ is treated as an email as-is, matching SsoProxyLoginController.resolveEmail() semantics. The previous @.+\..+ form rejected legitimate emails with no dot in the domain (e.g. user@localhost); now they parse the same way in both code paths. Tests at :235 and :263 cover the @-without-dot case.
| const domain = this.twentyConfigService.get('DEFAULT_EMAIL_DOMAIN'); | ||
| const localPart = trimmed.split('@')[0]; | ||
|
|
||
| return `${localPart}@${domain}`; |
There was a problem hiding this comment.
Addressed in commit 4ac21b60b8 — fail-closed on unresolved bare-username identity. When the proxy header is a bare username (no @) and DEFAULT_EMAIL_DOMAIN is unset, normalizeProxyIdentity now returns null instead of synthesising an invalid local@ value. matchesProxyIdentity treats the null as mismatch → flush. Test refuses and clears tokenPair when bare proxy identity arrives without DEFAULT_EMAIL_DOMAIN configured (line 392) pins the behaviour.
Agent-Logs-Url: https://github.com/Pressingly/twenty/sessions/0ce3b648-9f2d-4277-879c-7cd33477f39a Co-authored-by: awais786 <445320+awais786@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Pressingly/twenty/sessions/0ce3b648-9f2d-4277-879c-7cd33477f39a Co-authored-by: awais786 <445320+awais786@users.noreply.github.com>
Aligned the guard with the proxy-login controller behavior in 4ac21b6: identities with |
JwtAuthGuard only runs on routes decorated with the guard. Twenty's REST and GraphQL middleware also validate the JWT and hydrate the request via MiddlewareService — without the mismatch check there, a stale cookie request could still be served as the previous user through those paths. Extract matchesProxyIdentity / resolveProxyIdentity / normalizeProxyIdentity / clearTokenPairCookie into a shared proxy-identity.util.ts. Invoke from both hydrateGraphqlRequest and hydrateRestRequest in MiddlewareService. JwtAuthGuard now consumes the same utility so logic can't drift across call sites. Bidirectional normalisation (`.toLowerCase().trim()` on BOTH the proxy header and the JWT user email) is required by openspec/specs/proxy-auth-middleware/spec.md "Match is case- and whitespace-insensitive". Asymmetric normalisation would falsely trigger mismatch on whitespace-padded legacy DB rows. Added a specific regression-guard test covering both case- and whitespace- variants on either side. Gating preserved: AUTH_TYPE !== 'SSO' skips entirely (non-SSO deploys untouched); jwtEmail absent skips (API keys / app tokens have no User); proxy headers absent passes through (not a logout signal per spec). Tests: 12 new cases covering match, mismatch, header absence, AUTH_TYPE=PASSWORD, API key, no-token short-circuit, user-header fallback, fail-closed on bare username + missing DEFAULT_EMAIL_DOMAIN, missing response object, and the case/ whitespace regression guards.
|
@copilot review Final pre-merge review request. Branch is at
Changes since the previous review:
Please confirm no remaining leaks across the auth surface. |
Reviewed at e709fa6. I don’t see another stale-session leak on the browser auth surface: the proxy-identity mismatch check now covers the guarded REST/controller path plus both GraphQL and REST hydration paths, and the remaining direct |
Summary
Closes the stale-session-on-user-switch class of bug in Twenty.
JwtAuthGuardvalidated the Bearer JWT from thetokenPaircookie without consulting the upstream identity that oauth2-proxy was asserting. Once Twenty issued the cookie via/auth/sso/proxy-login, the cookie was the sole source of identity on every subsequent request.After a portal "Logout all" + login as a different user — which clears the shared
_oauth2_proxycookie and Cognito SSO session but not Twenty'stokenPaircookie ontwenty.<domain>— refreshing the Twenty tab kept serving the previous user.Implements proxy-auth-middleware Rule 2 ("Identity mismatch SHALL flush the existing session immediately") per the cross-app contract in
awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md.Behaviour
After successful
validateTokenByRequest, before binding data to the request:AUTH_TYPE !== SSOdata.userabsent (API key, application token)response.clearCookie('tokenPair', { path: '/' })→return false. Frontend bootstrap finds notokenPair, routes through/auth/sso/proxy-loginunder new upstream identity, gets fresh tokensSpec conformance
clearCookie('tokenPair')beforereturn false; no path where mismatch is detected and the stale cookie survivesif (!headerRaw || headerRaw.trim() === '') return trueshort-circuits the mismatch pathnormalizeProxyEmail(headerRaw)(lowercase + trim) on header;.toLowerCase()ondata.user.email. Regression-guard test pins this.indexOf-based check (atIdx > 0 && dotIdx > atIdx + 1), not regex. Same rule Outline twentyhq#19 adopted.<local>@${DEFAULT_EMAIL_DOMAIN}matches the shape Twenty's/auth/sso/proxy-loginuses to provision the usertwentyConfigService.get('AUTH_TYPE') === 'SSO'gates the entire mismatch checkTest plan
packages/twenty-server/src/engine/guards/__tests__/jwt-auth.guard.spec.ts— 11 cases, all passing:clearCookieclearCookieclearCookie('tokenPair', { path: '/' })DEFAULT_EMAIL_DOMAINAUTH_TYPEunset → mismatch check NOT run (non-SSO bypass)useron context) → mismatch check NOT runuseron context) → mismatch check NOT runclearCookieunavailable (defensive harness check) → still refuses cleanlyvalidateTokenByRequestthrows → returns falseRun:
cd packages/twenty-server && npx jest src/engine/guards/__tests__/jwt-auth.guard.spec.tsManual repro verification
twenty.<domain>).Before: still served as A.
After:
tokenPaircleared on the refresh; bootstrap routes through/auth/sso/proxy-login; served as B.Out of scope
This addresses Twenty only. Plane has the analogous fix in Pressingly/plane#29, Outline in Pressingly/outline#19, Penpot in Pressingly/penpot#18. SurfSense is architecturally immune (FastAPI re-derives identity from headers on every request, no native session cookie).
🤖 Generated with Claude Code