From 3fb407bd901af26c4cd3499fef087acc84363c9a Mon Sep 17 00:00:00 2001 From: kzoeps Date: Fri, 1 May 2026 16:40:12 +0600 Subject: [PATCH 01/12] fix(pds-core): scope chooser enrichment to account rows --- .../src/__tests__/chooser-enrichment.test.ts | 277 ++++++++++++++++++ packages/pds-core/src/chooser-enrichment.ts | 3 + 2 files changed, 280 insertions(+) diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index 53ae36bf..1eef294b 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -30,6 +30,283 @@ describe('buildChooserEnrichmentScript (HYPER-268)', () => { }) }) +class FakeTextNode { + readonly nodeType = 3 + + constructor(readonly data: string) {} +} + +class FakeClassList { + private readonly classes = new Set() + + add(className: string): void { + this.classes.add(className) + } + + contains(className: string): boolean { + return this.classes.has(className) + } + + set(value: string): void { + this.classes.clear() + for (const className of value.split(/\s+/)) { + if (className) this.classes.add(className) + } + } +} + +class FakeElement { + readonly nodeType = 1 + readonly childNodes: Array = [] + readonly dataset: Record = {} + readonly classList = new FakeClassList() + readonly style: Record = {} + parentElement: FakeElement | null = null + textContentOverride: string | null = null + + constructor( + readonly tagName: string, + private readonly attributes: Record = {}, + ) {} + + appendChild(child: FakeElement | FakeTextNode): void { + if (child instanceof FakeElement) { + child.parentElement = this + } + this.childNodes.push(child) + } + + set textContent(value: string) { + this.textContentOverride = value + } + + set className(value: string) { + this.classList.set(value) + } + + get textContent(): string { + if (this.textContentOverride !== null) return this.textContentOverride + return this.childNodes + .map((child) => + child instanceof FakeTextNode ? child.data : child.textContent, + ) + .join('') + } + + closest(selector: string): FakeElement | null { + if (selector !== '[role="button"][tabindex="0"]') return null + let current: FakeElement | null = this + while (current) { + if ( + current.attributes.role === 'button' && + current.attributes.tabindex === '0' + ) { + return current + } + current = current.parentElement + } + return null + } + + querySelectorAll(selector: string): FakeElement[] { + const descendants = this.descendants() + if (selector === 'button, a') { + return descendants.filter( + (el) => el.tagName === 'button' || el.tagName === 'a', + ) + } + if (selector === '[role="button"]') { + return descendants.filter((el) => el.attributes.role === 'button') + } + return [] + } + + querySelector(selector: string): FakeElement | null { + if ( + selector === + '[role="button"][aria-label="Login to account that is not listed"]' + ) { + return ( + this.descendants().find( + (el) => + el.attributes.role === 'button' && + el.attributes['aria-label'] === 'Login to account that is not listed', + ) ?? null + ) + } + return null + } + + descendants(): FakeElement[] { + const result: FakeElement[] = [] + const visit = (node: FakeElement): void => { + for (const child of node.childNodes) { + if (child instanceof FakeElement) { + result.push(child) + visit(child) + } + } + } + visit(this) + return result + } +} + +class FakeDocument { + readonly root = new FakeElement('div', { id: 'root' }) + readyState = 'loading' + private domContentLoadedListener: (() => void) | null = null + + get documentElement(): FakeElement { + return this.root + } + + getElementById(id: string): FakeElement | null { + return id === 'root' ? this.root : null + } + + createTreeWalker(root: FakeElement): { nextNode: () => FakeElement | null } { + const elements = root.descendants() + let index = 0 + return { + nextNode: () => elements[index++] ?? null, + } + } + + createElement(tagName: string): FakeElement { + return new FakeElement(tagName) + } + + querySelector(): null { + return null + } + + addEventListener(event: string, listener: () => void): void { + if (event === 'DOMContentLoaded') this.domContentLoadedListener = listener + } + + dispatchDOMContentLoaded(): void { + this.readyState = 'complete' + this.domContentLoadedListener?.() + } +} + +function appendText(parent: FakeElement, text: string): void { + parent.appendChild(new FakeTextNode(text)) +} + +function runChooserEnrichmentScript(document: FakeDocument): void { + const globals = globalThis as typeof globalThis & Record + const previous = { + window: globals.window, + document: globals.document, + Node: globals.Node, + NodeFilter: globals.NodeFilter, + MutationObserver: globals.MutationObserver, + } + const fakeWindow: Record = { + location: { search: '' }, + } + + try { + globals.window = fakeWindow + globals.document = document + globals.Node = { TEXT_NODE: 3 } + globals.NodeFilter = { SHOW_ELEMENT: 1 } + globals.MutationObserver = class { + observe(): void {} + } + + new Function(buildChooserEnrichmentScript())() + fakeWindow.__sessions = [ + { + account: { + sub: 'did:plc:alice', + email: 'alice@example.test', + preferred_username: 'alice.test', + }, + }, + { + account: { + sub: 'did:plc:bob', + email: 'bob@example.test', + preferred_username: 'bob.test', + }, + }, + ] + document.dispatchDOMContentLoaded() + } finally { + globals.window = previous.window + globals.document = previous.document + globals.Node = previous.Node + globals.NodeFilter = previous.NodeFilter + globals.MutationObserver = previous.MutationObserver + } +} + +describe('buildChooserEnrichmentScript account row scoping', () => { + it('does not enrich consent copy outside a chooser account row', () => { + const document = new FakeDocument() + const paragraph = new FakeElement('p') + const consentHandle = new FakeElement('span') + appendText(consentHandle, 'alice.test') + paragraph.appendChild(consentHandle) + appendText(paragraph, ' grants access to did:plc:alice') + document.root.appendChild(paragraph) + + runChooserEnrichmentScript(document) + + expect(document.root.descendants()).not.toContainEqual( + expect.objectContaining({ textContentOverride: 'alice@example.test' }), + ) + expect(consentHandle.classList.contains('epds-handle-label')).toBe(false) + }) + + it('enriches a chooser-like account row', () => { + const document = new FakeDocument() + const row = new FakeElement('div', { role: 'button', tabindex: '0' }) + const wrap = new FakeElement('span') + const handle = new FakeElement('span') + appendText(handle, 'alice.test') + wrap.appendChild(handle) + row.appendChild(wrap) + document.root.appendChild(row) + + runChooserEnrichmentScript(document) + + const emailLabel = wrap.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-email-label'), + ) as FakeElement | undefined + + expect(emailLabel).toBeInstanceOf(FakeElement) + expect(emailLabel?.textContent).toBe('alice@example.test') + expect(handle.classList.contains('epds-handle-label')).toBe(true) + }) + + it('enriches multiple chooser-like account rows', () => { + const document = new FakeDocument() + const rows = ['alice.test', 'bob.test'].map((handleText) => { + const row = new FakeElement('div', { role: 'button', tabindex: '0' }) + const wrap = new FakeElement('span') + const handle = new FakeElement('span') + appendText(handle, handleText) + wrap.appendChild(handle) + row.appendChild(wrap) + document.root.appendChild(row) + return { wrap, handle } + }) + + runChooserEnrichmentScript(document) + + expect(rows[0].wrap.textContent).toContain('alice@example.test') + expect(rows[1].wrap.textContent).toContain('bob@example.test') + expect(rows[0].handle.classList.contains('epds-handle-label')).toBe(true) + expect(rows[1].handle.classList.contains('epds-handle-label')).toBe(true) + }) +}) + describe('sha256Base64', () => { it('produces a stable SHA256 base64 hash', () => { // Known value for the empty string. diff --git a/packages/pds-core/src/chooser-enrichment.ts b/packages/pds-core/src/chooser-enrichment.ts index 6a38778f..dbf76ecd 100644 --- a/packages/pds-core/src/chooser-enrichment.ts +++ b/packages/pds-core/src/chooser-enrichment.ts @@ -180,6 +180,9 @@ export function buildChooserEnrichmentScript(): string { if (email) matches.push({ el: node, email: email }); } matches.forEach(function(m) { + // oauth-provider-ui 0.4.3 renders chooser rows this way; revisit if the upstream PDS UI is upgraded. + var row = m.el.closest('[role="button"][tabindex="0"]'); + if (!row) return; // Upstream wraps the handle span in a flex-row container: // // HANDLE From 2acbc43a4b0b652cffec2c4e28ae32df6d015193 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Sat, 2 May 2026 17:49:41 +0600 Subject: [PATCH 02/12] test(pds-core): fix chooser enrichment test lint --- .../src/__tests__/chooser-enrichment.test.ts | 79 +++++++++---------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index 1eef294b..bf680cc7 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -1,3 +1,4 @@ +import { runInNewContext } from 'node:vm' import { describe, expect, it, vi } from 'vitest' import { appendScriptHashToCsp, @@ -95,7 +96,12 @@ class FakeElement { closest(selector: string): FakeElement | null { if (selector !== '[role="button"][tabindex="0"]') return null - let current: FakeElement | null = this + + if (this.attributes.role === 'button' && this.attributes.tabindex === '0') { + return this + } + + let current = this.parentElement while (current) { if ( current.attributes.role === 'button' && @@ -130,7 +136,8 @@ class FakeElement { this.descendants().find( (el) => el.attributes.role === 'button' && - el.attributes['aria-label'] === 'Login to account that is not listed', + el.attributes['aria-label'] === + 'Login to account that is not listed', ) ?? null ) } @@ -196,52 +203,42 @@ function appendText(parent: FakeElement, text: string): void { } function runChooserEnrichmentScript(document: FakeDocument): void { - const globals = globalThis as typeof globalThis & Record - const previous = { - window: globals.window, - document: globals.document, - Node: globals.Node, - NodeFilter: globals.NodeFilter, - MutationObserver: globals.MutationObserver, - } const fakeWindow: Record = { location: { search: '' }, } + const sandbox = { + document, + MutationObserver: class { + observed = false - try { - globals.window = fakeWindow - globals.document = document - globals.Node = { TEXT_NODE: 3 } - globals.NodeFilter = { SHOW_ELEMENT: 1 } - globals.MutationObserver = class { - observe(): void {} - } + observe(): void { + this.observed = true + } + }, + Node: { TEXT_NODE: 3 }, + NodeFilter: { SHOW_ELEMENT: 1 }, + URLSearchParams, + window: fakeWindow, + } - new Function(buildChooserEnrichmentScript())() - fakeWindow.__sessions = [ - { - account: { - sub: 'did:plc:alice', - email: 'alice@example.test', - preferred_username: 'alice.test', - }, + runInNewContext(buildChooserEnrichmentScript(), sandbox) // NOSONAR — test executes only the deterministic script generated in this repository. + fakeWindow.__sessions = [ + { + account: { + sub: 'did:plc:alice', + email: 'alice@example.test', + preferred_username: 'alice.test', }, - { - account: { - sub: 'did:plc:bob', - email: 'bob@example.test', - preferred_username: 'bob.test', - }, + }, + { + account: { + sub: 'did:plc:bob', + email: 'bob@example.test', + preferred_username: 'bob.test', }, - ] - document.dispatchDOMContentLoaded() - } finally { - globals.window = previous.window - globals.document = previous.document - globals.Node = previous.Node - globals.NodeFilter = previous.NodeFilter - globals.MutationObserver = previous.MutationObserver - } + }, + ] + document.dispatchDOMContentLoaded() } describe('buildChooserEnrichmentScript account row scoping', () => { From b3ce75910a5c9d0d60d8f703c52f5c701a3cd1d1 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Mon, 4 May 2026 12:12:29 +0600 Subject: [PATCH 03/12] fix(pds-core): match chooser enrichment identifiers exactly --- .changeset/scoped-account-enrichment.md | 9 + .../src/__tests__/chooser-enrichment.test.ts | 636 +++++++++++++++++- packages/pds-core/src/chooser-enrichment.ts | 359 ++++++++-- 3 files changed, 949 insertions(+), 55 deletions(-) create mode 100644 .changeset/scoped-account-enrichment.md diff --git a/.changeset/scoped-account-enrichment.md b/.changeset/scoped-account-enrichment.md new file mode 100644 index 00000000..be7a2aa6 --- /dev/null +++ b/.changeset/scoped-account-enrichment.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Account choices are easier to recognize during sign-in and app approval. + +**Affects:** End users + +**End users:** Sign-in, app approval, and account-management screens now make it easier to identify accounts by email while preserving public handles where they are helpful. Authorization screens for generated-handle accounts show email as the primary identifier and keep the generated handle available through an accessible explanation. diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index bf680cc7..d2450410 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -62,8 +62,15 @@ class FakeElement { readonly dataset: Record = {} readonly classList = new FakeClassList() readonly style: Record = {} + id = '' + hidden = false + type = '' parentElement: FakeElement | null = null textContentOverride: string | null = null + private readonly eventListeners = new Map< + string, + Array<(event: FakeEvent) => void> + >() constructor( readonly tagName: string, @@ -77,6 +84,15 @@ class FakeElement { this.childNodes.push(child) } + insertAdjacentElement(position: string, element: FakeElement): void { + if (position !== 'afterend' || !this.parentElement) return + const siblings = this.parentElement.childNodes + const index = siblings.indexOf(this) + if (index === -1) return + element.parentElement = this.parentElement + siblings.splice(index + 1, 0, element) + } + set textContent(value: string) { this.textContentOverride = value } @@ -85,6 +101,15 @@ class FakeElement { this.classList.set(value) } + setAttribute(name: string, value: string): void { + this.attributes[name] = value + if (name === 'id') this.id = value + } + + getAttribute(name: string): string | null { + return this.attributes[name] ?? null + } + get textContent(): string { if (this.textContentOverride !== null) return this.textContentOverride return this.childNodes @@ -95,6 +120,17 @@ class FakeElement { } closest(selector: string): FakeElement | null { + if (selector === 'a') { + if (this.tagName === 'a') return this + + let current = this.parentElement + while (current) { + if (current.tagName === 'a') return current + current = current.parentElement + } + return null + } + if (selector !== '[role="button"][tabindex="0"]') return null if (this.attributes.role === 'button' && this.attributes.tabindex === '0') { @@ -114,6 +150,20 @@ class FakeElement { return null } + addEventListener(event: string, listener: (event: FakeEvent) => void): void { + const listeners = this.eventListeners.get(event) ?? [] + listeners.push(listener) + this.eventListeners.set(event, listeners) + } + + dispatchEvent(eventName: string): FakeEvent { + const event = new FakeEvent() + for (const listener of this.eventListeners.get(eventName) ?? []) { + listener(event) + } + return event + } + querySelectorAll(selector: string): FakeElement[] { const descendants = this.descendants() if (selector === 'button, a') { @@ -124,6 +174,9 @@ class FakeElement { if (selector === '[role="button"]') { return descendants.filter((el) => el.attributes.role === 'button') } + if (selector === 'h2') { + return descendants.filter((el) => el.tagName === 'h2') + } return [] } @@ -141,6 +194,22 @@ class FakeElement { ) ?? null ) } + if (selector === 'meta[name="epds-handle-mode"]') { + return ( + this.descendants().find( + (el) => + el.tagName === 'meta' && el.attributes.name === 'epds-handle-mode', + ) ?? null + ) + } + if (selector === 'meta[name="epds-auth-origin"]') { + return ( + this.descendants().find( + (el) => + el.tagName === 'meta' && el.attributes.name === 'epds-auth-origin', + ) ?? null + ) + } return null } @@ -159,6 +228,19 @@ class FakeElement { } } +class FakeEvent { + defaultPrevented = false + propagationStopped = false + + preventDefault(): void { + this.defaultPrevented = true + } + + stopPropagation(): void { + this.propagationStopped = true + } +} + class FakeDocument { readonly root = new FakeElement('div', { id: 'root' }) readyState = 'loading' @@ -184,8 +266,8 @@ class FakeDocument { return new FakeElement(tagName) } - querySelector(): null { - return null + querySelector(selector: string): FakeElement | null { + return this.root.querySelector(selector) } addEventListener(event: string, listener: () => void): void { @@ -202,9 +284,19 @@ function appendText(parent: FakeElement, text: string): void { parent.appendChild(new FakeTextNode(text)) } -function runChooserEnrichmentScript(document: FakeDocument): void { +function runChooserEnrichmentScript( + document: FakeDocument, + globals: { + __sessions?: unknown[] + __deviceSessions?: unknown[] + } = {}, + location: { pathname: string; search?: string } = { + pathname: '/oauth/authorize', + search: '', + }, +): void { const fakeWindow: Record = { - location: { search: '' }, + location, } const sandbox = { document, @@ -222,12 +314,14 @@ function runChooserEnrichmentScript(document: FakeDocument): void { } runInNewContext(buildChooserEnrichmentScript(), sandbox) // NOSONAR — test executes only the deterministic script generated in this repository. - fakeWindow.__sessions = [ + fakeWindow.__sessions = globals.__sessions ?? [ { + selected: true, account: { sub: 'did:plc:alice', email: 'alice@example.test', preferred_username: 'alice.test', + selected: true, }, }, { @@ -238,9 +332,120 @@ function runChooserEnrichmentScript(document: FakeDocument): void { }, }, ] + if (globals.__deviceSessions) { + fakeWindow.__deviceSessions = globals.__deviceSessions + } document.dispatchDOMContentLoaded() } +function createChooserRow( + document: FakeDocument, + identifierText: string, +): { row: FakeElement; wrap: FakeElement; identifier: FakeElement } { + const row = new FakeElement('div', { role: 'button', tabindex: '0' }) + const wrap = new FakeElement('span') + const identifier = new FakeElement('span') + appendText(identifier, identifierText) + wrap.appendChild(identifier) + row.appendChild(wrap) + document.root.appendChild(row) + return { row, wrap, identifier } +} + +function createAccountListRow( + document: FakeDocument, + identifierText: string, + { emptyTitle = false }: { emptyTitle?: boolean } = {}, +): { + anchor: FakeElement + title?: FakeElement + wrap: FakeElement + identifier: FakeElement +} { + const anchor = new FakeElement('a', { + href: '/account/did:plc:alice', + 'aria-label': 'View and manage account for alice.test', + }) + const wrap = new FakeElement('span') + const identifier = new FakeElement('span') + const title = emptyTitle ? new FakeElement('h2') : undefined + + if (title) anchor.appendChild(title) + appendText(identifier, identifierText) + wrap.appendChild(identifier) + anchor.appendChild(wrap) + document.root.appendChild(anchor) + return { anchor, title, wrap, identifier } +} + +function createAccountSelector( + document: FakeDocument, + identifierTexts: string[], +): { button: FakeElement; identifiers: FakeElement[]; wrap: FakeElement } { + const button = new FakeElement('button', { + 'aria-label': 'Select an account', + }) + const wrap = new FakeElement('span') + const identifiers = identifierTexts.map((identifierText) => { + const identifier = new FakeElement('p') + appendText(identifier, identifierText) + wrap.appendChild(identifier) + return identifier + }) + button.appendChild(wrap) + document.root.appendChild(button) + return { button, identifiers, wrap } +} + +function createConsentIdentity( + document: FakeDocument, + textBefore: string, + identifierText: string, + textAfter: string, +): { container: FakeElement; identifier: FakeElement } { + const container = new FakeElement('p') + appendText(container, textBefore) + const identifier = new FakeElement('b') + appendText(identifier, identifierText) + container.appendChild(identifier) + appendText(container, textAfter) + document.root.appendChild(container) + return { container, identifier } +} + +function selectedAliceSession(preferredUsername = 'alice.test'): unknown[] { + return [ + { + selected: true, + account: { + sub: 'did:plc:alice', + email: 'alice@example.test', + preferred_username: preferredUsername, + selected: true, + }, + }, + ] +} + +function aliceDeviceSession(): unknown[] { + return [ + { + account: { + sub: 'did:plc:alice', + email: 'alice@example.test', + preferred_username: 'alice.test', + }, + selected: true, + }, + ] +} + +function appendHandleModeMeta(document: FakeDocument, mode: string): void { + document.root.appendChild( + new FakeElement('meta', { name: 'epds-handle-mode', content: mode }), + ) +} + describe('buildChooserEnrichmentScript account row scoping', () => { it('does not enrich consent copy outside a chooser account row', () => { const document = new FakeDocument() @@ -261,13 +466,11 @@ describe('buildChooserEnrichmentScript account row scoping', () => { it('enriches a chooser-like account row', () => { const document = new FakeDocument() - const row = new FakeElement('div', { role: 'button', tabindex: '0' }) - const wrap = new FakeElement('span') - const handle = new FakeElement('span') - appendText(handle, 'alice.test') - wrap.appendChild(handle) - row.appendChild(wrap) - document.root.appendChild(row) + const { + row, + wrap, + identifier: handle, + } = createChooserRow(document, 'alice.test') runChooserEnrichmentScript(document) @@ -280,6 +483,248 @@ describe('buildChooserEnrichmentScript account row scoping', () => { expect(emailLabel).toBeInstanceOf(FakeElement) expect(emailLabel?.textContent).toBe('alice@example.test') expect(handle.classList.contains('epds-handle-label')).toBe(true) + expect(handle.style.display).toBeUndefined() + expect(row.getAttribute('aria-label')).toBe('Sign in as alice@example.test') + }) + + it('uses email as the visible random-mode identifier and describes the hidden handle', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { + row, + wrap, + identifier: handle, + } = createChooserRow(document, 'alice.test') + + runChooserEnrichmentScript(document) + + const emailLabel = wrap.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-email-label'), + ) as FakeElement | undefined + const handleDescription = row.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-hidden-handle-description'), + ) as FakeElement | undefined + + expect(emailLabel?.textContent).toBe('alice@example.test') + expect(handle.style.display).toBe('none') + expect(handleDescription?.textContent).toBe('Underlying handle: alice.test') + expect(row.getAttribute('aria-describedby')).toBe(handleDescription?.id) + expect(emailLabel?.getAttribute('title')).toBeNull() + expect(row.getAttribute('aria-label')).toBe('Sign in as alice@example.test') + }) + + it('does not hide random-mode handles outside oauth authorize', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { wrap, identifier, anchor } = createAccountListRow( + document, + 'alice.test', + ) + + runChooserEnrichmentScript( + document, + {}, + { pathname: '/account', search: '' }, + ) + + expect(wrap.textContent).toContain('alice@example.test') + expect(identifier.style.display).toBeUndefined() + expect(anchor.getAttribute('aria-label')).toBe( + 'View and manage account for alice@example.test (@alice.test)', + ) + }) + + it('uses an empty account-list title slot for the email on account pages', () => { + const document = new FakeDocument() + const { title, identifier, anchor } = createAccountListRow( + document, + 'alice.test', + { emptyTitle: true }, + ) + + runChooserEnrichmentScript( + document, + {}, + { pathname: '/account', search: '' }, + ) + + expect(title?.textContent).toBe('alice@example.test') + expect(identifier.textContent).toBe('alice.test') + expect(identifier.style.display).toBeUndefined() + expect(anchor.getAttribute('aria-label')).toBe( + 'View and manage account for alice@example.test (@alice.test)', + ) + }) + + it('leaves non-account links on account pages untouched', () => { + const document = new FakeDocument() + const anotherAccount = new FakeElement('a', { + href: '/account/login', + 'aria-label': 'Sign in with another account', + }) + appendText(anotherAccount, 'Sign in with another account') + document.root.appendChild(anotherAccount) + const terms = new FakeElement('a', { href: '/terms' }) + appendText(terms, 'Terms') + document.root.appendChild(terms) + const prose = new FakeElement('p') + appendText(prose, 'Manage alice.test from this page.') + document.root.appendChild(prose) + + runChooserEnrichmentScript( + document, + {}, + { pathname: '/account', search: '' }, + ) + + expect(anotherAccount.textContent).toBe('Sign in with another account') + expect(terms.textContent).toBe('Terms') + expect(prose.textContent).toBe('Manage alice.test from this page.') + expect(document.root.textContent).not.toContain('alice@example.test') + }) + + it('adds email next to the current account selector handle on account detail pages', () => { + const document = new FakeDocument() + const { button, identifiers, wrap } = createAccountSelector(document, [ + 'alice.test', + ]) + + runChooserEnrichmentScript( + document, + { __sessions: [], __deviceSessions: aliceDeviceSession() }, + { pathname: '/account/did:plc:alice', search: '' }, + ) + + expect(identifiers[0].textContent).toBe('alice.test') + expect(identifiers[0].style.display).toBeUndefined() + expect(wrap.textContent).toContain('alice@example.test') + expect(button.getAttribute('aria-label')).toBe( + 'Select account alice@example.test (@alice.test)', + ) + }) + + it('collapses duplicate current account selector handle lines to email plus handle', () => { + const document = new FakeDocument() + const { button, identifiers } = createAccountSelector(document, [ + 'alice.test', + 'alice.test', + ]) + + runChooserEnrichmentScript( + document, + { __sessions: [], __deviceSessions: aliceDeviceSession() }, + { pathname: '/account/did:plc:alice', search: '' }, + ) + + expect(identifiers[0].textContent).toBe('alice@example.test') + expect(identifiers[1].textContent).toBe('alice.test') + expect(button.textContent).toBe('alice@example.testalice.test') + expect(button.getAttribute('aria-label')).toBe( + 'Select account alice@example.test (@alice.test)', + ) + }) + + it('keeps the current account selector handle visible when account pages use random mode', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { identifiers, wrap } = createAccountSelector(document, [ + 'alice.test', + ]) + + runChooserEnrichmentScript( + document, + { __sessions: [], __deviceSessions: aliceDeviceSession() }, + { pathname: '/account/did:plc:alice', search: '' }, + ) + + expect(identifiers[0].textContent).toBe('alice.test') + expect(identifiers[0].style.display).toBeUndefined() + expect(wrap.textContent).toContain('alice@example.test') + }) + + it('does not enrich non-selector controls on account detail pages', () => { + const document = new FakeDocument() + createAccountSelector(document, ['alice.test']) + const connectedApp = new FakeElement('button', { + 'aria-label': 'Open app settings', + }) + appendText(connectedApp, 'alice.test') + document.root.appendChild(connectedApp) + const signOut = new FakeElement('button', { 'aria-label': 'Sign out' }) + appendText(signOut, 'Sign out alice.test') + document.root.appendChild(signOut) + const breadcrumb = new FakeElement('a', { href: '/account' }) + appendText(breadcrumb, 'alice.test') + document.root.appendChild(breadcrumb) + + runChooserEnrichmentScript( + document, + { __sessions: [], __deviceSessions: aliceDeviceSession() }, + { pathname: '/account/did:plc:alice', search: '' }, + ) + + expect(connectedApp.textContent).toBe('alice.test') + expect(signOut.textContent).toBe('Sign out alice.test') + expect(breadcrumb.textContent).toBe('alice.test') + }) + + it('enriches exact at-prefixed handle matches', () => { + const document = new FakeDocument() + const { wrap, identifier } = createChooserRow(document, '@alice.test') + + runChooserEnrichmentScript(document) + + expect(wrap.textContent).toContain('alice@example.test') + expect(identifier.classList.contains('epds-handle-label')).toBe(true) + }) + + it('enriches exact DID matches', () => { + const document = new FakeDocument() + const { wrap, identifier } = createChooserRow(document, 'did:plc:alice') + + runChooserEnrichmentScript(document) + + expect(wrap.textContent).toContain('alice@example.test') + expect(identifier.classList.contains('epds-handle-label')).toBe(true) + }) + + it('enriches rows from captured device sessions', () => { + const document = new FakeDocument() + const { wrap, identifier } = createChooserRow(document, 'carol.test') + + runChooserEnrichmentScript(document, { + __sessions: [], + __deviceSessions: [ + { + account: { + sub: 'did:plc:carol', + email: 'carol@example.test', + preferred_username: 'carol.test', + }, + selected: true, + }, + ], + }) + + expect(wrap.textContent).toContain('carol@example.test') + expect(identifier.classList.contains('epds-handle-label')).toBe(true) + }) + + it('does not enrich substring-only chooser row prose', () => { + const document = new FakeDocument() + const { wrap, identifier } = createChooserRow( + document, + 'Signed in as alice.test', + ) + + runChooserEnrichmentScript(document) + + expect(wrap.textContent).not.toContain('alice@example.test') + expect(identifier.classList.contains('epds-handle-label')).toBe(false) }) it('enriches multiple chooser-like account rows', () => { @@ -304,6 +749,161 @@ describe('buildChooserEnrichmentScript account row scoping', () => { }) }) +describe('buildChooserEnrichmentScript consent identity enrichment', () => { + it('keeps the picker consent handle visible and adds an accessible email tooltip', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript(document) + + const icon = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-info-icon'), + ) as FakeElement | undefined + const tooltip = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-tooltip'), + ) as FakeElement | undefined + + expect(identifier.textContent).toBe('alice.test') + expect(icon).toBeInstanceOf(FakeElement) + expect(icon?.tagName).toBe('button') + expect(icon?.getAttribute('aria-describedby')).toBe(tooltip?.id) + expect(tooltip?.textContent).toBe( + 'This handle is associated with alice@example.test.', + ) + }) + + it('treats picker-with-random and default consent like picker consent', () => { + for (const mode of ['picker-with-random', null]) { + const document = new FakeDocument() + if (mode) appendHandleModeMeta(document, mode) + const { container, identifier } = createConsentIdentity( + document, + 'Example App wants to access your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript(document, { + __sessions: selectedAliceSession(), + }) + + expect(identifier.textContent).toBe('alice.test') + expect(container.textContent).toContain( + 'This handle is associated with alice@example.test.', + ) + } + }) + + it('shows the email for random consent and exposes the public handle in the tooltip', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { container, identifier } = createConsentIdentity( + document, + 'Grant access to your ', + '@alice.test', + ' account', + ) + + runChooserEnrichmentScript(document, { + __sessions: selectedAliceSession('@alice.test'), + }) + + expect(identifier.textContent).toBe('alice@example.test') + expect(container.textContent).toContain( + 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', + ) + expect(container.textContent).not.toContain('@@alice.test') + }) + + it('leaves generic and legal consent paragraphs untouched', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const legal = new FakeElement('p') + appendText( + legal, + 'By clicking Authorize, you confirm that alice.test is your account.', + ) + document.root.appendChild(legal) + const unrelated = createConsentIdentity( + document, + 'Grant access to your ', + 'bob.test', + ' account', + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + expect(document.root.textContent).not.toContain('This handle is associated') + expect(unrelated.identifier.textContent).toBe('bob.test') + }) + + it('opens the tooltip on hover/focus and toggles it on click/tap', () => { + const document = new FakeDocument() + const { container } = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + const icon = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-info-icon'), + ) as FakeElement + const tooltip = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-tooltip'), + ) as FakeElement + + expect(tooltip.hidden).toBe(true) + icon.dispatchEvent('mouseenter') + expect(tooltip.hidden).toBe(false) + icon.dispatchEvent('mouseleave') + expect(tooltip.hidden).toBe(true) + icon.dispatchEvent('focus') + expect(tooltip.hidden).toBe(false) + icon.dispatchEvent('blur') + expect(tooltip.hidden).toBe(true) + const click = icon.dispatchEvent('click') + expect(click.defaultPrevented).toBe(true) + expect(tooltip.hidden).toBe(false) + icon.dispatchEvent('click') + expect(tooltip.hidden).toBe(true) + }) + + it('does not apply consent tooltip behavior on account pages', () => { + const document = new FakeDocument() + createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { pathname: '/account', search: '' }, + ) + + expect(document.root.textContent).not.toContain('This handle is associated') + }) +}) + describe('sha256Base64', () => { it('produces a stable SHA256 base64 hash', () => { // Known value for the empty string. @@ -662,13 +1262,15 @@ describe('buildChooserEnrichmentScript handle-mode hiding (HYPER-268 Layer 4)', expect(script).toContain('querySelector(\'meta[name="epds-handle-mode"]\')') }) - it("hides the handle span and sets a title tooltip when mode is 'random'", () => { + it("hides the handle span and adds an accessible description when mode is 'random'", () => { const script = buildChooserEnrichmentScript() - // Hiding strategy: display:none on the handle element + title - // attribute on the email label carrying the original handle text. - expect(script).toContain("hideHandle = handleMode === 'random'") + // Hiding strategy: display:none on the handle element plus an + // aria-describedby target carrying the original handle text. + expect(script).toContain( + "hideHandle = handleMode === 'random' && isOauthAuthorizePage()", + ) expect(script).toContain("m.el.style.display = 'none'") - expect(script).toContain('label.title = ownText') + expect(script).toContain("appendAriaReference(row, 'aria-describedby'") }) it('leaves the handle visible for picker / picker-with-random', () => { diff --git a/packages/pds-core/src/chooser-enrichment.ts b/packages/pds-core/src/chooser-enrichment.ts index dbf76ecd..b9369c91 100644 --- a/packages/pds-core/src/chooser-enrichment.ts +++ b/packages/pds-core/src/chooser-enrichment.ts @@ -53,19 +53,19 @@ export function buildChooserEnrichmentScript(): string { // sets window.__deviceSessions (type: readonly ActiveDeviceSession[]) // Both contain { account: { sub, email, preferred_username, ... }, ... } // so our DOM-enrichment heuristic can operate on either one. - var captured = null; + var capturedGlobals = Object.create(null); function interceptGlobal(name) { try { Object.defineProperty(window, name, { configurable: true, set: function(v) { - captured = v; + capturedGlobals[name] = v; // Forward to a plain data prop so the SPA still sees the value. Object.defineProperty(window, name, { configurable: true, enumerable: true, writable: true, value: v, }); }, - get: function() { return captured; }, + get: function() { return capturedGlobals[name]; }, }); } catch (_) {} } @@ -75,8 +75,9 @@ export function buildChooserEnrichmentScript(): string { // Current OAuth flow's handle-assignment mode, written into a // by the pds-core middleware. When // "random", the handle is a server-generated opaque string that the - // user never chose, so we hide it from the chooser and expose it only - // via a title= tooltip — the email remains the primary identifier. + // user never chose, so OAuth authorize chooser rows show the email as + // the primary identifier while keeping the handle available through an + // explicit accessible description. // Any unknown / missing value disables hiding and renders handle + // email side-by-side, same as pre-Layer-4 behaviour. function readHandleMode() { @@ -131,30 +132,265 @@ export function buildChooserEnrichmentScript(): string { return authOrigin + '/oauth/authorize?' + params.toString(); } + function buildAccounts() { + return buildAccountsFromSources([capturedGlobals.__sessions, capturedGlobals.__deviceSessions]); + } + + function buildDeviceAccounts() { + return buildAccountsFromSources([capturedGlobals.__deviceSessions]); + } + + function buildAccountsFromSources(sources) { + var accounts = []; + sources.forEach(function(source) { + if (!Array.isArray(source)) return; + source.forEach(function(session) { + var account = session && session.account; + if (!account) return; + accounts.push({ + sub: account.sub || '', + email: account.email || '', + preferred_username: account.preferred_username || '', + selected: !!(account.selected || session.selected), + }); + }); + }); + return accounts; + } + + function matchAccountIdentifier(accounts, text) { + var trimmed = (text || '').trim(); + if (!trimmed) return null; + for (var i = 0; i < accounts.length; i++) { + var account = accounts[i]; + if (!account.email) continue; + if (account.preferred_username) { + if (trimmed === account.preferred_username) return account; + if (trimmed === '@' + account.preferred_username) return account; + } + if (account.sub && trimmed === account.sub) return account; + } + return null; + } + + function selectedAccount(accounts) { + for (var i = 0; i < accounts.length; i++) { + if (accounts[i].selected) return accounts[i]; + } + return accounts.length === 1 ? accounts[0] : null; + } + + function formatPublicHandle(handle) { + return handle && handle.charAt(0) === '@' ? handle : '@' + handle; + } + + function appendAriaReference(el, attr, id) { + var current = el.getAttribute(attr) || ''; + var refs = current ? current.split(/\\s+/) : []; + for (var i = 0; i < refs.length; i++) { + if (refs[i] === id) return; + } + refs.push(id); + el.setAttribute(attr, refs.join(' ').trim()); + } + + function appendIdentityInfoIcon(el, tooltipText) { + var id = 'epds-identity-tooltip-' + Math.random().toString(36).slice(2); + var icon = document.createElement('button'); + icon.type = 'button'; + icon.className = 'epds-identity-info-icon'; + icon.textContent = 'ⓘ'; + icon.setAttribute('aria-label', 'Identity information'); + icon.setAttribute('aria-describedby', id); + icon.setAttribute('aria-expanded', 'false'); + icon.style.cssText = 'border:0;background:transparent;padding:0 0 0 .25em;cursor:pointer;font:inherit;line-height:1;color:inherit;'; + + var tooltip = document.createElement('span'); + tooltip.id = id; + tooltip.className = 'epds-identity-tooltip'; + tooltip.setAttribute('role', 'tooltip'); + tooltip.hidden = true; + tooltip.textContent = tooltipText; + tooltip.style.cssText = 'position:absolute;z-index:10;max-width:20rem;margin-left:.35em;padding:.35em .5em;border-radius:.25rem;background:#111;color:#fff;font-size:.875em;line-height:1.3;'; + + var pinned = false; + function show() { + tooltip.hidden = false; + icon.setAttribute('aria-expanded', 'true'); + } + function hide() { + if (pinned) return; + tooltip.hidden = true; + icon.setAttribute('aria-expanded', 'false'); + } + icon.addEventListener('mouseenter', show); + icon.addEventListener('focus', show); + icon.addEventListener('mouseleave', hide); + icon.addEventListener('blur', hide); + icon.addEventListener('click', function(e) { + e.preventDefault(); + e.stopPropagation(); + pinned = tooltip.hidden; + if (pinned) show(); + else hide(); + }); + + el.insertAdjacentElement('afterend', tooltip); + el.insertAdjacentElement('afterend', icon); + } + + function isConsentIdentityElement(el) { + var parent = el.parentElement; + if (!parent) return false; + var tagName = (el.tagName || '').toLowerCase(); + if (tagName !== 'b' && tagName !== 'strong') return false; + return true; + } + + function enrichConsentIdentity(accounts, handleMode) { + if (!isOauthAuthorizePage()) return; + var selected = selectedAccount(accounts); + var consentAccounts = selected ? [selected] : accounts; + var root = document.getElementById('root'); + if (!root) return; + var walker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT); + var node; + while ((node = walker.nextNode())) { + if (node.dataset && node.dataset.epdsConsentEnriched) continue; + var text = (node.textContent || '').trim(); + if (!text || !isConsentIdentityElement(node)) continue; + var account = matchAccountIdentifier(consentAccounts, text); + if (!account) continue; + + if (handleMode === 'random') { + node.textContent = account.email; + appendIdentityInfoIcon( + node, + 'Public AT Protocol handle: ' + formatPublicHandle(text) + '. Handles are public account names used by AT Protocol apps.', + ); + } else { + appendIdentityInfoIcon( + node, + 'This handle is associated with ' + account.email + '.', + ); + } + if (node.dataset) node.dataset.epdsConsentEnriched = '1'; + } + } + + function isOauthAuthorizePage() { + return window.location && window.location.pathname === '/oauth/authorize'; + } + + function isAccountPage() { + var pathname = (window.location && window.location.pathname) || ''; + return pathname === '/account' || pathname.indexOf('/account/') === 0; + } + + function isAccountDetailPage() { + var pathname = (window.location && window.location.pathname) || ''; + return pathname.indexOf('/account/did:') === 0; + } + + function ownTextOf(el) { + var own = ''; + for (var i = 0; i < el.childNodes.length; i++) { + var c = el.childNodes[i]; + if (c.nodeType === Node.TEXT_NODE) own += c.data; + } + return own; + } + + function accountSelectorButtons(root) { + var buttons = []; + var candidates = root.querySelectorAll('button, a'); + for (var i = 0; i < candidates.length; i++) { + var candidate = candidates[i]; + if ((candidate.tagName || '').toLowerCase() !== 'button') continue; + if (candidate.getAttribute('aria-label') !== 'Select an account') continue; + buttons.push(candidate); + } + return buttons; + } + + function enrichAccountSelector(root) { + if (!isAccountDetailPage()) return; + var accounts = buildDeviceAccounts(); + if (!accounts.length) return; + var selectors = accountSelectorButtons(root); + for (var i = 0; i < selectors.length; i++) { + var selector = selectors[i]; + if (selector.dataset && selector.dataset.epdsAccountSelectorEnriched) continue; + var walker = document.createTreeWalker(selector, NodeFilter.SHOW_ELEMENT); + var node; + var selectorMatches = []; + while ((node = walker.nextNode())) { + if (node.dataset && node.dataset.epdsEnriched) continue; + var own = ownTextOf(node); + if (!own) continue; + var account = matchAccountIdentifier(accounts, own); + if (account) selectorMatches.push({ el: node, account: account, text: own }); + } + if (!selectorMatches.length) continue; + var match = selectorMatches[0]; + var duplicateMatches = selectorMatches.filter(function(candidate) { + return candidate.account === match.account; + }); + var publicIdentifier = match.account.preferred_username + ? formatPublicHandle(match.account.preferred_username) + : formatPublicHandle((match.text || '').trim()); + selector.setAttribute('aria-label', 'Select account ' + match.account.email + ' (' + publicIdentifier + ')'); + if (duplicateMatches.length > 1) { + duplicateMatches[0].el.textContent = match.account.email; + duplicateMatches[0].el.classList.add('epds-email-label'); + for (var d = 1; d < duplicateMatches.length; d++) { + duplicateMatches[d].el.classList.add('epds-handle-label'); + if (duplicateMatches[d].el.dataset) duplicateMatches[d].el.dataset.epdsEnriched = '1'; + } + } else { + match.el.classList.add('epds-handle-label'); + var label = document.createElement('span'); + label.className = 'epds-email-label'; + label.style.cssText = + 'min-width:0;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;' + label.textContent = match.account.email; + var wrap = match.el.parentElement; + if (wrap) { + wrap.style.flexDirection = 'column'; + wrap.style.alignItems = 'flex-start'; + wrap.style.minWidth = '0'; + match.el.insertAdjacentElement('afterend', label); + } else { + match.el.appendChild(label); + } + } + if (match.el.dataset) match.el.dataset.epdsEnriched = '1'; + if (selector.dataset) selector.dataset.epdsAccountSelectorEnriched = '1'; + } + } + // Enrich each visible account row with its email. Runs repeatedly // via a MutationObserver because the SPA hydrates/re-renders after // initial HTML delivery. function enrich() { - if (!captured || !Array.isArray(captured)) return; + if (!isOauthAuthorizePage() && !isAccountPage()) return; + var accounts = buildAccounts(); + if (!accounts.length) return; var handleMode = readHandleMode(); - var hideHandle = handleMode === 'random'; - var byHandle = Object.create(null); - var bySub = Object.create(null); - captured.forEach(function(s) { - var a = s && s.account; - if (!a) return; - if (a.preferred_username) byHandle[a.preferred_username] = a.email || ''; - if (a.sub) bySub[a.sub] = a.email || ''; - }); + var hideHandle = handleMode === 'random' && isOauthAuthorizePage(); + + enrichConsentIdentity(accounts, handleMode); - // Find the deepest element whose own text content contains a known - // handle or sub, and append the email next to it. Upstream's markup + // Find the deepest element whose own trimmed text exactly matches a + // known handle, @handle, or DID, and append the email next to it. + // Upstream's markup // varies between versions; walking by leaf-element text is more // resilient than guessing at class names. We skip elements that have // children whose text also matches (so we only label the deepest // match per row — usually a or similar inline container). var root = document.getElementById('root'); if (!root) return; + enrichAccountSelector(root); var walker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT); var node; var matches = []; @@ -162,24 +398,64 @@ export function buildChooserEnrichmentScript(): string { if (node.dataset && node.dataset.epdsEnriched) continue; // Compute "own text" — text content excluding descendant element // text. We approximate by joining Text-node children's data. - var own = ''; - for (var i = 0; i < node.childNodes.length; i++) { - var c = node.childNodes[i]; - if (c.nodeType === Node.TEXT_NODE) own += c.data; - } + var own = ownTextOf(node); if (!own) continue; - var email = ''; - for (var handle in byHandle) { - if (own.indexOf(handle) >= 0) { email = byHandle[handle]; break; } + var account = matchAccountIdentifier(accounts, own); + if (account) matches.push({ el: node, account: account, email: account.email }); + } + function accountListAnchor(el) { + var anchor = el.closest('a'); + if (!anchor) return null; + var href = anchor.getAttribute('href') || ''; + var ariaLabel = anchor.getAttribute('aria-label') || ''; + if (href.indexOf('/account/did:') !== 0) return null; + if (ariaLabel.indexOf('View and manage account for') !== 0) return null; + return anchor; + } + function emptyAccountTitle(anchor) { + var headings = anchor.querySelectorAll('h2'); + for (var i = 0; i < headings.length; i++) { + if (!((headings[i].textContent || '').trim())) return headings[i]; } - if (!email) { - for (var sub in bySub) { - if (own.indexOf(sub) >= 0) { email = bySub[sub]; break; } + return null; + } + function enrichAccountListRow(m) { + var anchor = accountListAnchor(m.el); + if (!anchor || (anchor.dataset && anchor.dataset.epdsAccountListEnriched)) return; + var ownText = (m.el.textContent || '').trim(); + var publicIdentifier = m.account.preferred_username + ? formatPublicHandle(m.account.preferred_username) + : formatPublicHandle(ownText); + var title = emptyAccountTitle(anchor); + if (title) { + title.textContent = m.email; + title.classList.add('epds-email-label'); + } else { + var label = document.createElement('span'); + label.className = 'epds-email-label'; + label.style.cssText = + 'min-width:0;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;' + label.textContent = m.email; + var wrap = m.el.parentElement; + if (wrap) { + wrap.style.flexDirection = 'column'; + wrap.style.alignItems = 'flex-start'; + wrap.style.minWidth = '0'; + wrap.appendChild(label); + } else { + m.el.appendChild(label); } } - if (email) matches.push({ el: node, email: email }); + m.el.classList.add('epds-handle-label'); + anchor.setAttribute('aria-label', 'View and manage account for ' + m.email + ' (' + publicIdentifier + ')'); + if (m.el.dataset) m.el.dataset.epdsEnriched = '1'; + if (anchor.dataset) anchor.dataset.epdsAccountListEnriched = '1'; } matches.forEach(function(m) { + if (isAccountPage()) { + enrichAccountListRow(m); + return; + } // oauth-provider-ui 0.4.3 renders chooser rows this way; revisit if the upstream PDS UI is upgraded. var row = m.el.closest('[role="button"][tabindex="0"]'); if (!row) return; @@ -199,6 +475,7 @@ export function buildChooserEnrichmentScript(): string { // .epds-email-label { order: -1 } // No inline typography (font-size, color, weight) so normal CSS // specificity rules apply when branding wants to override. + var ownText = (m.el.textContent || '').trim(); var label = document.createElement('span'); label.className = 'epds-email-label'; label.style.cssText = @@ -216,19 +493,25 @@ export function buildChooserEnrichmentScript(): string { m.el.appendChild(label); } + row.setAttribute('aria-label', 'Sign in as ' + m.email); + // Random-handle mode: the handle is server-assigned gibberish // the user never chose (e.g. "frail-ivy-cabbage.pds.example"). - // We use display:none — which removes the element from the - // accessibility tree — intentionally. Announcing the opaque - // string to screen-reader users carries no semantic value and - // actively confuses the row's accessible name ("DID xyz, handle - // frail-ivy-cabbage, email alice@example"). The email label - // immediately below stays visible and announced; power users - // can still inspect the handle via the tooltip we set on it. + // Keep it out of normal visual layout, but attach a separate + // non-interactive description so assistive technology can still + // expose the underlying handle without making it the row name. if (hideHandle) { - var ownText = (m.el.textContent || '').trim(); if (ownText) { - label.title = ownText; + var description = document.createElement('span'); + var descriptionId = 'epds-hidden-handle-' + matches.indexOf(m); + description.id = descriptionId; + description.className = 'epds-hidden-handle-description'; + description.textContent = 'Underlying handle: ' + ownText; + description.style.cssText = + 'position:absolute;width:1px;height:1px;padding:0;margin:-1px;overflow:hidden;clip:rect(0,0,0,0);white-space:nowrap;border:0;'; + description.setAttribute('aria-hidden', 'false'); + row.appendChild(description); + appendAriaReference(row, 'aria-describedby', descriptionId); } m.el.style.display = 'none'; } From 5d307c89eb381b213b547b06cf70b01f1734a504 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Mon, 4 May 2026 12:51:24 +0600 Subject: [PATCH 04/12] fix(pds-core): tighten consent identity enrichment --- .../session-reuse-bugs.steps.ts | 100 +++++- features/session-reuse-bugs.feature | 2 +- .../src/__tests__/chooser-enrichment.test.ts | 312 +++++++++++++++++- .../src/__tests__/preview-chooser.test.ts | 22 ++ .../src/__tests__/preview-consent.test.ts | 56 ++++ packages/pds-core/src/chooser-enrichment.ts | 56 +++- packages/pds-core/src/lib/preview-consent.ts | 42 ++- 7 files changed, 563 insertions(+), 27 deletions(-) diff --git a/e2e/step-definitions/session-reuse-bugs.steps.ts b/e2e/step-definitions/session-reuse-bugs.steps.ts index 23cc5bdf..26b875fa 100644 --- a/e2e/step-definitions/session-reuse-bugs.steps.ts +++ b/e2e/step-definitions/session-reuse-bugs.steps.ts @@ -320,8 +320,8 @@ When( // pds-core's chooser middleware reads to inject // into the chooser's // . 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,93 @@ Then( }, ) +type HiddenHandleDescriptionRow = { + describedBy: string | null + descriptions: { + id: string + isHiddenHandleDescription: boolean + text: string + }[] + emailTitle: string | null + 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 = + window.getComputedStyle(handleLabel).display === 'none' + if (!handleIsHidden) return null + + 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'), + 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) + expect( + descriptionText.slice(prefixIndex + prefix.length).trim().length, + ).toBeGreaterThan(0) + + expect( + row.emailTitle, + `Row ${row.rowIndex}: .epds-email-label should not expose the hidden handle through title=`, + ).toBeNull() } }, ) diff --git a/features/session-reuse-bugs.feature b/features/session-reuse-bugs.feature index c35d3a4a..1548de87 100644 --- a/features/session-reuse-bugs.feature +++ b/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 diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index d2450410..95fe6dbf 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -402,10 +402,11 @@ function createConsentIdentity( textBefore: string, identifierText: string, textAfter: string, + tagName = 'b', ): { container: FakeElement; identifier: FakeElement } { const container = new FakeElement('p') appendText(container, textBefore) - const identifier = new FakeElement('b') + const identifier = new FakeElement(tagName) appendText(identifier, identifierText) container.appendChild(identifier) appendText(container, textAfter) @@ -487,6 +488,27 @@ describe('buildChooserEnrichmentScript account row scoping', () => { expect(row.getAttribute('aria-label')).toBe('Sign in as alice@example.test') }) + it('enriches preview chooser rows in picker-with-random mode', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker-with-random') + const { + row, + wrap, + identifier: handle, + } = createChooserRow(document, 'alice.test') + + runChooserEnrichmentScript( + document, + {}, + { pathname: '/preview/chooser', search: '' }, + ) + + expect(wrap.textContent).toContain('alice@example.test') + expect(handle.classList.contains('epds-handle-label')).toBe(true) + expect(handle.style.display).toBeUndefined() + expect(row.getAttribute('aria-label')).toBe('Sign in as alice@example.test') + }) + it('uses email as the visible random-mode identifier and describes the hidden handle', () => { const document = new FakeDocument() appendHandleModeMeta(document, 'random') @@ -517,6 +539,38 @@ describe('buildChooserEnrichmentScript account row scoping', () => { expect(row.getAttribute('aria-label')).toBe('Sign in as alice@example.test') }) + it('uses email as the visible random-mode identifier on preview chooser rows', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { + row, + wrap, + identifier: handle, + } = createChooserRow(document, 'alice.test') + + runChooserEnrichmentScript( + document, + {}, + { pathname: '/preview/chooser', search: '' }, + ) + + const emailLabel = wrap.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-email-label'), + ) as FakeElement | undefined + const handleDescription = row.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-hidden-handle-description'), + ) as FakeElement | undefined + + expect(emailLabel?.textContent).toBe('alice@example.test') + expect(handle.style.display).toBe('none') + expect(handleDescription?.textContent).toBe('Underlying handle: alice.test') + expect(row.getAttribute('aria-describedby')).toBe(handleDescription?.id) + }) + it('does not hide random-mode handles outside oauth authorize', () => { const document = new FakeDocument() appendHandleModeMeta(document, 'random') @@ -750,7 +804,7 @@ describe('buildChooserEnrichmentScript account row scoping', () => { }) describe('buildChooserEnrichmentScript consent identity enrichment', () => { - it('keeps the picker consent handle visible and adds an accessible email tooltip', () => { + it('enriches grant-access consent identity copy', () => { const document = new FakeDocument() appendHandleModeMeta(document, 'picker') const { container, identifier } = createConsentIdentity( @@ -782,6 +836,93 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { ) }) + it('enriches client-wants-access consent identity copy', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + 'Example App wants to access your ', + 'alice.test', + ' account', + 'strong', + ) + + runChooserEnrichmentScript(document, { + __sessions: selectedAliceSession(), + }) + + expect(identifier.textContent).toBe('alice.test') + expect(container.textContent).toContain( + 'This handle is associated with alice@example.test.', + ) + }) + + it('enriches upstream main-card consent identity copy without same-paragraph client name', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + 'wants to access your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript(document, { + __sessions: selectedAliceSession(), + }) + + const iconIndex = container.childNodes.findIndex( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-info-icon'), + ) + const identifierIndex = container.childNodes.indexOf(identifier) + + expect(identifier.textContent).toBe('alice.test') + expect(iconIndex).toBe(identifierIndex + 1) + expect(container.textContent).toContain( + 'This handle is associated with alice@example.test.', + ) + }) + + it('enriches sidebar and main-card consent identities on the same page', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker-with-random') + const sidebar = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + const mainCard = createConsentIdentity( + document, + 'wants to access your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { pathname: '/preview/consent', search: '' }, + ) + + const icons = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-info-icon')) + const tooltips = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-tooltip')) + + expect(sidebar.identifier.textContent).toBe('alice.test') + expect(mainCard.identifier.textContent).toBe('alice.test') + expect(icons).toHaveLength(2) + expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ + 'This handle is associated with alice@example.test.', + 'This handle is associated with alice@example.test.', + ]) + }) + it('treats picker-with-random and default consent like picker consent', () => { for (const mode of ['picker-with-random', null]) { const document = new FakeDocument() @@ -825,6 +966,132 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { expect(container.textContent).not.toContain('@@alice.test') }) + it('enriches preview consent identity copy', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker-with-random') + const { container, identifier } = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { pathname: '/preview/consent', search: '' }, + ) + + expect(identifier.textContent).toBe('alice.test') + expect(container.textContent).toContain( + 'This handle is associated with alice@example.test.', + ) + }) + + it('enriches preview chooser consent state in picker-with-random mode', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker-with-random') + const sidebar = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + const mainCard = createConsentIdentity( + document, + 'wants to access your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { + pathname: '/preview/chooser', + search: '?epds_handle_mode=picker-with-random', + }, + ) + + const icons = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-info-icon')) + const tooltips = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-tooltip')) + + expect(sidebar.identifier.textContent).toBe('alice.test') + expect(mainCard.identifier.textContent).toBe('alice.test') + expect(icons).toHaveLength(2) + expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ + 'This handle is associated with alice@example.test.', + 'This handle is associated with alice@example.test.', + ]) + }) + + it('uses email as the visible preview chooser consent identity in random mode', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const sidebar = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + const mainCard = createConsentIdentity( + document, + 'wants to access your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { + pathname: '/preview/chooser', + search: '?epds_handle_mode=random', + }, + ) + + const icons = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-info-icon')) + const tooltips = document.root + .descendants() + .filter((el) => el.classList.contains('epds-identity-tooltip')) + + expect(sidebar.identifier.textContent).toBe('alice@example.test') + expect(mainCard.identifier.textContent).toBe('alice@example.test') + expect(icons).toHaveLength(2) + expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ + 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', + 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', + ]) + }) + + it('uses email as the visible preview consent identity in random mode', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'random') + const { container, identifier } = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { pathname: '/preview/consent', search: '' }, + ) + + expect(identifier.textContent).toBe('alice@example.test') + expect(container.textContent).toContain( + 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', + ) + }) + it('leaves generic and legal consent paragraphs untouched', () => { const document = new FakeDocument() appendHandleModeMeta(document, 'picker') @@ -847,6 +1114,45 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { expect(unrelated.identifier.textContent).toBe('bob.test') }) + it('leaves arbitrary bold selected-account legal copy untouched', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + 'By clicking Authorize, ', + 'alice.test', + ' confirms access.', + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + expect(identifier.textContent).toBe('alice.test') + expect(container.textContent).toBe( + 'By clicking Authorize, alice.test confirms access.', + ) + expect(container.textContent).not.toContain('This handle is associated') + }) + + it('leaves arbitrary strong selected-account technical prose untouched', () => { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + 'Technical details for ', + 'alice.test', + ' may include OAuth scopes.', + 'strong', + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + expect(identifier.textContent).toBe('alice.test') + expect(container.textContent).toBe( + 'Technical details for alice.test may include OAuth scopes.', + ) + expect(container.textContent).not.toContain('This handle is associated') + }) + it('opens the tooltip on hover/focus and toggles it on click/tap', () => { const document = new FakeDocument() const { container } = createConsentIdentity( @@ -1267,7 +1573,7 @@ describe('buildChooserEnrichmentScript handle-mode hiding (HYPER-268 Layer 4)', // Hiding strategy: display:none on the handle element plus an // aria-describedby target carrying the original handle text. expect(script).toContain( - "hideHandle = handleMode === 'random' && isOauthAuthorizePage()", + "hideHandle = handleMode === 'random' && isChooserLikePage()", ) expect(script).toContain("m.el.style.display = 'none'") expect(script).toContain("appendAriaReference(row, 'aria-describedby'") diff --git a/packages/pds-core/src/__tests__/preview-chooser.test.ts b/packages/pds-core/src/__tests__/preview-chooser.test.ts index 07ad8b4f..bff33e70 100644 --- a/packages/pds-core/src/__tests__/preview-chooser.test.ts +++ b/packages/pds-core/src/__tests__/preview-chooser.test.ts @@ -122,6 +122,28 @@ describe('createPreviewChooserHandler', () => { expect(res.body).toContain(`function readHandleMode()`) }) + it('places the enrichment script before __sessions hydration and includes fixture identities', async () => { + const handler = createPreviewChooserHandler(makeDeps())! + const res = mockRes() + await handler({ query: { numAccounts: '2' } }, res) + const scriptIndex = res.body!.indexOf('function readHandleMode()') + const hydrationIndex = res.body!.indexOf('window["__sessions"]') + + expect(scriptIndex).toBeGreaterThan(-1) + expect(hydrationIndex).toBeGreaterThan(-1) + expect(scriptIndex).toBeLessThan(hydrationIndex) + expect(res.body).toContain( + String.raw`\"preferred_username\":\"alice.preview.example\"`, + ) + expect(res.body).toContain( + String.raw`\"email\":\"alice@preview.example\"`, + ) + expect(res.body).toContain( + String.raw`\"preferred_username\":\"bob.preview.example\"`, + ) + expect(res.body).toContain(String.raw`\"email\":\"bob@preview.example\"`) + }) + it('reads the override from ?epds_handle_mode (production param name)', async () => { const handler = createPreviewChooserHandler(makeDeps())! const res = mockRes() diff --git a/packages/pds-core/src/__tests__/preview-consent.test.ts b/packages/pds-core/src/__tests__/preview-consent.test.ts index 26a76c26..94b2785c 100644 --- a/packages/pds-core/src/__tests__/preview-consent.test.ts +++ b/packages/pds-core/src/__tests__/preview-consent.test.ts @@ -80,6 +80,62 @@ describe('createPreviewConsentHandler', () => { expect(res.body).toContain('/@atproto/oauth-provider/~assets/') }) + it('injects handle-mode meta and enrichment script before __sessions hydration', async () => { + const handler = createPreviewConsentHandler({ + trustedClients: [], + resolveClientMetadata: () => Promise.resolve({}), + getClientCss: () => null, + logger: mockLogger(), + })! + const res = mockRes() + await handler({ query: {} }, res) + const handleModeIndex = res.body!.indexOf( + '', + ) + const enrichmentIndex = res.body!.indexOf('function readHandleMode()') + const hydrationIndex = res.body!.indexOf('window["__sessions"]') + + expect(handleModeIndex).toBeGreaterThan(-1) + expect(enrichmentIndex).toBeGreaterThan(-1) + expect(hydrationIndex).toBeGreaterThan(-1) + expect(handleModeIndex).toBeLessThan(enrichmentIndex) + expect(enrichmentIndex).toBeLessThan(hydrationIndex) + }) + + it('hydrates the selected preview session with handle and email', async () => { + const handler = createPreviewConsentHandler({ + trustedClients: [], + resolveClientMetadata: () => Promise.resolve({}), + getClientCss: () => null, + logger: mockLogger(), + })! + const res = mockRes() + await handler({ query: {} }, res) + + expect(res.body).toContain(String.raw`\"selected\":true`) + expect(res.body).toContain( + String.raw`\"preferred_username\":\"alice.preview.example\"`, + ) + expect(res.body).toContain( + String.raw`\"email\":\"alice@preview.example\"`, + ) + }) + + it('supports ?epds_handle_mode=random for consent preview', async () => { + const handler = createPreviewConsentHandler({ + trustedClients: [], + resolveClientMetadata: () => Promise.resolve({}), + getClientCss: () => null, + logger: mockLogger(), + })! + const res = mockRes() + await handler({ query: { epds_handle_mode: 'random' } }, res) + + expect(res.body).toContain( + '', + ) + }) + it('resolves client metadata and injects CSS for custom client_id', async () => { const trusted = 'https://trusted.example/client-metadata.json' const resolveClientMetadata = vi.fn(() => diff --git a/packages/pds-core/src/chooser-enrichment.ts b/packages/pds-core/src/chooser-enrichment.ts index b9369c91..887d1bf8 100644 --- a/packages/pds-core/src/chooser-enrichment.ts +++ b/packages/pds-core/src/chooser-enrichment.ts @@ -244,11 +244,43 @@ export function buildChooserEnrichmentScript(): string { if (!parent) return false; var tagName = (el.tagName || '').toLowerCase(); if (tagName !== 'b' && tagName !== 'strong') return false; - return true; + return hasApprovedConsentIdentityContext(el, parent); + } + + function normalizeConsentContextText(text) { + return (text || '').replace(/\\s+/g, ' ').trim(); + } + + function textAroundChild(parent, child) { + var before = ''; + var after = ''; + var seenChild = false; + for (var i = 0; i < parent.childNodes.length; i++) { + var node = parent.childNodes[i]; + if (node === child) { + seenChild = true; + continue; + } + var text = node.nodeType === Node.TEXT_NODE ? node.data : (node.textContent || ''); + if (seenChild) after += text; + else before += text; + } + return { + before: normalizeConsentContextText(before), + after: normalizeConsentContextText(after), + }; + } + + function hasApprovedConsentIdentityContext(el, parent) { + var context = textAroundChild(parent, el); + if (context.after !== 'account') return false; + if (context.before === 'Grant access to your') return true; + if (context.before === 'wants to access your') return true; + return /^.+ wants to access your$/.test(context.before); } function enrichConsentIdentity(accounts, handleMode) { - if (!isOauthAuthorizePage()) return; + if (!isConsentLikePage()) return; var selected = selectedAccount(accounts); var consentAccounts = selected ? [selected] : accounts; var root = document.getElementById('root'); @@ -282,6 +314,22 @@ export function buildChooserEnrichmentScript(): string { return window.location && window.location.pathname === '/oauth/authorize'; } + function isPreviewChooserPage() { + return window.location && window.location.pathname === '/preview/chooser'; + } + + function isPreviewConsentPage() { + return window.location && window.location.pathname === '/preview/consent'; + } + + function isChooserLikePage() { + return isOauthAuthorizePage() || isPreviewChooserPage(); + } + + function isConsentLikePage() { + return isOauthAuthorizePage() || isPreviewConsentPage() || isPreviewChooserPage(); + } + function isAccountPage() { var pathname = (window.location && window.location.pathname) || ''; return pathname === '/account' || pathname.indexOf('/account/') === 0; @@ -373,11 +421,11 @@ export function buildChooserEnrichmentScript(): string { // via a MutationObserver because the SPA hydrates/re-renders after // initial HTML delivery. function enrich() { - if (!isOauthAuthorizePage() && !isAccountPage()) return; + if (!isChooserLikePage() && !isPreviewConsentPage() && !isAccountPage()) return; var accounts = buildAccounts(); if (!accounts.length) return; var handleMode = readHandleMode(); - var hideHandle = handleMode === 'random' && isOauthAuthorizePage(); + var hideHandle = handleMode === 'random' && isChooserLikePage(); enrichConsentIdentity(accounts, handleMode); diff --git a/packages/pds-core/src/lib/preview-consent.ts b/packages/pds-core/src/lib/preview-consent.ts index d80260e5..917e4dbb 100644 --- a/packages/pds-core/src/lib/preview-consent.ts +++ b/packages/pds-core/src/lib/preview-consent.ts @@ -37,7 +37,15 @@ * routes' relaxed CSP. See {@link ./preview-shared.ts} for the shared * header set. */ -import { escapeHtml, renderPreviewIndexPage } from '@certified-app/shared' +import { + escapeHtml, + renderPreviewIndexPage, + resolveHandleMode, + VALID_HANDLE_MODES, + type ClientMetadata, + type HandleMode, +} from '@certified-app/shared' +import { buildChooserEnrichmentScript } from '../chooser-enrichment.js' import { DEFAULT_BRANDING_CSS } from './default-branding.js' import { applyPreviewHeaders, @@ -53,6 +61,10 @@ import { type ResponseLike, } from './preview-shared.js' +interface PreviewConsentFixture extends PreviewAuthorizeFixture { + handleMode: HandleMode +} + function buildSessions(): unknown { // Fixture session that drives the SPA straight to the consent screen: // `selected && !loginRequired && consentRequired` is the exact gate in @@ -75,7 +87,7 @@ function buildSessions(): unknown { /** Build the HTML page that the real oauth-provider SPA boots from. */ async function renderConsentHtml(opts: { - fixture: PreviewAuthorizeFixture + fixture: PreviewConsentFixture injectedCss: string | null }): Promise { const { scripts, styles } = await loadAssetRefs() @@ -102,6 +114,8 @@ async function renderConsentHtml(opts: { ? `` : '' const injectedStyle = `${defaultStyle}${clientStyle}` + const handleModeMeta = `` + const enrichmentScript = `` return ` @@ -109,11 +123,13 @@ async function renderConsentHtml(opts: { + ${handleModeMeta} Consent preview — ${escapeHtml(opts.fixture.clientId)} ${styleLinks} ${injectedStyle} + ${enrichmentScript}
@@ -125,6 +141,23 @@ async function renderConsentHtml(opts: { type PreviewConsentDeps = PreviewMetadataDeps +function resolveQueryHandleMode( + req: RequestLike, + metadata: ClientMetadata, +): HandleMode { + const queryMode = + typeof req.query.epds_handle_mode === 'string' + ? req.query.epds_handle_mode + : undefined + const rawMetaMode = metadata.epds_handle_mode + const metaMode = + typeof rawMetaMode === 'string' && + (VALID_HANDLE_MODES as readonly string[]).includes(rawMetaMode) + ? rawMetaMode + : undefined + return resolveHandleMode(queryMode, metaMode) +} + /** * Express handler factory: creates a GET /preview/consent handler if the * env var is on, returns null otherwise so the caller can skip wiring. @@ -142,10 +175,13 @@ export function createPreviewConsentHandler( 'Preview consent', ) - const fixture: PreviewAuthorizeFixture = { + const handleMode = resolveQueryHandleMode(req, metadata) + + const fixture: PreviewConsentFixture = { clientId, clientMetadata: metadata, isTrusted: deps.trustedClients.includes(clientId), + handleMode, } const html = await renderConsentHtml({ fixture, injectedCss }) From 391b1feddfe42369873821a86b2df6e9e4f11c5f Mon Sep 17 00:00:00 2001 From: kzoeps Date: Mon, 4 May 2026 16:50:09 +0600 Subject: [PATCH 05/12] fix(pds-core): keep consent tooltip open on first tap --- .beads/issues.jsonl | 1 + e2e/step-definitions/consent.steps.ts | 93 +++++++++++++++++++ features/consent-screen.feature | 12 +++ .../src/__tests__/chooser-enrichment.test.ts | 71 ++++++++++++-- packages/pds-core/src/chooser-enrichment.ts | 7 +- 5 files changed, 172 insertions(+), 12 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 552a3d89..b7c1c108 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -261,6 +261,7 @@ {"id":"ePDS-ipm.7","title":"Update recovery.test.ts to use dynamic OTP length instead of hardcoded 8","description":"## Files\n- packages/auth-service/src/__tests__/recovery.test.ts (modify)\n\n## What to do\n\nThere are two test blocks that hardcode OTP length to 8:\n\n### Block 1 (lines 122-128)\n```ts\ndescribe('Recovery flow: OTP pattern (8 digits)', () =\u003e {\n it('OTP sent by better-auth is 8 digits (configured in emailOTP plugin)', () =\u003e {\n const OTP_LENGTH = 8\n const pattern = new RegExp(`^[0-9]{${OTP_LENGTH}}$`)\n // ...\n })\n})\n```\n\n### Block 2 (lines 136-142)\n```ts\nit('OTP entry form uses maxlength=8 and pattern [0-9]{8}', () =\u003e {\n const maxlength = 8\n const pattern = '[0-9]{8}'\n expect(maxlength).toBe(8)\n expect(pattern).toContain('8')\n})\n```\n\n### Changes needed\n\nFor both blocks, read the OTP length from `process.env.OTP_LENGTH` with default 8:\n```ts\nconst OTP_LENGTH = parseInt(process.env.OTP_LENGTH ?? '8', 10)\n```\n\nUpdate the describe/it descriptions to not hardcode '8':\n- `'Recovery flow: OTP pattern'` (drop '8 digits')\n- `'OTP sent by better-auth matches configured length'`\n- `'OTP entry form uses configured maxlength and pattern'`\n\nUpdate Block 2 assertions to use the dynamic value:\n```ts\nconst maxlength = OTP_LENGTH\nconst pattern = `[0-9]{${OTP_LENGTH}}`\nexpect(maxlength).toBe(OTP_LENGTH)\nexpect(pattern).toContain(String(OTP_LENGTH))\n```\n\n## Don't\n- Don't change any other test files\n- Don't import from better-auth internals — just read process.env","acceptance_criteria":"1. No hardcoded 8 remains in OTP-related test assertions or descriptions\n2. Both test blocks read OTP_LENGTH from process.env with default 8\n3. Test descriptions are updated to not mention '8 digits'\n4. pnpm test passes with default OTP_LENGTH (unset = 8)","status":"closed","priority":2,"issue_type":"task","assignee":"karma.gainforest.id","owner":"karma.gainforest.id","estimated_minutes":20,"created_at":"2026-03-11T22:24:40.027924118+06:00","created_by":"karma.gainforest.id","updated_at":"2026-03-11T22:26:23.690810039+06:00","closed_at":"2026-03-11T22:26:23.690810039+06:00","close_reason":"627028c Use dynamic OTP_LENGTH in recovery tests instead of hardcoded 8","labels":["scope:trivial"],"dependencies":[{"issue_id":"ePDS-ipm.7","depends_on_id":"ePDS-ipm","type":"parent-child","created_at":"2026-03-11T22:24:40.029555266+06:00","created_by":"karma.gainforest.id"}]} {"id":"ePDS-ipm.8","title":"Document OTP_LENGTH env var in .env.example","description":"## Files\n- packages/auth-service/.env.example (modify)\n\n## What to do\n\nAdd the OTP_LENGTH env var documentation. Place it in the '-- Auth-service only --' section, after the session settings block (after line 61, before the social login section):\n\n```\n# OTP code length — number of digits in the email verification code (default: 8)\n# Must be between 4 and 12. Applies to login, recovery, and account settings OTP flows.\n# OTP_LENGTH=8\n```\n\nThe variable is commented out (like other optional vars in this file) since the default of 8 is sensible.\n\n## Don't\n- Don't modify any other .env.example files (the root one, pds-core, or demo)\n- Don't change any existing lines","acceptance_criteria":"1. OTP_LENGTH is documented in packages/auth-service/.env.example\n2. It's in the auth-service-only section, near the session settings\n3. Comment explains the valid range (4-12) and default (8)\n4. The variable line is commented out (# OTP_LENGTH=8)\n5. No existing lines are modified","status":"closed","priority":3,"issue_type":"task","assignee":"karma.gainforest.id","owner":"karma.gainforest.id","estimated_minutes":10,"created_at":"2026-03-11T22:24:48.428248126+06:00","created_by":"karma.gainforest.id","updated_at":"2026-03-11T22:26:17.022688894+06:00","closed_at":"2026-03-11T22:26:17.022688894+06:00","close_reason":"b8d6f58 docs: document OTP_LENGTH env var in auth-service .env.example","labels":["scope:trivial"],"dependencies":[{"issue_id":"ePDS-ipm.8","depends_on_id":"ePDS-ipm","type":"parent-child","created_at":"2026-03-11T22:24:48.42974985+06:00","created_by":"karma.gainforest.id"}]} {"id":"ePDS-ix2","title":"Fix: recovery .link-btn has padding:0 — touch target too small on mobile (from ePDS-aq2.5)","description":"Review of ePDS-aq2.5 found: The .link-btn CSS rule in recovery.ts (line 636) sets padding: 0, which means the touch target for 'Back to sign in' and 'Resend Code' links is only the text height (~20px). WCAG 2.5.5 recommends a minimum 44×44px touch target.\n\nEvidence:\n- recovery.ts line 636: padding: 0; (inside .link-btn rule)\n- The same issue exists in login-page.ts (line 565: padding: 0)\n\nThe .link-btn elements appear in:\n1. .form-actions row: 'Back to sign in' link (line 246)\n2. .otp-links row: 'Resend Code' button and 'Back to sign in' link (lines 319, 321)\n\nThese are the only interactive elements in those rows, so small touch targets are a real usability issue on mobile.\n\nFix: Add padding: 8px 0 (or similar) to .link-btn to increase the touch target height to at least 36px, while keeping the visual appearance the same. This fix should be applied to both recovery.ts and login-page.ts for consistency. Note: this is a pre-existing issue in login-page.ts that was copied into recovery.ts.","status":"open","priority":3,"issue_type":"bug","owner":"karma.gainforest.id","created_at":"2026-03-04T15:59:41.944209123+06:00","created_by":"karma.gainforest.id","updated_at":"2026-03-04T15:59:41.944209123+06:00","dependencies":[{"issue_id":"ePDS-ix2","depends_on_id":"ePDS-aq2.5","type":"discovered-from","created_at":"2026-03-04T15:59:42.002820695+06:00","created_by":"karma.gainforest.id"}]} +{"id":"ePDS-j448","title":"Epic: E2E coverage for consent and chooser enrichment","description":"Add end-to-end coverage for the user-facing enrichment introduced on this branch. Success means real OAuth consent screens assert identity enrichment in random and picker modes, and the session-reuse chooser profile asserts non-random chooser rows keep handle and email visible. Constraints: do not add preview-route e2e coverage; do not add multi-account/exact-matching e2e coverage in this scope; avoid coupling random consent coverage to random account provisioning risk. Known risks: consent DOM is upstream-owned and can be fragile; keep assertions anchored on visible role/text and ePDS stable classes.","status":"open","priority":2,"issue_type":"epic","owner":"kzoepa@gmail.com","created_at":"2026-05-04T17:17:28.800992481+06:00","created_by":"kzoeps","updated_at":"2026-05-04T17:17:28.800992481+06:00","labels":["scope:medium"]} {"id":"ePDS-j825","title":"Fix: renderNoAccountPage() response missing HTTP status code (from ePDS-01p.3)","description":"Review of ePDS-01p.3 found: In account-settings.ts line 69, when no PDS account is found for the session email, the response is sent as 'res.type(\"html\").send(renderNoAccountPage())' without setting an HTTP status code. This defaults to 200 OK, which is semantically incorrect for an error page and may confuse clients, monitoring tools, or browser caching. The 503 branch (PDS unavailable) correctly uses res.status(503), but the no-account branch silently returns 200. Fix: add an appropriate status code — either 403 Forbidden (the session is valid but the user has no account) or 404 Not Found. Suggested: res.status(403).type(\"html\").send(renderNoAccountPage()). Evidence: account-settings.ts line 69 — no .status() call before .type('html').send(...).","status":"open","priority":2,"issue_type":"bug","owner":"kzoepa@gmail.com","created_at":"2026-03-19T16:12:47.885869077+06:00","created_by":"kzoeps","updated_at":"2026-03-19T16:12:47.885869077+06:00","dependencies":[{"issue_id":"ePDS-j825","depends_on_id":"ePDS-01p.3","type":"discovered-from","created_at":"2026-03-19T16:13:23.934385443+06:00","created_by":"kzoeps"}]} {"id":"ePDS-jbs","title":"Fix: dark mode body background overridden by light-mode rule ordering (from ePDS-wae.2)","description":"The CSS has two body background rules: (1) body { background: var(--color-contrast-0); } at line ~364 and (2) @media (prefers-color-scheme: light) { body { background: white; } } at line ~371. In dark mode, rule (1) correctly uses --color-contrast-0 which resolves to hsl(265 20% 6%) — a near-black. However, when AUTH_BACKGROUND_COLOR is set, the bgColorStyle injects a \u003cstyle\u003ebody { background: ... !important; }\u003c/style\u003e block. The !important correctly overrides both rules. But if a client's background_color is set via OAuth metadata (b.background_color), it is NOT injected into the bgColorStyle block — it is only set via --color-panel (which only affects the panel, not body). The body background_color from client OAuth metadata is silently ignored. Document this limitation or implement it.","status":"open","priority":2,"issue_type":"bug","owner":"karma.gainforest.id","created_at":"2026-03-03T20:16:17.449655765+06:00","created_by":"karma.gainforest.id","updated_at":"2026-03-03T20:16:17.449655765+06:00","dependencies":[{"issue_id":"ePDS-jbs","depends_on_id":"ePDS-wae.2","type":"discovered-from","created_at":"2026-03-03T20:17:56.359797404+06:00","created_by":"karma.gainforest.id"}]} {"id":"ePDS-jk0","title":"Bug: recovery GET requires request_uri query param but plan says cookie should be sufficient","description":"## Finding\n\n`packages/auth-service/src/routes/recovery.ts` lines 42-44 return HTTP 400 if `request_uri` is absent from the query string:\n\n```ts\nif (!requestUri) {\n res.status(400).send(renderError('Missing request_uri parameter'))\n return\n}\n```\n\nThe login page recovery link at `login-page.ts:368` currently passes `request_uri` and `client_id` as query params to work around this:\n\n```ts\n\u003ca href=\"/auth/recover?request_uri=${...}\\\u0026client_id=${...}\" ...\u003e\n```\n\nThis is the current working state, but it means:\n\n1. The recovery page is reachable only via the login page link (which includes the params). Direct navigation or a stale link without params returns 400.\n2. The plan to simplify the recovery link to just `/auth/recover` (change #5) would break the GET handler unless change #4 (read from cookie instead) is implemented first. These two changes are tightly coupled and must be implemented atomically.\n3. The fallback at lines 47-51 (`getAuthFlowByRequestUri`) only runs when `clientId` is missing but `requestUri` is present — it never fires when `requestUri` itself is absent.\n\n## Not a regression — this is the current design\n\nThis is not a regression from any recent change. It is a pre-existing coupling that the refactoring plan must address carefully. The plan correctly identifies this (change #4), but the implementation must ensure the GET handler is updated before or simultaneously with simplifying the login page link (change #5). If done in two separate commits, the intermediate state would break recovery.","status":"open","priority":2,"issue_type":"bug","owner":"karma.gainforest.id","created_at":"2026-03-18T17:11:00.440621594+06:00","created_by":"karma.gainforest.id","updated_at":"2026-03-18T17:11:00.440621594+06:00"} diff --git a/e2e/step-definitions/consent.steps.ts b/e2e/step-definitions/consent.steps.ts index 5982b7a3..ec36bcfd 100644 --- a/e2e/step-definitions/consent.steps.ts +++ b/e2e/step-definitions/consent.steps.ts @@ -35,6 +35,36 @@ 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.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + Then('a consent screen is displayed', async function (this: EpdsWorld) { const page = getPage(this) @@ -99,6 +129,69 @@ 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) + await expect(page.getByText(requireScenarioEmail(this))).toBeVisible() + }, +) + +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 tooltipControl = page.getByRole('button', { + name: 'Identity information', + }) + + await expect(tooltipControl).toHaveAttribute('type', 'button') + await expect(tooltipControl).toHaveAttribute('aria-expanded', 'false') + const describedBy = await tooltipControl.getAttribute('aria-describedby') + expect(describedBy?.trim()).toBeTruthy() + + await tooltipControl.click() + await expect(tooltipControl).toHaveAttribute('aria-expanded', 'true') + + const tooltip = page.locator(`#${describedBy?.trim()}`) + await expect(tooltip).toHaveAttribute('role', 'tooltip') + await expect(tooltip).toBeVisible() + await expect(tooltip).toContainText('Public AT Protocol handle:') + await expect(tooltip).toContainText(publicHandle) + }, +) + +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(`\\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) { diff --git a/features/consent-screen.feature b/features/consent-screen.feature index 69871b57..830b9d64 100644 --- a/features/consent-screen.feature +++ b/features/consent-screen.feature @@ -72,6 +72,18 @@ 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: 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 diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index 95fe6dbf..a446d9c1 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -414,6 +414,24 @@ function createConsentIdentity( return { container, identifier } } +function findConsentIdentityTooltip(container: FakeElement): { + icon: FakeElement + tooltip: FakeElement +} { + const icon = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-info-icon'), + ) as FakeElement + const tooltip = container.childNodes.find( + (child) => + child instanceof FakeElement && + child.classList.contains('epds-identity-tooltip'), + ) as FakeElement + + return { icon, tooltip } +} + function selectedAliceSession(preferredUsername = 'alice.test'): unknown[] { return [ { @@ -1164,31 +1182,64 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) - const icon = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-info-icon'), - ) as FakeElement - const tooltip = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-tooltip'), - ) as FakeElement + const { icon, tooltip } = findConsentIdentityTooltip(container) expect(tooltip.hidden).toBe(true) + expect(icon.getAttribute('aria-expanded')).toBe('false') icon.dispatchEvent('mouseenter') expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') icon.dispatchEvent('mouseleave') expect(tooltip.hidden).toBe(true) + expect(icon.getAttribute('aria-expanded')).toBe('false') icon.dispatchEvent('focus') expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') icon.dispatchEvent('blur') expect(tooltip.hidden).toBe(true) + expect(icon.getAttribute('aria-expanded')).toBe('false') const click = icon.dispatchEvent('click') expect(click.defaultPrevented).toBe(true) + expect(click.propagationStopped).toBe(true) + expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') + icon.dispatchEvent('mouseleave') + expect(tooltip.hidden).toBe(false) + icon.dispatchEvent('blur') expect(tooltip.hidden).toBe(false) icon.dispatchEvent('click') expect(tooltip.hidden).toBe(true) + expect(icon.getAttribute('aria-expanded')).toBe('false') + }) + + it('keeps the tooltip pinned open when touch focus fires before click', () => { + const document = new FakeDocument() + const { container } = createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + const { icon, tooltip } = findConsentIdentityTooltip(container) + + icon.dispatchEvent('focus') + expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') + + icon.dispatchEvent('click') + expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') + + icon.dispatchEvent('blur') + expect(tooltip.hidden).toBe(false) + expect(icon.getAttribute('aria-expanded')).toBe('true') + + icon.dispatchEvent('click') + expect(tooltip.hidden).toBe(true) + expect(icon.getAttribute('aria-expanded')).toBe('false') }) it('does not apply consent tooltip behavior on account pages', () => { diff --git a/packages/pds-core/src/chooser-enrichment.ts b/packages/pds-core/src/chooser-enrichment.ts index 887d1bf8..c6ff193e 100644 --- a/packages/pds-core/src/chooser-enrichment.ts +++ b/packages/pds-core/src/chooser-enrichment.ts @@ -230,9 +230,12 @@ export function buildChooserEnrichmentScript(): string { icon.addEventListener('click', function(e) { e.preventDefault(); e.stopPropagation(); - pinned = tooltip.hidden; + pinned = !pinned; if (pinned) show(); - else hide(); + else { + tooltip.hidden = true; + icon.setAttribute('aria-expanded', 'false'); + } }); el.insertAdjacentElement('afterend', tooltip); From 6fa92f02b344c22d7d28dce48767973d11716260 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Tue, 5 May 2026 11:14:47 +0600 Subject: [PATCH 06/12] fix(pds-core): resolve handle mode from OAuth client metadata --- .../generated-handle-account-recognition.md | 11 + .changeset/scoped-account-enrichment.md | 9 - e2e/step-definitions/consent.steps.ts | 36 +- .../session-reuse-bugs.steps.ts | 13 +- .../__tests__/callback-handle-mode.test.ts | 216 ++++++++ .../auth-service/src/routes/choose-handle.ts | 1 + packages/auth-service/src/routes/complete.ts | 62 ++- .../src/__tests__/chooser-enrichment.test.ts | 487 +++++++++++------- .../__tests__/epds-callback-authorize.test.ts | 33 ++ packages/pds-core/src/chooser-enrichment.ts | 63 ++- packages/pds-core/src/index.ts | 31 +- .../pds-core/src/lib/client-css-injection.ts | 38 +- .../src/lib/epds-callback-authorize.ts | 26 + .../pds-core/src/lib/oauth-request-context.ts | 37 ++ 14 files changed, 785 insertions(+), 278 deletions(-) create mode 100644 .changeset/generated-handle-account-recognition.md delete mode 100644 .changeset/scoped-account-enrichment.md create mode 100644 packages/auth-service/src/__tests__/callback-handle-mode.test.ts create mode 100644 packages/pds-core/src/__tests__/epds-callback-authorize.test.ts create mode 100644 packages/pds-core/src/lib/epds-callback-authorize.ts create mode 100644 packages/pds-core/src/lib/oauth-request-context.ts diff --git a/.changeset/generated-handle-account-recognition.md b/.changeset/generated-handle-account-recognition.md new file mode 100644 index 00000000..349f6724 --- /dev/null +++ b/.changeset/generated-handle-account-recognition.md @@ -0,0 +1,11 @@ +--- +'ePDS': patch +--- + +Accounts with automatically generated handles are easier to recognize during sign-in and app approval. + +**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 generated-handle accounts while keeping the public handle available where it helps explain the account. The final approval step no longer briefly exposes the generated random handle when an app requested email-first presentation. + +**Client app developers:** `epds_handle_mode` is now applied consistently to pds-core-rendered approval and chooser pages from either the current `/oauth/authorize` query parameter or OAuth client metadata, including pushed authorization requests where the current 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, and metadata lookup failures fall back to the existing default behavior. diff --git a/.changeset/scoped-account-enrichment.md b/.changeset/scoped-account-enrichment.md deleted file mode 100644 index be7a2aa6..00000000 --- a/.changeset/scoped-account-enrichment.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -'ePDS': patch ---- - -Account choices are easier to recognize during sign-in and app approval. - -**Affects:** End users - -**End users:** Sign-in, app approval, and account-management screens now make it easier to identify accounts by email while preserving public handles where they are helpful. Authorization screens for generated-handle accounts show email as the primary identifier and keep the generated handle available through an accessible explanation. diff --git a/e2e/step-definitions/consent.steps.ts b/e2e/step-definitions/consent.steps.ts index ec36bcfd..aaf92839 100644 --- a/e2e/step-definitions/consent.steps.ts +++ b/e2e/step-definitions/consent.steps.ts @@ -62,7 +62,7 @@ function formatRawPublicHandle(handle: string): string { } function escapeRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + return value.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`) } Then('a consent screen is displayed', async function (this: EpdsWorld) { @@ -144,7 +144,23 @@ Then( 'the consent page shows the email as the primary account identifier', async function (this: EpdsWorld) { const page = getPage(this) - await expect(page.getByText(requireScenarioEmail(this))).toBeVisible() + const scenarioEmail = requireScenarioEmail(this) + const grantAccessText = new RegExp( + String.raw`\bGrant\s+access\s+to\s+your\b`, + ) + const accountCardText = new RegExp( + String.raw`\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) }, ) @@ -153,19 +169,23 @@ Then( async function (this: EpdsWorld) { const page = getPage(this) const publicHandle = formatPublicHandle(requireScenarioHandle(this)) - const tooltipControl = page.getByRole('button', { - name: 'Identity information', - }) + const tooltipControl = page + .getByRole('main') + .getByRole('button', { name: 'Identity information' }) 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(`#${describedBy?.trim()}`) + const tooltip = page.locator(`#${tooltipId}`) await expect(tooltip).toHaveAttribute('role', 'tooltip') await expect(tooltip).toBeVisible() await expect(tooltip).toContainText('Public AT Protocol handle:') @@ -183,7 +203,9 @@ Then( formatRawPublicHandle(scenarioHandle), ].map( (publicHandle) => - new RegExp(`\\byour\\s+${escapeRegex(publicHandle)}\\s+account\\b`), + new RegExp( + String.raw`\byour\s+${escapeRegex(publicHandle)}\s+account\b`, + ), ) for (const primaryHandlePattern of primaryHandlePatterns) { diff --git a/e2e/step-definitions/session-reuse-bugs.steps.ts b/e2e/step-definitions/session-reuse-bugs.steps.ts index 26b875fa..d946a9ff 100644 --- a/e2e/step-definitions/session-reuse-bugs.steps.ts +++ b/e2e/step-definitions/session-reuse-bugs.steps.ts @@ -425,6 +425,7 @@ type HiddenHandleDescriptionRow = { text: string }[] emailTitle: string | null + hiddenHandleText: string rowIndex: number } @@ -446,9 +447,10 @@ Then( if (!row || !handleLabel) return null const handleIsHidden = - window.getComputedStyle(handleLabel).display === 'none' + 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) ?? [] @@ -468,6 +470,7 @@ Then( describedBy, descriptions, emailTitle: emailLabel.getAttribute('title'), + hiddenHandleText, rowIndex, } }) @@ -496,9 +499,13 @@ Then( prefixIndex, `Row ${row.rowIndex}: expected hidden-handle description to contain "${prefix}", got "${descriptionText}"`, ).toBeGreaterThanOrEqual(0) + const describedHiddenHandle = descriptionText + .slice(prefixIndex + prefix.length) + .trim() expect( - descriptionText.slice(prefixIndex + prefix.length).trim().length, - ).toBeGreaterThan(0) + describedHiddenHandle, + `Row ${row.rowIndex}: expected hidden-handle description suffix to match the hidden handle text`, + ).toBe(row.hiddenHandleText) expect( row.emailTitle, diff --git a/packages/auth-service/src/__tests__/callback-handle-mode.test.ts b/packages/auth-service/src/__tests__/callback-handle-mode.test.ts new file mode 100644 index 00000000..23a50566 --- /dev/null +++ b/packages/auth-service/src/__tests__/callback-handle-mode.test.ts @@ -0,0 +1,216 @@ +import { randomBytes } from 'node:crypto' +import type { AddressInfo } from 'node:net' +import express from 'express' +import cookieParser from 'cookie-parser' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { HandleMode } from '@certified-app/shared' +import type { AuthServiceContext } from '../context.js' +import { createChooseHandleRouter } from '../routes/choose-handle.js' +import { createCompleteRouter } from '../routes/complete.js' + +const mocks = vi.hoisted(() => ({ + getDidByEmail: vi.fn(), + pingParRequest: vi.fn(), + resolveRecoveryEmail: vi.fn(), + resolveClientBranding: vi.fn(), +})) + +vi.mock('../lib/get-did-by-email.js', () => ({ + getDidByEmail: mocks.getDidByEmail, +})) + +vi.mock('../lib/ping-par-request.js', () => ({ + pingParRequest: mocks.pingParRequest, +})) + +vi.mock('../lib/resolve-recovery-email.js', () => ({ + resolveRecoveryEmail: mocks.resolveRecoveryEmail, +})) + +vi.mock('../lib/client-metadata.js', () => ({ + resolveClientBranding: mocks.resolveClientBranding, +})) + +const AUTH_FLOW_COOKIE = 'epds_auth_flow' +const realFetch = globalThis.fetch.bind(globalThis) + +function makeCtx(handleMode: HandleMode | null): AuthServiceContext { + return { + config: { + pdsPublicUrl: 'https://pds.example', + pdsHostname: 'pds.example', + epdsCallbackSecret: 'test-callback-secret', + trustedClients: [], + }, + db: { + getAuthFlow: vi.fn(() => ({ + flowId: 'flow-1', + requestUri: 'urn:ietf:params:oauth:request_uri:req-123', + clientId: 'https://app.example/client.json', + handleMode, + })), + deleteAuthFlow: vi.fn(), + }, + } as unknown as AuthServiceContext +} + +function makeAuth() { + return { + api: { + getSession: vi.fn(() => + Promise.resolve({ user: { email: 'Alice@example.com' } }), + ), + }, + } +} + +async function startApp( + ctx: AuthServiceContext, + auth: ReturnType, +): Promise<{ baseUrl: string; close: () => Promise }> { + const app = express() + app.disable('x-powered-by') + app.use(cookieParser()) + app.use(express.urlencoded({ extended: false })) + app.use(createCompleteRouter(ctx, auth)) + app.use(createChooseHandleRouter(ctx, auth)) + + const server = app.listen(0) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.once('listening', () => { + resolve() + }) + }) + server.unref() + const port = (server.address() as AddressInfo).port + return { + baseUrl: `http://127.0.0.1:${port}`, + close: () => + new Promise((resolve) => { + server.close(() => { + resolve() + }) + }), + } +} + +function parseRedirect(res: globalThis.Response): URL { + const location = res.headers.get('location') + if (!location) throw new Error('Missing redirect location') + return new URL(location) +} + +function normalizeFetchUrl(input: Parameters[0]): URL { + if (input instanceof URL) return input + if (typeof input === 'string') return new URL(input) + return new URL(input.url) +} + +describe('auth-service epds-callback handle mode threading', () => { + let priorEnv: { pdsInternalUrl?: string; internalSecret?: string } + + beforeEach(() => { + priorEnv = { + pdsInternalUrl: process.env.PDS_INTERNAL_URL, + internalSecret: process.env.EPDS_INTERNAL_SECRET, + } + process.env.PDS_INTERNAL_URL = 'http://pds.internal' // NOSONAR test-only internal mocked URL + process.env.EPDS_INTERNAL_SECRET = 'test-internal-secret' + mocks.getDidByEmail.mockReset() + mocks.pingParRequest.mockReset() + mocks.resolveRecoveryEmail.mockReset() + mocks.resolveClientBranding.mockReset() + mocks.pingParRequest.mockResolvedValue({ ok: true }) + mocks.resolveRecoveryEmail.mockResolvedValue(null) + mocks.resolveClientBranding.mockResolvedValue({ + customCss: null, + customFaviconUrl: null, + customFaviconUrlDark: null, + }) + vi.stubGlobal( + 'fetch', + vi.fn((input: Parameters[0], init?: RequestInit) => { + const url = normalizeFetchUrl(input) + if (url.hostname === '127.0.0.1') return realFetch(input, init) + return Promise.resolve({ + ok: true, + json: () => Promise.resolve({ exists: false }), + }) + }), + ) + }) + + afterEach(() => { + if (priorEnv.pdsInternalUrl === undefined) + delete process.env.PDS_INTERNAL_URL + else process.env.PDS_INTERNAL_URL = priorEnv.pdsInternalUrl + if (priorEnv.internalSecret === undefined) + delete process.env.EPDS_INTERNAL_SECRET + else process.env.EPDS_INTERNAL_SECRET = priorEnv.internalSecret + vi.unstubAllGlobals() + }) + + it('includes the stored canonical handle mode for random-mode new users', async () => { + mocks.getDidByEmail.mockResolvedValue(null) + const app = await startApp(makeCtx('random'), makeAuth()) + try { + const res = await fetch(`${app.baseUrl}/auth/complete`, { + redirect: 'manual', + headers: { cookie: `${AUTH_FLOW_COOKIE}=flow-1` }, + }) + + expect(res.status).toBe(303) + const url = parseRedirect(res) + expect(url.pathname).toBe('/oauth/epds-callback') + expect(url.searchParams.get('epds_handle_mode')).toBe('random') + expect(url.searchParams.has('handle')).toBe(false) + } finally { + await app.close() + } + }) + + it('includes the stored canonical handle mode for existing users', async () => { + mocks.getDidByEmail.mockResolvedValue(randomBytes(16).toString('hex')) + const app = await startApp(makeCtx('picker-with-random'), makeAuth()) + try { + const res = await fetch(`${app.baseUrl}/auth/complete`, { + redirect: 'manual', + headers: { cookie: `${AUTH_FLOW_COOKIE}=flow-1` }, + }) + + expect(res.status).toBe(303) + const url = parseRedirect(res) + expect(url.pathname).toBe('/oauth/epds-callback') + expect(url.searchParams.get('epds_handle_mode')).toBe( + 'picker-with-random', + ) + } finally { + await app.close() + } + }) + + it('includes the stored canonical handle mode for chosen-handle callbacks', async () => { + mocks.getDidByEmail.mockResolvedValue(null) + const app = await startApp(makeCtx('picker'), makeAuth()) + try { + const res = await fetch(`${app.baseUrl}/auth/choose-handle`, { + method: 'POST', + redirect: 'manual', + headers: { + cookie: `${AUTH_FLOW_COOKIE}=flow-1`, + 'content-type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ handle: 'Alice1' }), + }) + + expect(res.status).toBe(303) + const url = parseRedirect(res) + expect(url.pathname).toBe('/oauth/epds-callback') + expect(url.searchParams.get('epds_handle_mode')).toBe('picker') + expect(url.searchParams.get('handle')).toBe('alice1') + } finally { + await app.close() + } + }) +}) diff --git a/packages/auth-service/src/routes/choose-handle.ts b/packages/auth-service/src/routes/choose-handle.ts index fa987463..d6821e62 100644 --- a/packages/auth-service/src/routes/choose-handle.ts +++ b/packages/auth-service/src/routes/choose-handle.ts @@ -368,6 +368,7 @@ export function createChooseHandleRouter( ctx.config.epdsCallbackSecret, ) const params = new URLSearchParams({ ...callbackParams, ts, sig }) + if (flow.handleMode) params.set('epds_handle_mode', flow.handleMode) logger.info( { email, flowId, fullHandle }, diff --git a/packages/auth-service/src/routes/complete.ts b/packages/auth-service/src/routes/complete.ts index 7ec7873e..527e6d71 100644 --- a/packages/auth-service/src/routes/complete.ts +++ b/packages/auth-service/src/routes/complete.ts @@ -35,6 +35,35 @@ const logger = createLogger('auth:complete') const AUTH_FLOW_COOKIE = 'epds_auth_flow' +async function resolveCompleteIdentity( + email: string, + flowId: string, + ctx: AuthServiceContext, + pdsUrl: string, + internalSecret: string, +): Promise<{ email: string; did: string | null }> { + const did = await getDidByEmail(email, pdsUrl, internalSecret) + if (did) return { email, did } + + // Recovery path: session email is a backup email, not a primary. Resolve + // the backup-email -> DID mapping (auth-service-owned) and then DID -> + // primary email via pds-core's internal API, so the downstream callback + // signs the user's real account email, not the recovery address. + const recovered = await resolveRecoveryEmail( + email, + ctx, + pdsUrl, + internalSecret, + ) + if (!recovered) return { email, did: null } + + logger.info( + { flowId, did: recovered.did }, + 'Recovery: translated backup email to primary email via DID', + ) + return { email: recovered.email, did: recovered.did } +} + export function createCompleteRouter( ctx: AuthServiceContext, // eslint-disable-next-line @typescript-eslint/no-explicit-any -- better-auth instance has no exported type @@ -97,31 +126,16 @@ export function createCompleteRouter( return } - let email = session.user.email.toLowerCase() + const sessionEmail = session.user.email.toLowerCase() // Step 4: Check whether this is a new user (no PDS account for email). - let did = await getDidByEmail(email, pdsUrl, internalSecret) - - // Recovery path: session email is a backup email, not a primary. Resolve - // the backup-email → DID mapping (auth-service-owned) and then DID → - // primary email via pds-core's internal API, so the downstream callback - // signs the user's real account email, not the recovery address. - if (!did) { - const recovered = await resolveRecoveryEmail( - email, - ctx, - pdsUrl, - internalSecret, - ) - if (recovered) { - logger.info( - { flowId, did: recovered.did }, - 'Recovery: translated backup email to primary email via DID', - ) - email = recovered.email - did = recovered.did - } - } + const { email, did } = await resolveCompleteIdentity( + sessionEmail, + flowId, + ctx, + pdsUrl, + internalSecret, + ) const isNewAccount = !did @@ -166,6 +180,7 @@ export function createCompleteRouter( ctx.config.epdsCallbackSecret, ) const params = new URLSearchParams({ ...callbackParams, ts, sig }) + params.set('epds_handle_mode', flow.handleMode) logger.info( { email, flowId }, 'New user (random mode): skipping handle picker, redirecting to epds-callback', @@ -209,6 +224,7 @@ export function createCompleteRouter( ctx.config.epdsCallbackSecret, ) const params = new URLSearchParams({ ...callbackParams, ts, sig }) + if (flow.handleMode) params.set('epds_handle_mode', flow.handleMode) const redirectUrl = `${ctx.config.pdsPublicUrl}/oauth/epds-callback?${params.toString()}` logger.info( diff --git a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts index a446d9c1..c7244c02 100644 --- a/packages/pds-core/src/__tests__/chooser-enrichment.test.ts +++ b/packages/pds-core/src/__tests__/chooser-enrichment.test.ts @@ -284,16 +284,85 @@ function appendText(parent: FakeElement, text: string): void { parent.appendChild(new FakeTextNode(text)) } +const DEFAULT_CHOOSER_LOCATION = { + pathname: '/oauth/authorize', + search: '', +} + +const ALICE_ASSOCIATED_TOOLTIP = + 'This handle is associated with alice@example.test.' +const ASSOCIATED_TOOLTIP_PREFIX = 'This handle is associated' +const ALICE_PUBLIC_HANDLE_TOOLTIP = + 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.' +const EMAIL_LABEL_CLASS = 'epds-email-label' +const HIDDEN_HANDLE_DESCRIPTION_CLASS = 'epds-hidden-handle-description' +const IDENTITY_INFO_ICON_CLASS = 'epds-identity-info-icon' +const IDENTITY_TOOLTIP_CLASS = 'epds-identity-tooltip' + +function findChildWithClass( + parent: FakeElement, + className: string, +): FakeElement | undefined { + return parent.childNodes.find( + (child) => + child instanceof FakeElement && child.classList.contains(className), + ) as FakeElement | undefined +} + +function findEmailLabel(parent: FakeElement): FakeElement | undefined { + return findChildWithClass(parent, EMAIL_LABEL_CLASS) +} + +function findHiddenHandleDescription( + parent: FakeElement, +): FakeElement | undefined { + return findChildWithClass(parent, HIDDEN_HANDLE_DESCRIPTION_CLASS) +} + +function findDescendantsWithClass( + parent: FakeElement, + className: string, +): FakeElement[] { + return parent.descendants().filter((el) => el.classList.contains(className)) +} + +function expectConsentTooltip( + container: FakeElement, + expectedText: string, +): void { + const icon = findChildWithClass(container, IDENTITY_INFO_ICON_CLASS) + const tooltip = findChildWithClass(container, IDENTITY_TOOLTIP_CLASS) + + expect(icon).toBeInstanceOf(FakeElement) + expect(icon?.tagName).toBe('button') + expect(icon?.getAttribute('aria-describedby')).toBe(tooltip?.id) + expect(tooltip?.textContent).toBe(expectedText) +} + +function expectConsentTooltipTexts( + document: FakeDocument, + expectedTexts: string[], +): void { + const icons = findDescendantsWithClass( + document.root, + IDENTITY_INFO_ICON_CLASS, + ) + const tooltips = findDescendantsWithClass( + document.root, + IDENTITY_TOOLTIP_CLASS, + ) + + expect(icons).toHaveLength(expectedTexts.length) + expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual(expectedTexts) +} + function runChooserEnrichmentScript( document: FakeDocument, globals: { __sessions?: unknown[] __deviceSessions?: unknown[] } = {}, - location: { pathname: string; search?: string } = { - pathname: '/oauth/authorize', - search: '', - }, + location: { pathname: string; search?: string } = DEFAULT_CHOOSER_LOCATION, ): void { const fakeWindow: Record = { location, @@ -414,22 +483,83 @@ function createConsentIdentity( return { container, identifier } } +function createPreviewChooserConsentIdentities( + document: FakeDocument, + mode: string, +): { + sidebar: { container: FakeElement; identifier: FakeElement } + mainCard: { container: FakeElement; identifier: FakeElement } +} { + appendHandleModeMeta(document, mode) + + return { + sidebar: createConsentIdentity( + document, + 'Grant access to your ', + 'alice.test', + ' account', + ), + mainCard: createConsentIdentity( + document, + 'wants to access your ', + 'alice.test', + ' account', + ), + } +} + +function runPreviewChooserConsentEnrichment( + document: FakeDocument, + mode: string, +): void { + runChooserEnrichmentScript( + document, + { __sessions: selectedAliceSession() }, + { + pathname: '/preview/chooser', + search: `?epds_handle_mode=${mode}`, + }, + ) +} + +function expectArbitraryConsentProseUntouched({ + prefix, + identifierText, + suffix, + tagName, + expectedText, +}: { + prefix: string + identifierText: string + suffix: string + tagName?: string + expectedText: string +}): void { + const document = new FakeDocument() + appendHandleModeMeta(document, 'picker') + const { container, identifier } = createConsentIdentity( + document, + prefix, + identifierText, + suffix, + tagName, + ) + + runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) + + expect(identifier.textContent).toBe(identifierText) + expect(container.textContent).toBe(expectedText) + expect(container.textContent).not.toContain(ASSOCIATED_TOOLTIP_PREFIX) +} + function findConsentIdentityTooltip(container: FakeElement): { icon: FakeElement tooltip: FakeElement } { - const icon = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-info-icon'), - ) as FakeElement - const tooltip = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-tooltip'), - ) as FakeElement + const icon = findChildWithClass(container, IDENTITY_INFO_ICON_CLASS) + const tooltip = findChildWithClass(container, IDENTITY_TOOLTIP_CLASS) - return { icon, tooltip } + return { icon: icon as FakeElement, tooltip: tooltip as FakeElement } } function selectedAliceSession(preferredUsername = 'alice.test'): unknown[] { @@ -493,11 +623,7 @@ describe('buildChooserEnrichmentScript account row scoping', () => { runChooserEnrichmentScript(document) - const emailLabel = wrap.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-email-label'), - ) as FakeElement | undefined + const emailLabel = findEmailLabel(wrap) expect(emailLabel).toBeInstanceOf(FakeElement) expect(emailLabel?.textContent).toBe('alice@example.test') @@ -538,16 +664,8 @@ describe('buildChooserEnrichmentScript account row scoping', () => { runChooserEnrichmentScript(document) - const emailLabel = wrap.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-email-label'), - ) as FakeElement | undefined - const handleDescription = row.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-hidden-handle-description'), - ) as FakeElement | undefined + const emailLabel = findEmailLabel(wrap) + const handleDescription = findHiddenHandleDescription(row) expect(emailLabel?.textContent).toBe('alice@example.test') expect(handle.style.display).toBe('none') @@ -572,16 +690,8 @@ describe('buildChooserEnrichmentScript account row scoping', () => { { pathname: '/preview/chooser', search: '' }, ) - const emailLabel = wrap.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-email-label'), - ) as FakeElement | undefined - const handleDescription = row.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-hidden-handle-description'), - ) as FakeElement | undefined + const emailLabel = findEmailLabel(wrap) + const handleDescription = findHiddenHandleDescription(row) expect(emailLabel?.textContent).toBe('alice@example.test') expect(handle.style.display).toBe('none') @@ -834,24 +944,8 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { runChooserEnrichmentScript(document) - const icon = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-info-icon'), - ) as FakeElement | undefined - const tooltip = container.childNodes.find( - (child) => - child instanceof FakeElement && - child.classList.contains('epds-identity-tooltip'), - ) as FakeElement | undefined - expect(identifier.textContent).toBe('alice.test') - expect(icon).toBeInstanceOf(FakeElement) - expect(icon?.tagName).toBe('button') - expect(icon?.getAttribute('aria-describedby')).toBe(tooltip?.id) - expect(tooltip?.textContent).toBe( - 'This handle is associated with alice@example.test.', - ) + expectConsentTooltip(container, ALICE_ASSOCIATED_TOOLTIP) }) it('enriches client-wants-access consent identity copy', () => { @@ -870,9 +964,7 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { }) expect(identifier.textContent).toBe('alice.test') - expect(container.textContent).toContain( - 'This handle is associated with alice@example.test.', - ) + expect(container.textContent).toContain(ALICE_ASSOCIATED_TOOLTIP) }) it('enriches upstream main-card consent identity copy without same-paragraph client name', () => { @@ -892,15 +984,13 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { const iconIndex = container.childNodes.findIndex( (child) => child instanceof FakeElement && - child.classList.contains('epds-identity-info-icon'), + child.classList.contains(IDENTITY_INFO_ICON_CLASS), ) const identifierIndex = container.childNodes.indexOf(identifier) expect(identifier.textContent).toBe('alice.test') expect(iconIndex).toBe(identifierIndex + 1) - expect(container.textContent).toContain( - 'This handle is associated with alice@example.test.', - ) + expect(container.textContent).toContain(ALICE_ASSOCIATED_TOOLTIP) }) it('enriches sidebar and main-card consent identities on the same page', () => { @@ -925,19 +1015,11 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { { pathname: '/preview/consent', search: '' }, ) - const icons = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-info-icon')) - const tooltips = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-tooltip')) - expect(sidebar.identifier.textContent).toBe('alice.test') expect(mainCard.identifier.textContent).toBe('alice.test') - expect(icons).toHaveLength(2) - expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ - 'This handle is associated with alice@example.test.', - 'This handle is associated with alice@example.test.', + expectConsentTooltipTexts(document, [ + ALICE_ASSOCIATED_TOOLTIP, + ALICE_ASSOCIATED_TOOLTIP, ]) }) @@ -957,9 +1039,7 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { }) expect(identifier.textContent).toBe('alice.test') - expect(container.textContent).toContain( - 'This handle is associated with alice@example.test.', - ) + expect(container.textContent).toContain(ALICE_ASSOCIATED_TOOLTIP) } }) @@ -978,9 +1058,7 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { }) expect(identifier.textContent).toBe('alice@example.test') - expect(container.textContent).toContain( - 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', - ) + expect(container.textContent).toContain(ALICE_PUBLIC_HANDLE_TOOLTIP) expect(container.textContent).not.toContain('@@alice.test') }) @@ -1001,90 +1079,40 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { ) expect(identifier.textContent).toBe('alice.test') - expect(container.textContent).toContain( - 'This handle is associated with alice@example.test.', - ) + expect(container.textContent).toContain(ALICE_ASSOCIATED_TOOLTIP) }) it('enriches preview chooser consent state in picker-with-random mode', () => { const document = new FakeDocument() - appendHandleModeMeta(document, 'picker-with-random') - const sidebar = createConsentIdentity( - document, - 'Grant access to your ', - 'alice.test', - ' account', - ) - const mainCard = createConsentIdentity( - document, - 'wants to access your ', - 'alice.test', - ' account', - ) - - runChooserEnrichmentScript( + const { sidebar, mainCard } = createPreviewChooserConsentIdentities( document, - { __sessions: selectedAliceSession() }, - { - pathname: '/preview/chooser', - search: '?epds_handle_mode=picker-with-random', - }, + 'picker-with-random', ) - const icons = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-info-icon')) - const tooltips = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-tooltip')) + runPreviewChooserConsentEnrichment(document, 'picker-with-random') expect(sidebar.identifier.textContent).toBe('alice.test') expect(mainCard.identifier.textContent).toBe('alice.test') - expect(icons).toHaveLength(2) - expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ - 'This handle is associated with alice@example.test.', - 'This handle is associated with alice@example.test.', + expectConsentTooltipTexts(document, [ + ALICE_ASSOCIATED_TOOLTIP, + ALICE_ASSOCIATED_TOOLTIP, ]) }) it('uses email as the visible preview chooser consent identity in random mode', () => { const document = new FakeDocument() - appendHandleModeMeta(document, 'random') - const sidebar = createConsentIdentity( + const { sidebar, mainCard } = createPreviewChooserConsentIdentities( document, - 'Grant access to your ', - 'alice.test', - ' account', - ) - const mainCard = createConsentIdentity( - document, - 'wants to access your ', - 'alice.test', - ' account', - ) - - runChooserEnrichmentScript( - document, - { __sessions: selectedAliceSession() }, - { - pathname: '/preview/chooser', - search: '?epds_handle_mode=random', - }, + 'random', ) - const icons = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-info-icon')) - const tooltips = document.root - .descendants() - .filter((el) => el.classList.contains('epds-identity-tooltip')) + runPreviewChooserConsentEnrichment(document, 'random') expect(sidebar.identifier.textContent).toBe('alice@example.test') expect(mainCard.identifier.textContent).toBe('alice@example.test') - expect(icons).toHaveLength(2) - expect(tooltips.map((tooltip) => tooltip.textContent)).toEqual([ - 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', - 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', + expectConsentTooltipTexts(document, [ + ALICE_PUBLIC_HANDLE_TOOLTIP, + ALICE_PUBLIC_HANDLE_TOOLTIP, ]) }) @@ -1105,9 +1133,7 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { ) expect(identifier.textContent).toBe('alice@example.test') - expect(container.textContent).toContain( - 'Public AT Protocol handle: @alice.test. Handles are public account names used by AT Protocol apps.', - ) + expect(container.textContent).toContain(ALICE_PUBLIC_HANDLE_TOOLTIP) }) it('leaves generic and legal consent paragraphs untouched', () => { @@ -1128,47 +1154,28 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) - expect(document.root.textContent).not.toContain('This handle is associated') + expect(document.root.textContent).not.toContain(ASSOCIATED_TOOLTIP_PREFIX) expect(unrelated.identifier.textContent).toBe('bob.test') }) it('leaves arbitrary bold selected-account legal copy untouched', () => { - const document = new FakeDocument() - appendHandleModeMeta(document, 'picker') - const { container, identifier } = createConsentIdentity( - document, - 'By clicking Authorize, ', - 'alice.test', - ' confirms access.', - ) - - runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) - - expect(identifier.textContent).toBe('alice.test') - expect(container.textContent).toBe( - 'By clicking Authorize, alice.test confirms access.', - ) - expect(container.textContent).not.toContain('This handle is associated') + expectArbitraryConsentProseUntouched({ + prefix: 'By clicking Authorize, ', + identifierText: 'alice.test', + suffix: ' confirms access.', + expectedText: 'By clicking Authorize, alice.test confirms access.', + }) }) it('leaves arbitrary strong selected-account technical prose untouched', () => { - const document = new FakeDocument() - appendHandleModeMeta(document, 'picker') - const { container, identifier } = createConsentIdentity( - document, - 'Technical details for ', - 'alice.test', - ' may include OAuth scopes.', - 'strong', - ) - - runChooserEnrichmentScript(document, { __sessions: selectedAliceSession() }) - - expect(identifier.textContent).toBe('alice.test') - expect(container.textContent).toBe( - 'Technical details for alice.test may include OAuth scopes.', - ) - expect(container.textContent).not.toContain('This handle is associated') + expectArbitraryConsentProseUntouched({ + prefix: 'Technical details for ', + identifierText: 'alice.test', + suffix: ' may include OAuth scopes.', + tagName: 'strong', + expectedText: + 'Technical details for alice.test may include OAuth scopes.', + }) }) it('opens the tooltip on hover/focus and toggles it on click/tap', () => { @@ -1257,7 +1264,7 @@ describe('buildChooserEnrichmentScript consent identity enrichment', () => { { pathname: '/account', search: '' }, ) - expect(document.root.textContent).not.toContain('This handle is associated') + expect(document.root.textContent).not.toContain(ASSOCIATED_TOOLTIP_PREFIX) }) }) @@ -1689,6 +1696,92 @@ describe('createChooserEnrichmentMiddleware handle-mode meta (HYPER-268 Layer 4) expect(written).toContain('') }) + it('resolves client metadata handle mode from request_uri when client_id is absent', async () => { + const resolveClientIdFromRequestUri = vi + .fn() + .mockResolvedValue('https://demo.example/client') + const resolveClientMetadata = vi + .fn() + .mockResolvedValue({ epds_handle_mode: 'random' as const }) + + const written = await captureWrittenHtml( + { + resolveClientMetadata, + resolveClientIdFromRequestUri, + }, + { request_uri: 'urn:ietf:params:oauth:request_uri:req-123' }, + ) + + expect(resolveClientIdFromRequestUri).toHaveBeenCalledWith( + 'urn:ietf:params:oauth:request_uri:req-123', + ) + expect(resolveClientMetadata).toHaveBeenCalledWith( + 'https://demo.example/client', + ) + expect(written).toContain('') + }) + + it('keeps explicit query handle mode ahead of request_uri metadata', async () => { + const resolveClientIdFromRequestUri = vi + .fn() + .mockResolvedValue('https://demo.example/client') + const resolveClientMetadata = vi + .fn() + .mockResolvedValue({ epds_handle_mode: 'random' as const }) + + const written = await captureWrittenHtml( + { + resolveClientMetadata, + resolveClientIdFromRequestUri, + }, + { + epds_handle_mode: 'picker', + request_uri: 'urn:ietf:params:oauth:request_uri:req-123', + }, + ) + + expect(resolveClientIdFromRequestUri).not.toHaveBeenCalled() + expect(resolveClientMetadata).not.toHaveBeenCalled() + expect(written).toContain('') + }) + + it('degrades silently when request_uri client-id resolution rejects', async () => { + const written = await captureWrittenHtml( + { + resolveClientMetadata: vi.fn(), + resolveClientIdFromRequestUri: () => + Promise.reject(new Error('request expired')), + }, + { request_uri: 'urn:ietf:params:oauth:request_uri:req-123' }, + ) + + expect(written).toContain( + '', + ) + }) + + it('degrades silently when request_uri metadata lookup rejects', async () => { + const resolveClientMetadata = vi + .fn() + .mockRejectedValue(new Error('metadata unavailable')) + + const written = await captureWrittenHtml( + { + resolveClientMetadata, + resolveClientIdFromRequestUri: () => + Promise.resolve('https://demo.example/client'), + }, + { request_uri: 'urn:ietf:params:oauth:request_uri:req-123' }, + ) + + expect(resolveClientMetadata).toHaveBeenCalledWith( + 'https://demo.example/client', + ) + expect(written).toContain( + '', + ) + }) + it('ignores invalid handle modes from metadata (fall through to fallback)', async () => { const written = await captureWrittenHtml( { @@ -1705,9 +1798,29 @@ describe('createChooserEnrichmentMiddleware handle-mode meta (HYPER-268 Layer 4) ) }) - it('degrades silently when the metadata resolver rejects', async () => { + it('ignores invalid request_uri metadata handle modes through the shared resolver', async () => { + const written = await captureWrittenHtml( + { + resolveClientMetadata: () => + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- deliberate bad value + Promise.resolve({ epds_handle_mode: 'garbage' as any }), + resolveClientIdFromRequestUri: () => + Promise.resolve('https://demo.example/client'), + }, + { request_uri: 'urn:ietf:params:oauth:request_uri:req-123' }, + ) + + expect(written).toContain( + '', + ) + }) + + it('logs and falls back when the metadata resolver rejects', async () => { + const debug = vi.fn() + const written = await captureWrittenHtml( { + logger: { debug }, resolveClientMetadata: () => Promise.reject(new Error('network error')), }, { client_id: 'https://demo.example/client' }, @@ -1716,6 +1829,14 @@ describe('createChooserEnrichmentMiddleware handle-mode meta (HYPER-268 Layer 4) expect(written).toContain( '', ) + expect(debug).toHaveBeenCalledWith( + expect.objectContaining({ + err: expect.any(Error), + queryMode: undefined, + requestUri: undefined, + }), + 'chooser-enrichment: failed to resolve handle mode from OAuth request context', + ) }) }) diff --git a/packages/pds-core/src/__tests__/epds-callback-authorize.test.ts b/packages/pds-core/src/__tests__/epds-callback-authorize.test.ts new file mode 100644 index 00000000..6bdadf36 --- /dev/null +++ b/packages/pds-core/src/__tests__/epds-callback-authorize.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { buildEpdsCallbackAuthorizeUrl } from '../lib/epds-callback-authorize.js' + +describe('buildEpdsCallbackAuthorizeUrl', () => { + it('preserves a valid display-only epds_handle_mode on the final authorize redirect', () => { + const url = buildEpdsCallbackAuthorizeUrl({ + pdsUrl: 'https://pds.example', + requestUri: 'urn:ietf:params:oauth:request_uri:req-123', + clientId: 'https://app.example/client.json', + handleMode: 'random', + }) + + expect(url.pathname).toBe('/oauth/authorize') + expect(url.searchParams.get('request_uri')).toBe( + 'urn:ietf:params:oauth:request_uri:req-123', + ) + expect(url.searchParams.get('client_id')).toBe( + 'https://app.example/client.json', + ) + expect(url.searchParams.get('epds_handle_mode')).toBe('random') + }) + + it('drops invalid epds_handle_mode values from the final authorize redirect', () => { + const url = buildEpdsCallbackAuthorizeUrl({ + pdsUrl: 'https://pds.example', + requestUri: 'urn:ietf:params:oauth:request_uri:req-123', + clientId: 'https://app.example/client.json', + handleMode: 'garbage', + }) + + expect(url.searchParams.has('epds_handle_mode')).toBe(false) + }) +}) diff --git a/packages/pds-core/src/chooser-enrichment.ts b/packages/pds-core/src/chooser-enrichment.ts index c6ff193e..8aa22fd2 100644 --- a/packages/pds-core/src/chooser-enrichment.ts +++ b/packages/pds-core/src/chooser-enrichment.ts @@ -25,6 +25,10 @@ import type { ResolveClientMetadataOptions, } from '@certified-app/shared' import { resolveHandleMode, VALID_HANDLE_MODES } from '@certified-app/shared' +import { + resolveOAuthClientIdFromQuery, + type ResolveClientIdFromRequestUri, +} from './lib/oauth-request-context.js' /** * Build the post-hydration enrichment script injected into `/account*` @@ -43,7 +47,7 @@ import { resolveHandleMode, VALID_HANDLE_MODES } from '@certified-app/shared' * this runs in a plain `