Fix OAuth account enrichment with tooltip explanation

.changeset/sign-in-account-presentation.md

@@ -0,0 +1,15 @@
+---
+'ePDS': patch
+---
+
+Sign-in screens now show your email more consistently and wait until they are ready before accepting clicks.
+
+**Affects:** End users, Client app developers
+
+**End users:**
+
+- Sign-in, app approval, account chooser, and account-management screens now use email as the primary identifier for accounts with generated handles, while still showing the public handle where it helps explain the account.
+- The final approval step no longer briefly exposes a generated random handle when an app asked to show email first.
+- The email sign-in button now waits until the page is ready before accepting clicks, so an early tap does not start a broken sign-in attempt.
+
+**Client app developers:** `epds_handle_mode` is now applied consistently to approval and account-chooser pages from either the current `/oauth/authorize` query parameter or OAuth client metadata, including pushed authorization requests where the browser URL only contains `request_uri`. Explicit query parameters still take precedence over client metadata. Valid modes stored for an auth flow are preserved through `/oauth/epds-callback`; invalid callback values are ignored rather than forwarded; metadata lookup failures fall back to the existing default behavior.

e2e/step-definitions/consent.steps.ts

@@ -9,6 +9,7 @@
 
 import { Given, Then, When } from '@cucumber/cucumber'
 import { expect } from '@playwright/test'
+import type { Locator, Page } from '@playwright/test'
 import type { EpdsWorld } from '../support/world.js'
 import { testEnv } from '../support/env.js'
 import {
@@ -35,6 +36,66 @@ import { fillOtp } from '../support/otp.js'
 // Note: When('the user clicks {string}') lives in common.steps.ts — it is a
 // generic UI interaction step used here for "Authorize" and "Deny access" buttons.
 
+function requireScenarioEmail(world: EpdsWorld): string {
+  if (!world.testEmail) {
+    throw new Error(
+      'No test email set — "a returning user has a PDS account" step must run first',
+    )
+  }
+  return world.testEmail
+}
+
+function requireScenarioHandle(world: EpdsWorld): string {
+  if (!world.userHandle) {
+    throw new Error(
+      'No user handle set — "a returning user has a PDS account" step must run first',
+    )
+  }
+  return world.userHandle
+}
+
+function formatPublicHandle(handle: string): string {
+  return handle.startsWith('@') ? handle : `@${handle}`
+}
+
+function formatRawPublicHandle(handle: string): string {
+  return handle.startsWith('@') ? handle.slice(1) : handle
+}
+
+function escapeRegex(value: string): string {
+  return value.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`)
+}
+
+async function openIdentityTooltip(page: Page): Promise<Locator> {
+  // .first(): enrichment walks every matching identity node, so a consent
+  // page rendering more than one approved phrasing ("Grant access to your
+  // X account" / "... wants to access your X account") gets an icon on
+  // each. Without this the locator resolves to several elements and
+  // Playwright throws a strict-mode error. Each icon carries its own
+  // aria-describedby, so asserting against the first is well-defined.
+  const tooltipControl = page
+    .getByRole('main')
+    .getByRole('button', { name: 'Identity information' })
+    .first()
+
+  await expect(tooltipControl).toHaveAttribute('type', 'button')
+  await expect(tooltipControl).toHaveAttribute('aria-expanded', 'false')
+  const describedBy = await tooltipControl.getAttribute('aria-describedby')
+  expect(describedBy?.trim()).toBeTruthy()
+  if (!describedBy?.trim()) {
+    throw new Error('Expected aria-describedby to reference a tooltip')
+  }
+  const [tooltipId] = describedBy.trim().split(/\s+/)
+
+  await tooltipControl.click()
+  await expect(tooltipControl).toHaveAttribute('aria-expanded', 'true')
+
+  const tooltip = page.locator(`#${tooltipId}`)
+  await expect(tooltip).toHaveAttribute('role', 'tooltip')
+  await expect(tooltip).toBeVisible()
+  return tooltip
+}
+
 Then('a consent screen is displayed', async function (this: EpdsWorld) {
   const page = getPage(this)
 
@@ -103,6 +164,80 @@ When(
   },
 )
 
+When(
+  'the untrusted demo client starts a new OAuth flow with random handle mode',
+  async function (this: EpdsWorld) {
+    if (!testEnv.demoUntrustedUrl) return 'pending'
+    const page = getPage(this)
+    const base = testEnv.demoUntrustedUrl.replace(/\/$/, '')
+    await page.goto(`${base}/flow3`)
+    await page.click('button[type=submit]')
+  },
+)
+
+Then(
+  'the consent page shows the email as the primary account identifier',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    const scenarioEmail = requireScenarioEmail(this)
+    const grantAccessText = /\bGrant\s+access\s+to\s+your\b/
+    const accountCardText = /\bwants\s+to\s+access\s+your\b/
+    const grantAccessParagraph = page.getByText(grantAccessText).first()
+    const accountCard = page
+      .getByRole('main')
+      .getByText(accountCardText)
+      .first()
+
+    await expect(grantAccessParagraph).toBeVisible()
+    await expect(grantAccessParagraph).toContainText(scenarioEmail)
+    await expect(accountCard).toBeVisible()
+    await expect(accountCard).toContainText(scenarioEmail)
+  },
+)
+
+Then(
+  'the consent identity tooltip exposes the public AT Protocol handle',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    const publicHandle = formatPublicHandle(requireScenarioHandle(this))
+    const tooltip = await openIdentityTooltip(page)
+    await expect(tooltip).toContainText('Public AT Protocol handle:')
+    await expect(tooltip).toContainText(publicHandle)
+  },
+)
+
+Then(
+  'the consent identity tooltip exposes the account email',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    const scenarioEmail = requireScenarioEmail(this)
+    const tooltip = await openIdentityTooltip(page)
+    await expect(tooltip).toContainText('This handle is associated with')
+    await expect(tooltip).toContainText(scenarioEmail)
+  },
+)
+
+Then(
+  'the public handle is not shown as the primary consent identifier',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    const scenarioHandle = requireScenarioHandle(this)
+    const primaryHandlePatterns = [
+      formatPublicHandle(scenarioHandle),
+      formatRawPublicHandle(scenarioHandle),
+    ].map(
+      (publicHandle) =>
+        new RegExp(
+          String.raw`\byour\s+${escapeRegex(publicHandle)}\s+account\b`,
+        ),
+    )
+
+    for (const primaryHandlePattern of primaryHandlePatterns) {
+      await expect(page.getByText(primaryHandlePattern)).toHaveCount(0)
+    }
+  },
+)
+
 Then(
   'the browser is redirected back to the untrusted demo client with an auth error',
   async function (this: EpdsWorld) {

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

@@ -320,8 +320,8 @@ When(
     // pds-core's chooser middleware reads to inject
     // <meta name="epds-handle-mode" content="random"> into the chooser's
     // <head>. The enrichment script reads that meta and hides the handle
-    // span (display:none on .epds-handle-label, title= on
-    // .epds-email-label) without touching the DB or the account's actual
+    // span (display:none on .epds-handle-label) and describes it through
+    // aria-describedby without touching the DB or the account's actual
     // stored handle.
     const page = getPage(this)
     const base = testEnv.demoTrustedUrl.replace(/\/$/, '')
@@ -417,25 +417,100 @@ Then(
   },
 )
 
+type HiddenHandleDescriptionRow = {
+  describedBy: string | null
+  descriptions: {
+    id: string
+    isHiddenHandleDescription: boolean
+    text: string
+  }[]
+  emailTitle: string | null
+  hiddenHandleText: string
+  rowIndex: number
+}
+
 Then(
-  'each row exposes the handle only via a title tooltip',
+  'each row exposes the hidden handle through an accessible description',
   async function (this: EpdsWorld) {
     const page = getPage(this)
-    // The script copies the hidden handle span's text into a title=
-    // attribute on the adjacent .epds-email-label so power-users can
-    // still inspect which account maps to which DID without the
-    // gibberish random handle cluttering the visual hierarchy.
-    const emailLabels = page.locator('.epds-email-label')
-    const count = await emailLabels.count()
-    expect(count).toBeGreaterThan(0)
-    for (let i = 0; i < count; i++) {
-      const title = await emailLabels.nth(i).getAttribute('title')
-      const titleRepr = title === null ? 'null' : `"${title}"`
+    await expect(page.locator('.epds-email-label').first()).toBeVisible({
+      timeout: 10_000,
+    })
+
+    const rows = await page
+      .locator('.epds-email-label')
+      .evaluateAll((emailLabels): HiddenHandleDescriptionRow[] => {
+        return emailLabels
+          .map((emailLabel, rowIndex) => {
+            const row = emailLabel.closest('[aria-label]')
+            const handleLabel = row?.querySelector('.epds-handle-label')
+            if (!row || !handleLabel) return null
+
+            const handleIsHidden =
+              globalThis.getComputedStyle(handleLabel).display === 'none'
+            if (!handleIsHidden) return null
+
+            const hiddenHandleText = handleLabel.textContent?.trim() ?? ''
+            const describedBy = row.getAttribute('aria-describedby')
+            const descriptionIds =
+              describedBy?.trim().split(/\s+/).filter(Boolean) ?? []
+            const descriptions = descriptionIds.map((id) => {
+              const describedElement = document.getElementById(id)
+              return {
+                id,
+                isHiddenHandleDescription:
+                  describedElement?.classList.contains(
+                    'epds-hidden-handle-description',
+                  ) ?? false,
+                text: describedElement?.textContent?.trim() ?? '',
+              }
+            })
+
+            return {
+              describedBy,
+              descriptions,
+              emailTitle: emailLabel.getAttribute('title'),
+              hiddenHandleText,
+              rowIndex,
+            }
+          })
+          .filter((row): row is HiddenHandleDescriptionRow => row !== null)
+      })
+
+    expect(rows.length).toBeGreaterThan(0)
+    for (const row of rows) {
       expect(
-        title,
-        `Row ${i}: expected .epds-email-label to carry the hidden handle as title=, got ${titleRepr}`,
+        row.describedBy,
+        `Row ${row.rowIndex}: expected chooser row to reference the hidden handle with aria-describedby`,
       ).toBeTruthy()
-      expect(title!.trim().length).toBeGreaterThan(0)
+
+      const description = row.descriptions.find(
+        (candidate) => candidate.isHiddenHandleDescription,
+      )
+      expect(
+        description,
+        `Row ${row.rowIndex}: expected aria-describedby to reference an .epds-hidden-handle-description element`,
+      ).toBeDefined()
+
+      const descriptionText = description?.text ?? ''
+      const prefix = 'Underlying handle:'
+      const prefixIndex = descriptionText.indexOf(prefix)
+      expect(
+        prefixIndex,
+        `Row ${row.rowIndex}: expected hidden-handle description to contain "${prefix}", got "${descriptionText}"`,
+      ).toBeGreaterThanOrEqual(0)
+      const describedHiddenHandle = descriptionText
+        .slice(prefixIndex + prefix.length)
+        .trim()
+      expect(
+        describedHiddenHandle,
+        `Row ${row.rowIndex}: expected hidden-handle description suffix to match the hidden handle text`,
+      ).toBe(row.hiddenHandleText)
+
+      expect(
+        row.emailTitle,
+        `Row ${row.rowIndex}: .epds-email-label should not expose the hidden handle through title=`,
+      ).toBeNull()
     }
   },
 )

features/consent-screen.feature

@@ -72,6 +72,29 @@ Feature: OAuth consent screen
     When the user later initiates an OAuth login via the untrusted demo client
     Then a consent screen is displayed
 
+  @untrusted-client @email
+  Scenario: Default picker consent tooltip shows email associated with the public handle
+    Given a returning user has a PDS account
+    When the untrusted demo client initiates an OAuth login
+    And the user enters the test email on the login page
+    And an OTP email arrives in the mail trap
+    And the user enters the OTP code
+    Then a consent screen is displayed
+    And it identifies the untrusted demo client by its URL host
+    And the consent identity tooltip exposes the account email
+
+  @untrusted-client @email
+  Scenario: Random-handle consent shows email with public handle in identity tooltip
+    Given a returning user has a PDS account
+    When the untrusted demo client starts a new OAuth flow with random handle mode
+    And the user enters the test email on the login page
+    And an OTP email arrives in the mail trap
+    And the user enters the OTP code
+    Then a consent screen is displayed
+    And the consent page shows the email as the primary account identifier
+    And the consent identity tooltip exposes the public AT Protocol handle
+    And the public handle is not shown as the primary consent identifier
+
   # TODO: automate once custom CSS injection is merged into the consent route
   # (renderConsent() needs to accept and apply clientBrandingCss from client metadata)
   @manual

features/session-reuse-bugs.feature

@@ -79,7 +79,7 @@ Feature: Welcome-page guard suppresses upstream's authentication UI
     When the demo client starts a new OAuth flow with random handle mode
     Then the browser lands on the ePDS enriched account picker
     And the enriched account picker renders without the handle visible
-    And each row exposes the handle only via a title tooltip
+    And each row exposes the hidden handle through an accessible description
     And the email remains visible as the primary identifier
 
   @pending