Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
volumes:
- pds-data:/data
restart: unless-stopped
Expand Down
2 changes: 1 addition & 1 deletion docs/design/cross-client-session-reuse.md
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ The auto-skip predicate is, with all conditions required:
1. `client_id` is on `PDS_OAUTH_TRUSTED_CLIENTS`.
2. Client metadata advertises `epds_skip_chooser_on_match: true`
(per-client opt-in; default off).
3. The welcome-page-guard's existing checks pass:
3. The auth-ui-guard's existing checks pass:
- `dev-id`/`ses-id` cookies parse and ses-id matches the device
row's active sessionId.
- `accountManager.listDeviceAccounts(deviceId)` returns at least
Expand Down
2 changes: 1 addition & 1 deletion docs/design/pds-white-boxing.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ pds-core back to the auth-service in a loop.

### 18. Welcome-page guard pre-routes `/oauth/authorize` and `/account*`

**File:** `packages/pds-core/src/welcome-page-guard.ts`, wired in
**File:** `packages/pds-core/src/auth-ui-guard.ts`, wired in
`packages/pds-core/src/index.ts`

A pre-route Express middleware intercepts `GET /oauth/authorize` and
Expand Down
2 changes: 1 addition & 1 deletion e2e/step-definitions/auth.steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ Then(
)

Then(
'the browser is redirected back to the demo client with a valid session',
"the demo client's welcome page confirms the user is signed in",
async function (this: EpdsWorld) {
await assertDemoClientSession(this)
},
Expand Down
180 changes: 172 additions & 8 deletions e2e/step-definitions/session-reuse-bugs.steps.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
},
Expand Down Expand Up @@ -193,6 +196,29 @@ When(
},
)

When(
"the demo client starts a new OAuth flow with the test user's handle as a PAR-body login_hint",
async function (this: EpdsWorld) {
if (!this.userHandle) {
throw new Error(
'world.userHandle missing — Background step must run first',
)
}
// login_hint_location=body puts the hint in the PAR body rather than the
// /oauth/authorize redirect URL — this matches the third-party
// OAuth-client pattern that drives upstream's matchesHint logic against
// `parameters.login_hint` from the stored PAR. Without this, upstream's
// request-manager-loaded parameters carry no hint and the guard's row 9
// bounce condition can never fire (because filterCandidateBindings sees
// every binding as a candidate, not just the stale one).
const page = getPage(this)
const url = new URL('/api/oauth/login', testEnv.demoUrl)
url.searchParams.set('login_hint', this.userHandle)
url.searchParams.set('login_hint_location', 'body')
await page.goto(url.toString())
},
)

Given(
'another user has a separate PDS account',
async function (this: EpdsWorld) {
Expand Down Expand Up @@ -332,7 +358,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(
Expand Down Expand Up @@ -564,3 +598,133 @@ 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())
},
)

// 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',
)
}
},
)
2 changes: 1 addition & 1 deletion features/account-recovery.feature
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Feature: Account recovery via backup emails
And the user enters the backup email on the recovery page
Then an OTP email arrives in the mail trap for the backup email
When the user enters the recovery OTP
Then the browser is redirected back to the demo client with a valid session
Then the demo client's welcome page confirms the user is signed in

Scenario: Recovery with non-existent email shows same UI (anti-enumeration)
Given the demo client initiates OAuth with the test email as login_hint
Expand Down
4 changes: 2 additions & 2 deletions features/passwordless-authentication.feature
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Feature: Passwordless authentication via email OTP
Then an OTP email arrives in the mail trap
And the email subject contains "ePDS Demo"
When the user enters the OTP code
Then the browser is redirected back to the demo client with a valid session
Then the demo client's welcome page confirms the user is signed in

@email
Scenario: Returning user who has already approved skips consent
Expand All @@ -44,7 +44,7 @@ Feature: Passwordless authentication via email OTP
Then an OTP email arrives in the mail trap
And the email subject contains "ePDS Demo"
When the user enters the OTP code
Then the browser is redirected back to the demo client with a valid session
Then the demo client's welcome page confirms the user is signed in

# --- Session reuse across OAuth clients (HYPER-268) ---
#
Expand Down
Loading
Loading