Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions .changeset/recovery-link-uses-real-request-uri.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'ePDS': patch
---

The "Recover with backup email" link no longer breaks the "Back to sign in" path.

**Affects:** End users

**End users:** clicking **Recover with backup email** on the sign-in page used to leave you on a recovery flow that couldn't return you to the original sign-in: hitting **Back to sign in** at the end landed on a "data you submitted is invalid" error page from the underlying OAuth machinery, because the link forwarded a placeholder URL instead of the real sign-in context. The link now carries the active sign-in's actual context, so the back path round-trips cleanly even if you decide not to use recovery after all.
60 changes: 60 additions & 0 deletions e2e/step-definitions/account-recovery.steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,63 @@ Then(
await assertNoEmailFor(this.backupEmail)
},
)

Then(
"the recovery link points at the active OAuth flow's request_uri",
async function (this: EpdsWorld) {
const page = getPage(this)
const flowUrl = new URL(page.url())
const expectedRequestUri = flowUrl.searchParams.get('request_uri')
if (!expectedRequestUri) {
throw new Error(`Current page has no request_uri: ${page.url()}`)
}
const recoveryHref = await page
.locator('#recovery-link')
.getAttribute('href')
if (!recoveryHref) throw new Error('Recovery link not found on page')
const linkRequestUri = new URL(
recoveryHref,
flowUrl.origin,
).searchParams.get('request_uri')
if (linkRequestUri !== expectedRequestUri) {
throw new Error(
`Recovery link request_uri mismatch:\n expected: ${expectedRequestUri}\n got: ${linkRequestUri}`,
)
}
this.lastRequestUri = expectedRequestUri
},
)

When(
'the user follows the recovery link and then returns to sign in',
async function (this: EpdsWorld) {
const page = getPage(this)
await page.locator('#recovery-link').click()
await expect(page).toHaveURL(/\/auth\/recover\?request_uri=/)
await expect(
page.getByRole('link', { name: 'Back to sign in' }),
).toBeVisible()
await page.getByRole('link', { name: 'Back to sign in' }).click()
},
)

Then(
"the original OAuth flow's sign-in page is restored",
async function (this: EpdsWorld) {
if (!this.lastRequestUri) {
throw new Error('No original request_uri was captured')
}
const page = getPage(this)
await expect(page).toHaveURL(/\/oauth\/authorize\?/)
const returnedRequestUri = new URL(page.url()).searchParams.get(
'request_uri',
)
expect(returnedRequestUri).toBe(this.lastRequestUri)
// PAR hint lookup is not guaranteed to be repeatable, so the returning
// page may show either the email or OTP step. The round-trip contract is
// that the original live request is restored instead of an OAuth error.
await expect(
page.locator('#form-send-otp:visible, #form-verify-otp:visible'),
).toHaveCount(1)
},
)
11 changes: 11 additions & 0 deletions features/account-recovery.feature
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ Feature: Account recovery via backup emails
Then the recovery OTP form is displayed
And no email arrives for that non-existent address

# The recovery link must carry the active authorization request rather
# than a placeholder. Exercise the complete browser round trip so the
# test proves Back to sign in restores the live OTP flow.
Scenario: Recovery link round-trips to the active OAuth flow
Given the demo client initiates OAuth with the test email as login_hint
Then an OTP email arrives in the mail trap
And the login page shows an OTP verification form
And the recovery link points at the active OAuth flow's request_uri
When the user follows the recovery link and then returns to sign in
Then the original OAuth flow's sign-in page is restored

# --- Backup email management ---

Scenario: User removes a backup email
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function renderLoginPageWithHeartbeat(heartbeatEnabled: boolean): string {
loginHint: '',
initialStep: 'email',
otpAlreadySent: false,
requestUri: 'urn:ietf:params:oauth:request_uri:req-abc',
csrfToken: 'csrf',
authBasePath: '/api/auth',
pdsPublicUrl: 'https://pds.example.com',
Expand Down
2 changes: 2 additions & 0 deletions packages/auth-service/src/__tests__/login-page.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ describe('renderLoginPage handle login button', () => {
loginHint: '',
initialStep: 'email',
otpAlreadySent: false,
requestUri: 'urn:ietf:params:oauth:request_uri:test-req',
csrfToken: 'csrf',
authBasePath: '/api/auth',
pdsPublicUrl: 'https://pds.example.com',
Expand Down Expand Up @@ -512,6 +513,7 @@ function renderDefault(): string {
loginHint: '',
initialStep: 'email',
otpAlreadySent: false,
requestUri: 'urn:ietf:params:oauth:request_uri:test-req',
csrfToken: 'csrf',
authBasePath: '/api/auth',
pdsPublicUrl: 'https://pds.example.com',
Expand Down
11 changes: 10 additions & 1 deletion packages/auth-service/src/routes/login-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router {
loginHint: emailHint,
initialStep,
otpAlreadySent,
requestUri,
csrfToken: res.locals.csrfToken,
authBasePath: '/api/auth',
pdsPublicUrl: ctx.config.pdsPublicUrl,
Expand Down Expand Up @@ -476,6 +477,14 @@ export function renderLoginPage(opts: {
loginHint: string
initialStep: 'email' | 'otp'
otpAlreadySent: boolean
/**
* The actual PAR request_uri that started this flow. Forwarded to
* the recovery link's query string so the recovery page can
* round-trip the user back to their original /oauth/authorize on
* "Back to sign in". Previously the link used a placeholder URL,
* which silently broke the back-to-sign-in path after recovery.
*/
requestUri: string
csrfToken: string
authBasePath: string
pdsPublicUrl: string
Expand Down Expand Up @@ -700,7 +709,7 @@ export function renderLoginPage(opts: {
<button type="button" class="btn-secondary" id="btn-resend">Resend code</button>
<button type="button" class="btn-secondary" id="btn-back">Use different email</button>
</div>
<a href="/auth/recover?request_uri=${encodeURIComponent(opts.pdsPublicUrl + '/placeholder')}"
<a href="/auth/recover?request_uri=${encodeURIComponent(opts.requestUri)}"
class="recovery-link" id="recovery-link">Recover with backup email</a>
</div>
</div>
Expand Down
7 changes: 7 additions & 0 deletions packages/auth-service/src/routes/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ export function createPreviewRouter(ctx: AuthServiceContext): Router {
loginHint: '',
initialStep: 'email',
otpAlreadySent: false,
// Preview pages don't sit behind a real OAuth flow — pass a
// recognisable fake request_uri so the recovery link renders
// something shaped right without pretending to point at a
// live PAR.
requestUri: 'urn:ietf:params:oauth:request_uri:preview',
csrfToken: fakeCsrfToken(),
authBasePath: '/api/auth',
pdsPublicUrl: ctx.config.pdsPublicUrl,
Expand Down Expand Up @@ -246,6 +251,8 @@ export function createPreviewRouter(ctx: AuthServiceContext): Router {
loginHint: FAKE_EMAIL,
initialStep: 'otp',
otpAlreadySent: true,
// See preview/login note above — fake request_uri.
requestUri: 'urn:ietf:params:oauth:request_uri:preview',
csrfToken: fakeCsrfToken(),
authBasePath: '/api/auth',
pdsPublicUrl: ctx.config.pdsPublicUrl,
Expand Down
Loading