From f1d7c1a74d36055aecc0ce9db99df9d12fc9dcde Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:01:44 +0100 Subject: [PATCH 1/2] fix: reuse the popup for the interactive retry instead of closing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DPoP provider always tries `prompt=none` first, so a fresh login is: silent attempt → `login_required` → interactive retry. The popup element closed the window on EVERY callback message, so the retry had to open a NEW window — outside the original click's (already consumed) user activation. Popup blockers stop that, stranding the user in the "User interaction needed … Open new window" dialog on every first login (and on slower connections the first open can be blocked too). Now the popup is kept open when the callback carries one of the OIDC "user must interact" errors (login_required / interaction_required / consent_required) — exactly the errors token providers retry right away. The retry's `open()` then merely NAVIGATES the existing named window, which browsers always allow. Users see one popup that goes from the silent bounce straight to the login page, with no interstitial dialog. A code response or a terminal error (e.g. access_denied when the user denies consent) still closes the window as before, as do cancel/abort. Verified live with a Playwright-driven login (Chrome popup blocker enabled) against a deployed app + Solid-OIDC broker. Co-Authored-By: Claude Fable 5 --- src/AuthorizationCodeFlow.ts | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/AuthorizationCodeFlow.ts b/src/AuthorizationCodeFlow.ts index ab6854a..f35cfb5 100644 --- a/src/AuthorizationCodeFlow.ts +++ b/src/AuthorizationCodeFlow.ts @@ -189,7 +189,18 @@ export class AuthorizationCodeFlow extends HTMLElement { const onMessage = (message: MessageEvent) => { signal.removeEventListener("abort", onAbort) this.#switchModal.close() - this.#authorizationWindow?.close() + + // When the server answered a silent (`prompt=none`) attempt with a "user + // interaction needed" error, the caller is about to retry interactively. + // Keep the popup open: the retry's `open()` then NAVIGATES this named + // window, which browsers allow without user activation. Closing it here + // would make the retry create a new window — popup blockers stop that + // (the original click's activation is already consumed), stranding the + // user in the "open new window" dialog. + if (!needsInteraction(message.data)) { + this.#authorizationWindow?.close() + } + respondWithCode(message.data) } @@ -247,3 +258,23 @@ export class AuthorizationCodeFlow extends HTMLElement { this.#cancelCodeRequest?.call(undefined, new CodeRequestCancelledError(this.#authorizationUri!)) } } + +/** + * Whether the authorization response is one of the OIDC "the user must interact" + * errors — exactly the errors token providers retry interactively right away. + * Anything else (a code, or a terminal error such as `access_denied`) ends the flow. + */ +function needsInteraction(authorizationResponse: unknown): boolean { + if (typeof authorizationResponse !== "string") { + return false + } + + let error + try { + error = new URL(authorizationResponse).searchParams.get("error") + } catch { + return false + } + + return error === "login_required" || error === "interaction_required" || error === "consent_required" +} From 1ce81c10e70feb30ea7e0c03e545854e66558ed0 Mon Sep 17 00:00:00 2001 From: Jesse Wright <63333554+jeswr@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:41:21 +0100 Subject: [PATCH 2/2] fix: only let the authorization popup's messages end the flow Review follow-up: filter the window message listener by message.source === the popup, so unrelated (or hostile) postMessage traffic can neither resolve the flow early nor spoof an authorization response. The listener now removes itself on the matching message instead of using { once: true }. Co-Authored-By: Claude Fable 5 --- src/AuthorizationCodeFlow.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/AuthorizationCodeFlow.ts b/src/AuthorizationCodeFlow.ts index f35cfb5..06fcf15 100644 --- a/src/AuthorizationCodeFlow.ts +++ b/src/AuthorizationCodeFlow.ts @@ -187,6 +187,14 @@ export class AuthorizationCodeFlow extends HTMLElement { this.#cancelCodeRequest = cancelCodeRequest const onMessage = (message: MessageEvent) => { + // Only the authorization popup may end the flow. Anything else on this + // window (other components, extensions, a hostile frame) must neither + // resolve the flow early nor spoof an authorization response. + if (message.source !== this.#authorizationWindow) { + return + } + + this.ownerDocument.defaultView?.removeEventListener("message", onMessage) signal.removeEventListener("abort", onAbort) this.#switchModal.close() @@ -213,7 +221,9 @@ export class AuthorizationCodeFlow extends HTMLElement { } signal.addEventListener("abort", onAbort, onlyOnce) - this.ownerDocument.defaultView?.addEventListener("message", onMessage, onlyOnce) + // Not `once`: the listener filters by source, so it must survive unrelated + // messages. It removes itself when the popup answers (or on abort). + this.ownerDocument.defaultView?.addEventListener("message", onMessage) this.#openAuthorizationWindow()