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

server/src/auth/callback.ts

@@ -17,10 +17,25 @@ export async function handleCallback(request: FastifyRequest, reply: FastifyRepl
   // inspected; DUOS_OAUTH_REDIRECT_URI supplies the base so it parses as an
   // absolute URL.
   const currentUrl = new URL(request.url, requireEnv('DUOS_OAUTH_REDIRECT_URI'))
-  const tokens = await oidc.authorizationCodeGrant(config, currentUrl, {
-    pkceCodeVerifier: request.session.pkceVerifier,
-    expectedState: request.session.pkceState,
-  })
+  let tokens: Awaited<ReturnType<typeof oidc.authorizationCodeGrant>>
+  try {
+    tokens = await oidc.authorizationCodeGrant(config, currentUrl, {
+      pkceCodeVerifier: request.session.pkceVerifier,
+      expectedState: request.session.pkceState,
+    })
+  }
+  catch (err: unknown) {
+    if (err instanceof oidc.AuthorizationResponseError) {
+      // B2C answered the authorization request with an error instead of a
+      // code — the user canceled on the B2C page (access_denied), or chose
+      // an identity the tenant's policy rejects. Land back in the
+      // SPA instead; a cancel is the user's own action and stays silent.
+      request.log.warn({ error: err.error, description: err.error_description }, '[auth] B2C authorization response is an error')
+      reply.redirect(err.error === 'access_denied' ? '/' : '/?signInError=provider')
+      return
+    }
+    throw err
+  }
 
   const claims = tokens.claims() // undefined when no id_token is present
 

server/src/auth/me.ts

@@ -5,43 +5,37 @@
 const UPSTREAM_TIMEOUT_MS = 5000
 
 /**
- * Answers an upstream 401/404. The DUOS API conflates "bad token" with "no
- * DUOS profile for this email" (both 401 — DuosUserAuthenticator turns the
- * lookup's NotFoundException into an empty principal), so the session's
- * `profileSeen` flag is the disambiguator this endpoint controls.
+ * Shown when the upstream 409 arrives without a usable message — the client
+ * needs something actionable even if the body was empty or malformed.
  */
-async function answerNoProfile(request: FastifyRequest, reply: FastifyReply): Promise<void> {
-  if (request.session.profileSeen) {
-    // This session has served a profile before, so "no profile" cannot be
-    // the explanation. The upstream is rejecting the token itself — revoked
-    // mid-lifetime (before refresh-before-forward would touch it) or the
-    // account was disabled. The terminal 401 is the honest answer; leaving
-    // the session alive would send the client into re-registering an
-    // existing user.
-    try {
-      await request.session.destroy()
-    }
-    catch (err: unknown) {
-      request.log.error({ err }, '[auth] upstream rejected a profile-seen session but it could not be destroyed — returning 401 anyway')
-    }
-    reply.clearCookie('sessionId').status(401).send({ authenticated: false })
-    return
+const PROVIDER_CONFLICT_FALLBACK_MESSAGE
+  = 'You may have previously signed in with a different authentication provider (Google or Microsoft). Please sign in with that provider.'
+
+/**
+ * Ends a session the upstream has authoritatively rejected.
+ */
+async function destroySession(request: FastifyRequest, reply: FastifyReply): Promise<void> {
+  try {
+    await request.session.destroy()
   }
-  // Never seen a profile: authenticated but not yet registered. Treating
-  // this 401 as a dead session destroyed every brand-new user's session on
-  // their first probe and made registration unreachable. In the rare case
-  // of a token revoked before first contact, the client's registration
-  // attempt fails too, which signs the session out. (404 kept deliberately:
-  // DT-3997 restores the upstream's 404 for unregistered users.)
-  reply.send({ authenticated: true, idp: request.session.idp })
+  catch (err: unknown) {
+    request.log.error({ err }, '[auth] upstream rejected the session but it could not be destroyed — answering as signed out anyway')
+  }
+  reply.clearCookie('sessionId')
 }
 
 /**
  * Confirms the user is authenticated against the upstream Consent API.
  * Forwards the upstream user profile and the active sub-provider — never the
  * tokens themselves, which stay server-side in the session.
+ *
+ * The upstream /api/user/me contract:
+ * - 200 registered profile
+ * - 401 rejected token
+ * - 404 authenticated but unregistered
+ * - 409 Sam sub-provider conflict
  */
 export async function getMe(request: FastifyRequest, reply: FastifyReply): Promise<void> {
   // The answer is per-session and now gates the whole SPA: no intermediary
   // (or the browser's heuristic cache) may replay one user's profile to
   // another, or a stale answer to the same user.
@@ -98,13 +92,46 @@
     return
   }
 
-  if (res.status === 401 || res.status === 404) {
-    await answerNoProfile(request, reply)
+  if (res.status === 401) {
+    // The upstream rejected the token itself — revoked mid-lifetime (before refresh-before-forward would
+    // touch it) or the account was disabled. The terminal 401 is final so the session must be destroyed.
+    // The client still needs to know it is signed out, so the reply goes out after the session is destroyed.
+    await destroySession(request, reply)
+    reply.status(401).send({ authenticated: false })
+    return
+  }
+
+  if (res.status === 404) {
+    // Authenticated but not yet registered — the session is valid, and the
+    // client's post-sign-in bootstrap needs authenticated:true (with user
+    // absent) to reach its registration flow.
+    reply.send({ authenticated: true, idp: request.session.idp })
+    return
+  }
+
+  if (res.status === 409) {
+    // Sam sub-provider conflict: the account exists under the other B2C sub-provider and Sam rejects it.
+    // Registration cannot succeed and the session cannot become usable, so end it — but forward the
+    // upstream's actionable message (sign in with the other provider, plus the support link) instead
+    // of a generic failure.
+    let message = PROVIDER_CONFLICT_FALLBACK_MESSAGE
+    try {
+      const body: unknown = await res.json()
+      const upstreamMessage = (body as { message?: unknown } | null)?.message
+      if (typeof upstreamMessage === 'string' && upstreamMessage.length > 0) {
+        message = upstreamMessage
+      }
+    }
+    catch {
+      // Unparseable body — the fallback message stands.
+    }
+    await destroySession(request, reply)
+    reply.status(409).send({ authenticated: false, error: 'provider_conflict', message })
     return
   }
 
   if (!res.ok) {
-    // A non-401 failure (5xx, upstream outage) says nothing about whether the
+    // A non-4xx failure (5xx, upstream outage) says nothing about whether the
     // token itself is still valid — don't destroy the session or parse an
     // error body as if it were a user profile.
     reply.status(502).send({ authenticated: false, error: 'upstream_unavailable' })
@@ -120,15 +147,6 @@
     return
   }
 
-  if (!request.session.profileSeen) {
-    request.session.profileSeen = true
-    // Saved explicitly before the reply, once per session (the flag never
-    // flips back): with rolling off, the onSend hook only skips its async
-    // save when the session is unmodified — see index.ts on the
-    // ERR_HTTP_HEADERS_SENT hazard a post-reply async save creates.
-    await request.session.save()
-  }
-
   reply.send({
     authenticated: true,
     user,

server/src/types/session.ts

@@ -23,10 +23,5 @@ declare module 'fastify' {
     // regardless — this field exists for the audit trail and observability,
     // not client selection.
     idp?: 'google' | 'microsoft'
-    // Set once /auth/me has served a DUOS profile on this session. The
-    // upstream conflates "no profile" with "bad token" (both 401), and this
-    // is the disambiguator: a 401/404 on a profile-seen session cannot mean
-    // "unregistered", so it is treated as a terminal token verdict.
-    profileSeen?: boolean
   }
 }

server/test/authCrypto.test.ts

@@ -310,6 +310,40 @@ describe('B2C OAuth callback (real openid-client validation against a fake B2C)'
     expect(res.statusCode).toBe(500)
   })
 
+  it('redirects to /?signInError=provider when B2C answers with an error instead of a code', async () => {
+    // Edge case: the user picked an identity the tenant's policy
+    // rejects (e.g. a personal Microsoft Live account). B2C redirects back
+    // with error=server_error — the real authorizationCodeGrant throws
+    // AuthorizationResponseError, which must land the browser in the SPA
+    const { cookie, state } = await login()
+
+    const res = await app.inject({
+      method: 'GET',
+      url: `/auth/callback?error=server_error&error_description=AADB2C90085&state=${state}`,
+      headers: { cookie },
+    })
+
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/?signInError=provider')
+    const sess = [...rows.values()][0].sess as Record<string, unknown>
+    expect(sess.accessToken).toBeUndefined()
+  })
+
+  it('redirects home silently when the user cancels on the B2C page (access_denied)', async () => {
+    const { cookie, state } = await login()
+
+    const res = await app.inject({
+      method: 'GET',
+      url: `/auth/callback?error=access_denied&error_description=AADB2C90091&state=${state}`,
+      headers: { cookie },
+    })
+
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/')
+    const sess = [...rows.values()][0].sess as Record<string, unknown>
+    expect(sess.accessToken).toBeUndefined()
+  })
+
   it('returns 400 token_missing_email_claim when a valid id_token has no email', async () => {
     // The token is cryptographically valid — the grant succeeds — so this
     // exercises the handler guard on top of real validation, not a crypto error.

server/test/me.test.ts

@@ -16,21 +16,18 @@ const ENV = { DUOS_API_URL: 'https://consent.dsde-dev.broadinstitute.org' }
 // tests exercise the forward path untouched.
 const FRESH_EXPIRY = () => Math.floor(Date.now() / 1000) + 3600
 
-function makeRequest(overrides: { accessToken?: string, idp?: 'google' | 'microsoft', tokenExpiry?: number, profileSeen?: boolean } = {}) {
+function makeRequest(overrides: { accessToken?: string, idp?: 'google' | 'microsoft', tokenExpiry?: number } = {}) {
   const destroy = vi.fn().mockResolvedValue(undefined)
-  const save = vi.fn().mockResolvedValue(undefined)
   const request = {
     session: {
       accessToken: overrides.accessToken,
       idp: overrides.idp,
       tokenExpiry: overrides.tokenExpiry ?? FRESH_EXPIRY(),
-      profileSeen: overrides.profileSeen,
       destroy,
-      save,
     },
     log: { error: vi.fn(), info: vi.fn() },
   }
-  return { request: request as unknown as FastifyRequest, destroy, save }
+  return { request: request as unknown as FastifyRequest, destroy }
 }
 
 function makeReply() {
@@ -106,33 +103,12 @@ describe('getMe', () => {
     })
   })
 
-  it('marks the session profile-seen on the first served profile, with an explicit pre-reply save', async () => {
-    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(200, { email: 'user@example.com' }) as never)
-    const { request, save } = makeRequest({ accessToken: 'test-access-token' })
-
-    await getMe(request, makeReply())
-
-    expect(request.session.profileSeen).toBe(true)
-    expect(save).toHaveBeenCalledOnce()
-  })
-
-  it('does not re-save a session already marked profile-seen', async () => {
-    // Focus revalidation makes this path hot — the flag costs one DB write
-    // per session, not one per probe.
-    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(200, { email: 'user@example.com' }) as never)
-    const { request, save } = makeRequest({ accessToken: 'test-access-token', profileSeen: true })
-
-    await getMe(request, makeReply())
-
-    expect(save).not.toHaveBeenCalled()
-  })
-
-  it.each([401, 404])('destroys a profile-seen session on an upstream %i — "no profile" cannot explain it', async (status) => {
+  it('destroys the session on an upstream 401', async () => {
     // A registered user's token revoked mid-lifetime forwards (no refresh due)
     // and 401s. Answering "authenticated, no user" here sent the client into
     // re-registering an existing account; the terminal 401 is the honest end.
-    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(status, {}) as never)
-    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token', idp: 'google', profileSeen: true })
+    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(401, {}) as never)
+    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token', idp: 'google' })
     const reply = makeReply()
 
     await getMe(request, reply)
@@ -143,6 +119,19 @@ describe('getMe', () => {
     expect(reply.send).toHaveBeenCalledWith({ authenticated: false })
   })
 
+  it('still answers 401 when the rejected session cannot be destroyed', async () => {
+    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(401, {}) as never)
+    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token' })
+    destroy.mockRejectedValue(new Error('store unavailable'))
+    const reply = makeReply()
+
+    await getMe(request, reply)
+
+    expect(reply.clearCookie).toHaveBeenCalledWith('sessionId')
+    expect(reply.status).toHaveBeenCalledWith(401)
+    expect(reply.send).toHaveBeenCalledWith({ authenticated: false })
+  })
+
   it('marks every answer uncacheable — the profile must never be replayed across sessions', async () => {
     vi.mocked(fetch).mockResolvedValue(makeFetchResponse(200, { email: 'user@example.com' }) as never)
     const { request } = makeRequest({ accessToken: 'test-access-token', idp: 'google' })
@@ -163,20 +152,35 @@ describe('getMe', () => {
     expect(reply.header).toHaveBeenCalledWith('cache-control', 'no-store')
   })
 
-  it('reports authenticated with no user on an upstream 401 — DUOS conflates "no profile" with "bad token"', async () => {
-    // DuosUserAuthenticator turns an unregistered email's NotFoundException
-    // into an empty principal → Dropwizard 401. The token itself was just
-    // refreshed, so this must NOT destroy the session: it is a brand-new
-    // user who needs the registration bootstrap, not a dead session.
-    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(401, {}) as never)
-    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token', idp: 'google' })
+  it('forwards the upstream 409 message and destroys the session — a Sam sub-provider conflict cannot be registered through', async () => {
+    const message = 'Email: user@example.com. You may have previously signed in with a different authentication provider (Google or Microsoft). Please sign in with that provider.'
+    vi.mocked(fetch).mockResolvedValue(makeFetchResponse(409, { message, code: 409 }) as never)
+    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token', idp: 'microsoft' })
     const reply = makeReply()
 
     await getMe(request, reply)
 
-    expect(destroy).not.toHaveBeenCalled()
-    expect(reply.clearCookie).not.toHaveBeenCalled()
-    expect(reply.send).toHaveBeenCalledWith({ authenticated: true, idp: 'google' })
+    expect(destroy).toHaveBeenCalledOnce()
+    expect(reply.clearCookie).toHaveBeenCalledWith('sessionId')
+    expect(reply.status).toHaveBeenCalledWith(409)
+    expect(reply.send).toHaveBeenCalledWith({ authenticated: false, error: 'provider_conflict', message })
+  })
+
+  it('answers the 409 with a fallback message when the upstream body is unusable', async () => {
+    const badBody = { status: 409, ok: false, json: vi.fn().mockRejectedValue(new Error('invalid JSON')) }
+    vi.mocked(fetch).mockResolvedValue(badBody as never)
+    const { request, destroy } = makeRequest({ accessToken: 'fresh-access-token' })
+    const reply = makeReply()
+
+    await getMe(request, reply)
+
+    expect(destroy).toHaveBeenCalledOnce()
+    expect(reply.status).toHaveBeenCalledWith(409)
+    expect(reply.send).toHaveBeenCalledWith({
+      authenticated: false,
+      error: 'provider_conflict',
+      message: expect.stringContaining('different authentication provider'),
+    })
   })
 
   it('does not refresh when the access token is comfortably fresh', async () => {
@@ -248,7 +252,7 @@ describe('getMe', () => {
     expect(reply.send).toHaveBeenCalledWith({ authenticated: true, idp: 'microsoft' })
   })
 
-  it('returns 502 without destroying the session when the upstream API errors with a non-401 status', async () => {
+  it('returns 502 without destroying the session when the upstream API errors with an unmapped status', async () => {
     vi.mocked(fetch).mockResolvedValue(makeFetchResponse(503, {}) as never)
     const { request, destroy } = makeRequest({ accessToken: 'test-access-token' })
     const reply = makeReply()

src/App.tsx

@@ -49,6 +49,24 @@ function App() {
     // mount instead of on every render.
   }, [])
 
+  /**
+   * The BFF /auth/callback lands here with ?signInError=provider when B2C answered the authorization request with an
+   * error instead of a code. The known case is when Microsoft provides login options that use an unsupported client id
+   */
+  useEffect(() => {
+    const queryParams = new URLSearchParams(location.search)
+    if (queryParams.get('signInError') === null) return
+    Notifications.showError({
+      text: 'Sign in could not be completed because the identity provider reported an error. '
+        + 'If you chose a personal Microsoft account (such as an Outlook.com or Live account), that account type is not supported — '
+        + 'please sign in with Google or with a Microsoft work or school account.',
+      // Long timeout: the message carries instructions the user must read.
+      timeout: 30000,
+    })
+    queryParams.delete('signInError')
+    navigate({ pathname: location.pathname, search: queryParams.toString() }, { replace: true })
+  }, [navigate, location.pathname, location.search])
+
   /**
      * Check for RAS Authentication URL params. If we have a code and state, we will call ECM APIs to get redirect
      * information and user linkage information. With that, we can sync the users account linkage and then redirect the

src/libs/auth/postSignIn.ts

@@ -232,10 +232,12 @@ export const completeSignIn = async ({ navigate, queryClient, redirectPath, isCa
   }
   catch (error) {
     if (cancelled()) return 'cancelled'
-    // Explicitly handle AzureB2C errors from Sam
     const errorMessage = extractError(error)
-    if (errorMessage.toLowerCase().includes('azureb2c authentication error')) {
-      Notifications.showError({ text: errorMessage })
+    // A 409 from getMe is the Sam sub-provider conflict the account lives under the other provider,
+    // so registration cannot succeed — surface the actionable message instead of attempting it.
+    if (errorStatus(error) === 409 || errorMessage.toLowerCase().includes('azureb2c authentication error')) {
+      // Long timeout: the message carries instructions and a support link.
+      Notifications.showError({ text: errorMessage, timeout: 30000 })
       await Auth.signOut()
       return 'signed-out'
     }