Welcome-page guard: cover upstream sign-in-view (rows 5/6/9)

docker-compose.yml

@@ -8,6 +8,13 @@ services:
     env_file: .env
     environment:
       - PDS_PORT=3000
+      # Enable /_internal/test/* hooks used by the e2e suite (e.g.
+      # backdating account_device.updatedAt past upstream's 7-day
+      # authenticationMaxAge so checkLoginRequired returns true). Mounted
+      # only here and on the Railway pr-base preview env; production
+      # deployments leave it unset. The router throws at startup if
+      # NODE_ENV=production.
+      - EPDS_TEST_HOOKS=${EPDS_TEST_HOOKS:-1}
     volumes:
       - pds-data:/data
     restart: unless-stopped

e2e/step-definitions/session-reuse-bugs.steps.ts

@@ -1,13 +1,16 @@
 /**
- * Step definitions for features/session-reuse-bugs.feature — the Layer 2
+ * Step definitions for features/session-reuse-bugs.feature — the
  * welcome-page guard scenarios. See docs/design/session-reuse-bugs.md for
  * the failure-mode taxonomy.
  *
  * These steps exercise the pre-route guard in pds-core that intercepts
  * /oauth/authorize and /account* before upstream's signin handler can
- * render the stock welcome page. The guard bounces to auth-service when
- * the dev-id/ses-id cookie pair is missing, malformed, or resolves to a
- * device with zero bound accounts.
+ * render its authentication UIs (stock welcome page or sign-in-view).
+ * The guard bounces to auth-service when:
+ *   - the dev-id/ses-id cookie pair is missing or malformed (rows 1–3)
+ *   - the cookie pair resolves to a device with zero bound accounts (row 4)
+ *   - the stored PAR carries `prompt=login` (row 5)
+ *   - every relevant binding's auth age exceeds 7 days (rows 6 / 9)
  *
  * All scenarios require a docker-compose topology where auth-service is a
  * sibling subdomain of pds-core so device-session cookies are domain-scoped
@@ -19,9 +22,10 @@ import { expect } from '@playwright/test'
 import type { EpdsWorld } from '../support/world.js'
 import { testEnv } from '../support/env.js'
 import { getPage } from '../support/utils.js'
-import { createAccountViaOAuth } from '../support/flows.js'
+import { createAccountViaOAuth, pickHandle } from '../support/flows.js'
 import { sharedBrowser } from '../support/hooks.js'
 import { clearMailpit, extractOtp, waitForEmail } from '../support/mailpit.js'
+import { fillOtp } from '../support/otp.js'
 
 // ---------------------------------------------------------------------------
 // Background: returning user completes an OAuth sign-in, leaving valid
@@ -55,8 +59,7 @@ Given(
     })
     const message = await waitForEmail(`to:${email}`)
     const otp = await extractOtp(message.ID)
-    await page.fill('#code', otp)
-    await page.click('#form-verify-otp .btn-primary')
+    await fillOtp(page, otp)
     await page.waitForURL('**/welcome', { timeout: 30_000 })
     await clearMailpit(email)
   },
@@ -332,7 +335,15 @@ Then(
   'the browser lands on the auth-service email-and-OTP form',
   async function (this: EpdsWorld) {
     const page = getPage(this)
-    await expect(page.locator('#email')).toBeVisible({ timeout: 30_000 })
+    // Wait for one of the auth-service login-page steps to be visible.
+    // initialStep is 'email' on no-hint flows and 'otp' when a login_hint
+    // resolves to an email — both land on the same login-page route, just
+    // with a different step displayed initially. Either is a valid "lands
+    // on the login page" outcome; the assertion is "we're on auth-service,
+    // not pds-core".
+    await expect(
+      page.locator('#step-email:not(.hidden), #step-otp.active'),
+    ).toBeVisible({ timeout: 30_000 })
     const url = new URL(page.url())
     const authHost = new URL(testEnv.authUrl).host
     expect(
@@ -564,3 +575,171 @@ Given(
     ).toBe(pdsHost)
   },
 )
+
+// ---------------------------------------------------------------------------
+// Sign-in-view leaks (rows 5/6/9). Distinct from the welcome-page leaks
+// above: cookies + bindings are valid, but every binding upstream would
+// consider has `loginRequired: true`, so upstream falls through to its
+// sign-in-view (handle + password form). ePDS accounts are passwordless, so
+// any path into that form is unusable. The guard must bounce these to
+// auth-service for a fresh OTP round.
+// ---------------------------------------------------------------------------
+
+When(
+  'the demo client starts a new OAuth flow with prompt=login',
+  async function (this: EpdsWorld) {
+    // /api/oauth/login on the demo accepts ?prompt=login and propagates
+    // it to BOTH the PAR body AND the authorize-URL query string
+    // (packages/demo/src/app/api/oauth/login/route.ts:146,162).
+    // Cookies are preserved (the device must already be bound from the
+    // Background) so once the auth-service-side OTP completes and the
+    // pds-core epds-callback redirects back to /oauth/authorize, the
+    // stored PAR still carries `prompt=login` — the row-5 trigger.
+    const page = getPage(this)
+    const url = new URL('/api/oauth/login', testEnv.demoUrl)
+    url.searchParams.set('prompt', 'login')
+    await page.goto(url.toString())
+  },
+)
+
+When('the user completes the OTP flow', async function (this: EpdsWorld) {
+  if (!testEnv.mailpitPass) return 'pending'
+  if (!this.testEmail) {
+    throw new Error(
+      'world.testEmail missing — Background step must establish a returning user first',
+    )
+  }
+  // Drive auth-service's email/OTP form to completion. Same shape as the
+  // Background's returning-user sign-in but invoked mid-scenario after
+  // the prompt=login redirect lands the user there.
+  const page = getPage(this)
+  await page.fill('#email', this.testEmail)
+  await page.click('button[type=submit]')
+  await expect(page.locator('#step-otp.active')).toBeVisible({
+    timeout: 30_000,
+  })
+  const message = await waitForEmail(`to:${this.testEmail}`)
+  const otp = await extractOtp(message.ID)
+  await fillOtp(page, otp)
+})
+
+Then(
+  'no upstream password field is rendered anywhere on the page',
+  async function (this: EpdsWorld) {
+    // Negative assertion: ePDS accounts are passwordless, so a password
+    // input on any screen we land on is a leak. Upstream's sign-in-view
+    // renders <input type="password">; auth-service's OTP form does not.
+    // Counting password inputs across the whole document keeps the
+    // assertion robust against future structural changes.
+    const page = getPage(this)
+    const passwordCount = await page.locator('input[type="password"]').count()
+    expect(
+      passwordCount,
+      `Found ${passwordCount} password input(s) on ${page.url()} — ePDS is passwordless, this is the upstream sign-in-view leak`,
+    ).toBe(0)
+  },
+)
+
+// Backdate `account_device.updated_at` past upstream's authenticationMaxAge
+// (7d) via a pds-core /_internal/test hook gated on EPDS_TEST_HOOKS=1. The
+// hook accepts a target DID and optional deviceId; omitting deviceId
+// backdates every device row for that DID (sufficient for row 6's
+// single-account device, while row 9 passes both did + deviceId for
+// surgical targeting).
+async function expireDeviceAccount(opts: {
+  did: string
+  deviceId?: string
+}): Promise<void> {
+  if (!testEnv.internalSecret) {
+    throw new Error(
+      'E2E_INTERNAL_SECRET is unset — cannot call /_internal/test/expire-device-account',
+    )
+  }
+  const url = new URL('/_internal/test/expire-device-account', testEnv.pdsUrl)
+  const res = await fetch(url, {
+    method: 'POST',
+    headers: {
+      'Content-Type': 'application/json',
+      'x-internal-secret': testEnv.internalSecret,
+    },
+    body: JSON.stringify(opts),
+  })
+  if (!res.ok) {
+    const body = await res.text()
+    throw new Error(`expire-device-account hook failed: ${res.status} ${body}`)
+  }
+}
+
+Given(
+  "the device's account_device row has been backdated past 7 days",
+  async function (this: EpdsWorld) {
+    if (!this.userDid) {
+      throw new Error('world.userDid missing — Background step must run first')
+    }
+    await expireDeviceAccount({ did: this.userDid })
+  },
+)
+
+Given('the device has two bound accounts', async function (this: EpdsWorld) {
+  if (!testEnv.mailpitPass) return 'pending'
+  if (!this.userDid) {
+    throw new Error('world.userDid missing — Background step must run first')
+  }
+  // Drive a second OAuth sign-in within the SAME browser context (no
+  // resetBrowserContext — the dev-id/ses-id cookies from the Background
+  // sign-in must survive). accountManager.upsertDeviceAccount(deviceId, sub)
+  // on the existing dev-id then binds the new account to the same device
+  // row, leaving the first binding intact.
+  //
+  // Identity extraction matches createAccountViaOAuth's regex pattern
+  // (e2e/support/flows.ts:165-173) — kept inline rather than reusing
+  // that helper because it writes to world.userDid/userHandle/testEmail
+  // which we explicitly do NOT want to clobber here.
+  const page = getPage(this)
+  const otherEmail = `row9-other-${Date.now()}@example.com`
+  await page.goto(testEnv.demoTrustedUrl)
+  await page.fill('#email', otherEmail)
+  await page.click('button[type=submit]')
+  await expect(page.locator('#step-otp.active')).toBeVisible({
+    timeout: 30_000,
+  })
+  const message = await waitForEmail(`to:${otherEmail}`)
+  const otp = await extractOtp(message.ID)
+  await fillOtp(page, otp)
+  await pickHandle(this)
+  await page.waitForURL('**/welcome', { timeout: 30_000 })
+
+  const bodyText = await page.locator('body').innerText()
+  const didMatch = /did:[a-z0-9:]+/i.exec(bodyText)
+  if (!didMatch) {
+    throw new Error('Could not find DID on welcome page for second user')
+  }
+  const handleMatch = /@([\w.-]+)/.exec(bodyText)
+  this.otherUserEmail = otherEmail
+  this.otherUserDid = didMatch[0]
+  this.otherUserHandle = handleMatch ? handleMatch[1] : undefined
+})
+
+Given(
+  "the test user's account_device row has been backdated past 7 days",
+  async function (this: EpdsWorld) {
+    if (!this.userDid) {
+      throw new Error('world.userDid missing — Background step must run first')
+    }
+    await expireDeviceAccount({ did: this.userDid })
+  },
+)
+
+Given(
+  "the other user's account_device row is fresh",
+  function (this: EpdsWorld) {
+    // No-op: the second sign-in driven by "the device has two bound
+    // accounts" leaves the other user's updatedAt at "now". This step
+    // exists to make the precondition explicit in the feature file.
+    if (!this.otherUserDid) {
+      throw new Error(
+        'world.otherUserDid missing — "the device has two bound accounts" step must run first',
+      )
+    }
+  },
+)

features/session-reuse-bugs.feature

@@ -1,15 +1,18 @@
 @session-reuse @docker-only @email
-Feature: Session-reuse resilience against stale device cookies
-  When a user's upstream @atproto/oauth-provider device cookies
-  (dev-id / ses-id) no longer match the server's device-session state,
-  the auth-service used to redirect them anyway. That landed the user on
-  pds-core's stock welcome page ("Authenticate / Create new account /
-  Sign in / Cancel") instead of the enriched ePDS account picker or the
-  email/OTP form.
-
-  ePDS must recover gracefully from every variety of cookie/server
-  divergence by falling back to the auth-service email/OTP form and
-  clearing the stale cookies so the next visit starts clean.
+Feature: Welcome-page guard suppresses upstream's authentication UI
+  Upstream @atproto/oauth-provider has two authentication UIs that ePDS
+  must never render: its stock welcome page ("Authenticate / Create new
+  account / Sign in / Cancel") and its sign-in-view (handle + password
+  form). Both are unreachable by design — ePDS accounts are passwordless
+  and authentication goes through auth-service's OTP flow.
+
+  The pre-route guard in pds-core (welcome-page-guard.ts) intercepts
+  every guarded GET and bounces to auth-service whenever upstream would
+  otherwise render either UI. ePDS must recover gracefully from every
+  variety of cookie/server divergence — and from every PAR-parameter
+  combination that would otherwise force upstream into its sign-in
+  form — by falling back to the auth-service email/OTP form and
+  clearing stale cookies so the next visit starts clean.
 
   See docs/design/session-reuse-bugs.md for the full failure-mode
   taxonomy. This feature covers the externally-reproducible cases.
@@ -140,3 +143,69 @@ Feature: Session-reuse resilience against stale device cookies
     When the demo client starts a new OAuth flow with the other user's handle as login_hint
     Then the login page renders directly at the OTP verification step
     And the OTP form will submit the other user's email
+
+  # ---------------------------------------------------------------------------
+  # Sign-in-view leaks (rows 5/6/9 of the failure-mode taxonomy). Distinct
+  # from the welcome-page leaks above: here cookies and bindings are valid,
+  # but every binding upstream would consider has `loginRequired: true`, so
+  # upstream's only remaining path is to render its sign-in-view (handle +
+  # password form). The chooser may render first as an intermediate step,
+  # but every account on it leads to sign-in-view on click — equally a leak.
+  # ePDS accounts are passwordless, so any path into that form is unusable.
+  # The guard must bounce these to auth-service for a fresh OTP round.
+  #
+  # Three independent triggers force every binding into loginRequired:
+  #   Row 5 — stored PAR `parameters.prompt === 'login'`
+  #   Row 6 — every binding's `account_device.updated_at` is older than
+  #           upstream's authenticationMaxAge (7 days)
+  #   Row 9 — `login_hint` resolves to a binding that's individually stale
+  #           even though other bindings on the device are fresh; upstream
+  #           pre-selects the hinted account and clicking it lands on
+  #           sign-in-view
+  # ---------------------------------------------------------------------------
+
+  Scenario: Row 5 — prompt=login forces every binding loginRequired
+    # The demo's "Force re-authentication" checkbox sets prompt=login on
+    # both the auth-service redirect query AND the PAR body. Auth-service's
+    # `shouldReuseSession` honours the query and serves the OTP form rather
+    # than redirecting to pds-core (session-reuse.ts:158, isForceLoginPrompt).
+    # The user completes OTP, and pds-core's epds-callback redirects to
+    # pds-core's own /oauth/authorize?request_uri=... with the now-fresh
+    # device cookies. At THAT hop the welcome-page guard fires:
+    #   - cookie pair is valid (just set by the callback)
+    #   - the device has one binding (the user just authenticated)
+    #   - the stored PAR `parameters.prompt` is still 'login' (the demo also
+    #     set it in the PAR body)
+    # Today the guard passes through; upstream's authorize() reads the
+    # stored PAR, marks the only session loginRequired, and the frontend
+    # renders sign-in-view. The guard must instead bounce back to
+    # auth-service for another OTP round.
+    When the demo client starts a new OAuth flow with prompt=login
+    And the user completes the OTP flow
+    Then the browser lands on the auth-service email-and-OTP form
+    And no upstream password field is rendered anywhere on the page
+
+  # Reproducing row 6 needs server-side white-box access to backdate
+  # `account_device.updated_at` past upstream's authenticationMaxAge
+  # (7 days). A pds-core /_internal/test/expire-device-account hook
+  # provides this, gated on EPDS_TEST_HOOKS=1 && NODE_ENV !== 'production'
+  # (mirroring auth-service's expire-otp / expire-auth-flow hooks).
+  Scenario: Row 6 — every binding's auth age is older than 7 days
+    Given the device's account_device row has been backdated past 7 days
+    When the demo client starts a new OAuth flow
+    Then the browser lands on the auth-service email-and-OTP form
+    And no upstream password field is rendered anywhere on the page
+
+  # Row 9 needs two bindings on the same device — one stale, one fresh —
+  # plus a login_hint that resolves to the stale one. Built on the same
+  # /_internal/test/expire-device-account hook used for row 6, plus a
+  # second OAuth sign-in driven within the SAME browser context (so the
+  # second account's upsertDeviceAccount binds to the existing dev-id
+  # rather than creating a fresh device).
+  Scenario: Row 9 — login_hint resolves to a stale binding on a multi-account device
+    Given the device has two bound accounts
+    And the test user's account_device row has been backdated past 7 days
+    And the other user's account_device row is fresh
+    When the demo client starts a new OAuth flow with the test user's handle as login_hint
+    Then the browser lands on the auth-service email-and-OTP form
+    And no upstream password field is rendered anywhere on the page

packages/pds-core/src/__tests__/welcome-page-guard.test.ts

@@ -233,6 +233,14 @@ type FakeProvider = {
       readDevice: (id: string) => Promise<{ sessionId: string } | null>
     }
   }
+  requestManager: {
+    store: {
+      readRequest: (
+        id: string,
+      ) => Promise<{ parameters?: Record<string, unknown> } | null>
+    }
+  }
+  checkLoginRequired: (binding: unknown) => boolean
 }
 
 /** Build a FakeProvider whose deviceStore returns a row matching the
@@ -242,6 +250,15 @@ function makeProvider(opts: {
   bindings?: () => Promise<unknown[]>
   sessionId?: string | null
   readDevice?: () => Promise<{ sessionId: string } | null>
+  // Stored PAR for the request_uri on the test URL. Returned shape mirrors
+  // what `(provider.requestManager as any).store.readRequest(id)` produces:
+  // a `{ parameters: { prompt?, login_hint?, ... } }` envelope.
+  readRequest?: (
+    id: string,
+  ) => Promise<{ parameters?: Record<string, unknown> } | null>
+  // Per-binding loginRequired predicate. Default: false (everything fresh).
+  // Tests for rows 6/9 supply a custom predicate.
+  checkLoginRequired?: (binding: unknown) => boolean
 }): FakeProvider {
   const ses = opts.sessionId === undefined ? VALID_SES : opts.sessionId
   return {
@@ -256,6 +273,12 @@ function makeProvider(opts: {
         ),
       },
     },
+    requestManager: {
+      store: {
+        readRequest: vi.fn(opts.readRequest ?? (() => Promise.resolve(null))),
+      },
+    },
+    checkLoginRequired: vi.fn(opts.checkLoginRequired ?? (() => false)),
   }
 }