Skip to content

[DT-4012] Handle the full consent /api/user/me contract and B2C authorization-response errors - #3881

Merged
rushtong merged 7 commits into
developfrom
gr-dt-4012-user-me-error-handling
Aug 26, 2026
Merged

[DT-4012] Handle the full consent /api/user/me contract and B2C authorization-response errors#3881
rushtong merged 7 commits into
developfrom
gr-dt-4012-user-me-error-handling

Conversation

@rushtong

@rushtong rushtong commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Addresses

DT-4012

Security risk: low — sign-in error paths change how a refusal is presented; no change to who is admitted.

Summary

Consent now answers GET /api/user/me with distinct statuses (DT-3997 #3021, DT-4011 #3032): 401 rejected token, 404 authenticated but unregistered, 409 Sam sub-provider conflict with an actionable message. The BFF and client now map that contract instead of the old conflated handling:

  • server/src/auth/me.ts: 401 destroys the session (the profileSeen disambiguator is dead machinery and is removed, along with its once-per-session store write); 404 keeps the "authenticated, no user" answer that routes new users into registration; 409 forwards the upstream message as { error: 'provider_conflict', message } after destroying the session — the account lives under the other provider, so the session cannot become usable.
  • src/libs/auth/session.ts: the probe treats 409 as an authoritative signed-out answer and shows the message (sign in with the other provider, plus the support link) instead of failing sign-in generically.
  • src/libs/auth/postSignIn.ts: a 409 from getMe signs the user out with the message instead of attempting a registration that cannot succeed. The legacy "azureb2c authentication error" substring check stays for older consent builds.

A second edge case is outside consent entirely: B2C can answer the authorization request itself with an error instead of a code — the user's own cancel, or a B2C-side failure. The observed case is AADB2C90289/invalid_client: the non-prod B2C tenant's federation client secret to the upstream Microsoft provider has expired, which fails every Microsoft sign-in in those environments (secret rotation is tracked separately). authorizationCodeGrant throws AuthorizationResponseError, and the resulting 500 answered the browser's top-level /auth/callback navigation with raw JSON, stranding the user:

  • server/src/auth/callback.ts: catch AuthorizationResponseError; a cancel (access_denied) redirects home silently, anything else logs the B2C error and redirects to /?signInError=provider. State-mismatch and token-validation failures still throw.
  • src/App.tsx: a fixed toast for the signInError marker, then the marker is stripped from the URL. The marker is a key, never reflected text, so the toast cannot echo attacker-supplied content.

Deployment note: the 401 tightening assumes every environment runs a consent build with both fixes above. An old consent behind this BFF would destroy a brand-new user's session on the first probe and block registration.

Testing

pnpm run lint, pnpm run type-check, pnpm test (368 files, 4312 tests), and the server suite (327 tests) pass. New coverage: 401 destroys the session (including a failing session store); 409 forwards the upstream message, falls back on an unusable body, and overwrites the probe's held signed-in answer; the bootstrap signs out on a 409 without a registration attempt; the callback's authorization-error redirects run against the real openid-client grant (fake B2C), alongside the unchanged security rejections; the App toast renders once and the marker is stripped.

The Microsoft sign-in repro was observed manually and produced the exact error this PR handles.

404 User Not Found -> New User Registration Case

Screenshot 2026-08-25 at 12 25 32 PM

409 Sam AzureB2C Error Case

Screenshot 2026-08-25 at 11 46 21 AM

500 Auth Provider Error

Screenshot 2026-08-25 at 1 32 59 PM

Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

🤖 Generated with Claude Code

rushtong and others added 2 commits August 25, 2026 09:56
…client

Consent now answers /api/user/me with distinct statuses (DT-3997 #3021,
DT-4011 #3032): 401 rejected token, 404 authenticated but unregistered,
409 Sam sub-provider conflict with an actionable message.

- server/src/auth/me.ts: 401 destroys the session (no more profileSeen
  disambiguation), 404 keeps the "authenticated, no user" answer, and
  409 forwards the upstream message as
  { error: 'provider_conflict', message } after destroying the session.
- server/src/types/session.ts: drop the now-dead profileSeen flag (and
  its once-per-session store write).
- src/libs/auth/session.ts: the probe treats 409 as an authoritative
  signed-out answer and shows the conflict message instead of failing
  sign-in generically.
- src/libs/auth/postSignIn.ts: a 409 from getMe signs the user out with
  the message instead of attempting a registration that cannot succeed;
  the legacy "azureb2c authentication error" substring check stays for
  older consent builds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aw 500 JSON

B2C can answer the authorization request with an error instead of a
code — the known case is a Microsoft identity outside the accepted
client ids (e.g. a personal Live account), plus the user's own cancel
(access_denied). authorizationCodeGrant then throws
AuthorizationResponseError, and the thrown 500 answered the browser's
top-level /auth/callback navigation with raw JSON, stranding the user.

- server/src/auth/callback.ts: catch AuthorizationResponseError; a
  cancel redirects home silently, anything else redirects to
  /?signInError=provider. Other failures (state mismatch, bad token)
  still throw.
- src/App.tsx: show a fixed error toast for the signInError marker and
  strip it from the URL. The marker is a key, never reflected text.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for DUOS Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 89.51% 11452 / 12794
🔵 Statements 88.89% 12177 / 13699
🔵 Functions 86.23% 3337 / 3870
🔵 Branches 81.6% 7160 / 8774
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/App.tsx 100% 94.11% 100% 100%
src/libs/auth/postSignIn.ts 95.74% 88.88% 100% 100% 106, 129, 159, 164
src/libs/auth/session.ts 100% 92.3% 100% 100%
server/src/auth/callback.ts 100% 100% 100% 100%
server/src/auth/me.ts 100% 95.45% 100% 100%
server/src/types/session.ts 100% 100% 100% 100%
Generated in workflow #6889 for commit 2396d50 by the Vitest Coverage Report Action

… (S3776)

Sonar flagged getMe at 18 against the allowed 15 after the 401/404/409
mapping landed. The refresh-before-forward block moves to
refreshedIfExpiring() and the 409 body parse to
providerConflictMessage() — same behavior, getMe now reads as one
status-to-answer mapping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI 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.

Pull request overview

Updates DUOS UI + BFF sign-in/session handling to match Consent’s expanded /api/user/me contract (401/404/409) and to gracefully handle Azure B2C authorization-response errors during /auth/callback, surfacing actionable user guidance instead of generic failures.

Changes:

  • Map Consent/BFF 409 provider_conflict to a signed-out session with a user-facing actionable toast, and prevent impossible registration attempts.
  • Treat upstream 401 as a terminal rejected-session response (destroy session) while preserving the “authenticated but unregistered” flow for 404.
  • Catch openid-client AuthorizationResponseError in /auth/callback and redirect back to the SPA with a stable signInError marker that the SPA turns into a toast and then strips.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/libs/auth/session.spec.ts Adds probe-level coverage for 409 provider-conflict behavior and caching semantics.
test/libs/auth/postSignIn.spec.ts Adds coverage to ensure 409 from getMe signs out and does not attempt registration; keeps legacy substring path.
test/components/App.spec.tsx Adds coverage for SPA toast + URL marker stripping when landing with ?signInError=provider.
src/libs/auth/session.ts Implements 409 handling in the BFF session probe and shows an actionable toast once per attempt.
src/libs/auth/postSignIn.ts Treats 409 from getMe as terminal conflict: show message + sign out rather than register.
src/App.tsx Adds a boot-time handler for signInError marker to show a fixed toast and strip the marker from the URL.
server/test/me.test.ts Updates server-side contract tests for 401 destroy behavior and new 409 forwarding + fallback.
server/test/authCrypto.test.ts Adds coverage for callback redirects when B2C returns authorization-response errors (server_error/access_denied).
server/src/types/session.ts Removes profileSeen from the session type (no longer needed with the new contract).
server/src/auth/me.ts Reworks /auth/me mapping for 401/404/409 and removes profileSeen-based behavior.
server/src/auth/callback.ts Catches AuthorizationResponseError and redirects to SPA rather than returning raw JSON errors.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread server/src/auth/me.ts
Comment thread server/src/auth/callback.ts
… level

- me.ts: the !res.ok comment claimed "non-4xx", but the branch also
  catches contract-undefined 4xx (400/403/429) — reworded to match the
  behavior, which stays deliberately transient.
- callback.ts: log the user's own B2C cancel (access_denied) at info so
  it stays out of warn-based alerting; real provider errors stay warn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rushtong
rushtong marked this pull request as ready for review August 25, 2026 16:34
@rushtong
rushtong requested a review from a team as a code owner August 25, 2026 16:34
@rushtong
rushtong requested review from fboulnois and kevinmarete and removed request for a team August 25, 2026 16:34
@rushtong

This comment was marked as outdated.

@rushtong
rushtong marked this pull request as draft August 25, 2026 17:12
rushtong and others added 3 commits August 25, 2026 13:12
…toast

The server_error on /auth/callback is not a rejected account type: the
non-prod B2C tenant's federation client secret to the upstream
Microsoft provider expired (AADB2C90289 / invalid_client), which fails
every Microsoft sign-in in those environments. The old toast told users
their account type was unsupported — wrong advice for a provider-side
fault. It now says the provider reported an error, try again, contact
Terra support; the server log keeps the B2C error and description.
Comments and the test's error_description now name the real cause.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@rushtong
rushtong marked this pull request as ready for review August 25, 2026 17:34

@fboulnois fboulnois 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.

👍

@rushtong
rushtong merged commit ee04a28 into develop Aug 26, 2026
12 checks passed
@rushtong
rushtong deleted the gr-dt-4012-user-me-error-handling branch August 26, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants