Skip to content

fix: handle missing transaction cookie with auth-state-aware response#797

Merged
cschetan77 merged 7 commits intomasterfrom
SDK-8003
Apr 16, 2026
Merged

fix: handle missing transaction cookie with auth-state-aware response#797
cschetan77 merged 7 commits intomasterfrom
SDK-8003

Conversation

@aks96
Copy link
Copy Markdown
Contributor

@aks96 aks96 commented Mar 12, 2026

Description

Problem

When the auth_verification transaction cookie is absent at the callback route, the SDK threw:

BadRequestError: checks.state argument is missing

This message is an internal openid-client detail, it gives developers no indication of why the cookie is missing or how to fix it. The same opaque error surfaced across three distinct root causes:

  1. Back-button / stale callback — user completed login, transaction cookie was consumed, then the browser re-fired the same /callback URL (e.g pressing back).
  2. Dropped SameSite=None cookie — browser rejected the cookie because the origin is HTTP, not HTTPS (Safari ITP, privacy mode, Chrome on non-HTTPS)
  3. legacySameSiteCookie=false + buggy browser — primary cookie blocked on cross-site return, no fallback cookie set
  4. Multi-app cookie collision — two apps on the same domain with the same transactionCookie.name and cookie.path, one app's login overwrote the other's cookie

Fix

After getOnce() returns undefined, the handler now branches on authentication state:

  1. Authenticated user → silent redirect to baseURL
    The transaction cookie was legitimately consumed on the first successful callback. The browser re-firing the same URL is a benign back-button navigation. The user is already logged in — redirect them home with no error.

  2. Unauthenticated user → descriptive 400 error
    The cookie is missing for a user who hasn't established a session yet. The error message names the cookie, lists all four root causes, and suggests fix for each — replacing the cryptic checks.state argument is missing with actionable developer guidance.

Testing

  1. Verify all existing and new tests pass.
  2. Stale Callback - Authenticated User.
  3. Missing Cookie - Unauthenticated User.
  4. Multi-App Cookie Collision.

@aks96 aks96 requested a review from a team as a code owner March 12, 2026 07:29
@cschetan77
Copy link
Copy Markdown
Contributor

Hey @aks96, went through the issue and fix being implemented in this PR -

  1. In my opinion This doesn't address the underlying root cause that most reporters are hitting
    Issue BadRequestError: checks.state argument is missing when routing between pages using browser's back button #625 seems to have two distinct failure modes that both produce the same error message, which makes them easy to conflate:

    • Cookie collision (what this PR targets): App 2's login flow overwrites App 1's auth_verification cookie before App 1's callback fires. Auto-deriving cookie.path from baseURL does prevent this — but only when
      the two apps are differentiated by URL path (e.g. example.com/app1 vs example.com/app2). If both apps sit at the same origin root, both derive cookie.path = '/' and still collide.
    • Stale callback re-fire (Main RC): A user completes login successfully, then presses the browser back button. The browser re-issues the same /callback?code=...&state=... stale request. By this point getOnce() has already consumed and deleted the auth_verification cookie on the first pass, so the second request arrives with no cookie. The code falls into the checks = {} path and openid-client throws checks.state argument is missing. This has nothing to do with cookie naming or path scoping — it might happen on a single app with a perfectly unique cookie configuration.

    The back-button scenario is arguably the more common one in single-app deployments, and it should remain a hard BadRequestError: checks.state argument is missing crash after this PR as well.

  2. This looks to be a silent breaking change for existing deployments
    Today session.cookie.path defaults to undefined. appSession.js has an explicit fallback (emptyCookieOptions.path = emptyCookieOptions.path || '/') that forces the session cookie onto Path=/. The transaction cookie similarly lands at Path=/ because the browser defaults an unset Path attribute to the root of the request URI (/login, /callback → both /).

    So right now, for every existing deployment, all cookies live at Path=/.

    After this PR, any app with a non-root baseURL (e.g. https://example.com/app1) starts writing cookies at Path=/app1. On upgrade:

    • Existing users' browsers hold session cookies at Path=/
    • The upgraded app reads and writes at Path=/app1
    • The browser ends up sending both the old and new cookie on some requests, behaviour becomes unpredictable, and in most cases the user gets silently logged out

    This only affects apps with a path component in their baseURL — root apps (https://example.com) are fine — but it's a session-invalidating upgrade for anyone who is affected and there's no mention of it in the
    PR description or changelog.

Let me know your thoughts.

@aks96
Copy link
Copy Markdown
Contributor Author

aks96 commented Mar 24, 2026

Thank you for your review.

  1. Two distinct failure modes: Issue BadRequestError: checks.state argument is missing when routing between pages using browser's back button #625 conflates cookie collision with the back-button/stale callback scenario. Auto-deriving cookie path only addresses the collision case, not the back-button issue where getOnce() has already consumed the cookie.

  2. I hadn't fully considered the upgrade path. Existing deployments have cookies at Path=/, and changing the default would cause those users to be silently logged out

I've revised this PR to make cookie path configuration opt-in rather than auto-derived. No breaking change

Comment thread lib/context.js Outdated
cschetan77
cschetan77 previously approved these changes Mar 30, 2026
Comment thread lib/context.js
Comment thread lib/context.js
Comment thread index.d.ts
@cschetan77 cschetan77 changed the title fix:auto-derive cookie path from baseURL fix: handle missing transaction cookie with auth-state-aware response Apr 13, 2026
@cschetan77 cschetan77 merged commit 82d0a7d into master Apr 16, 2026
9 checks passed
@cschetan77 cschetan77 deleted the SDK-8003 branch April 16, 2026 04:37
@cschetan77 cschetan77 mentioned this pull request Apr 16, 2026
@natewaddoups
Copy link
Copy Markdown

Thank you @cschetan77 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants