Skip to content

fix(session): Reject non-user access tokens in session cookies - #514

Merged
m0tzy merged 1 commit into
mainfrom
fix/session-user-access-token-claims
Nov 5, 2026
Merged

m0tzy merged 1 commit into
mainfrom
fix/session-user-access-token-claims

Conversation

@m0tzy

@m0tzy m0tzy commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Summary

verifyAccessToken in session.ts (used by updateSession/withAuth and assertValidAccessToken) only ran jwtVerify: signature, exp, and iss when WORKOS_ISSUER is set. It didn't check that the token was a user session access token, so other token types signed by the same environment key were accepted as a session.

const { payload } = await jwtVerify(accessToken, JWKS(), issuer ? { issuer } : undefined);
if (!isUserAccessTokenPayload(payload)) {
  return { status: 'invalid', cause: new Error('Access token is not a user access token') };
}
// typeof sid === 'string' && typeof auth_time === 'number'
// && (sub_profile === undefined || sub_profile === 'user') && sub_type === undefined

This matches hasRequiredAccessTokenClaims in @workos-inc/node. sub_profile is optional on user access tokens, so a missing claim is still accepted. A rejected token takes the existing invalid path: the cookie is deleted and no refresh is attempted, the same as a bad signature.

Tests: a new should reject a verified token with %s without attempting to refresh it covers sub_profile: ai_agent, sub_type: authkit, a missing or string auth_time, and a missing or numeric sid. All 6 cases fail without the session.ts change.

Fixture plumbing: most specs mocked jwtVerify to resolve undefined, true or {}. They now resolve the shared verifiedUserAccessToken from test-helpers.ts, and generateTestToken includes auth_time by default.

Link to Devin session: https://app.devin.ai/sessions/7e123fc2545a49c282b043a4abd58f70
Open in Devin Desktop: https://app.devin.ai/desktop/session/7e123fc2545a49c282b043a4abd58f70?variant=devin
Requested by: @m0tzy

verifyAccessToken now requires sid and auth_time, a sub_profile that is absent
or user, and no sub_type, so other token types signed by the same environment
key do not authenticate a session.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@m0tzy
m0tzy requested a review from a team as a code owner November 4, 2026 22:28
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from madison.packer

What's the fix here? https://linear.app/workos/issue/VULN-4839/lt-app26-10-user-vs-agent-jwt-type-confusion

@devin-ai-integration

devin-ai-integration Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Devin Review

🟢 Devin Review found nothing to fix · View findings · Follow Devin's work

@greptile-apps

greptile-apps Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; no actionable defects were found.

Summary

This PR rejects non-user access tokens before trusting or saving a session.

  • Session cookies now accept only tokens with user-session claims.

No actionable issues were found. Tests were not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Verify access token] --> B{User token claims?}
  B -->|Yes| C[Use or save session]
  B -->|No| D[Reject token]
  D --> E[Clear existing cookie or refuse session write]
Loading

Reviews (1) · Last reviewed commit: "fix(session): Reject non-user access tok..." · Reviewed by Greptile

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@m0tzy
m0tzy merged commit 6dd859e into main Nov 5, 2026
8 checks passed
@m0tzy
m0tzy deleted the fix/session-user-access-token-claims branch November 5, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants