fix: reuse the popup for the interactive retry — no more blocked second window / 'Open new window' dialog#13
Open
jeswr wants to merge 2 commits into
Open
fix: reuse the popup for the interactive retry — no more blocked second window / 'Open new window' dialog#13jeswr wants to merge 2 commits into
jeswr wants to merge 2 commits into
Conversation
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the AuthorizationCodeFlow popup lifecycle so that when a silent prompt=none attempt returns an OIDC “user must interact” error (login_required, interaction_required, consent_required), the popup is kept open. This enables the subsequent interactive retry to reuse (navigate) the same named popup window, avoiding popup-blocker failures that occur when trying to open a second window without user activation.
Changes:
- Keep the authorization popup open on OIDC “interaction needed” error callbacks to allow interactive retry to navigate the existing named window.
- Add a helper (
needsInteraction) to detect the relevant OIDC error responses from the callback URL.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
189
to
+193
| 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 |
Contributor
Author
There was a problem hiding this comment.
Fixed in 1ce81c1: the listener now ignores any message whose source is not the authorization popup, and removes itself on the matching message instead of using { once: true }.
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DPoPTokenProvideralways triesprompt=nonefirst, so a fresh login is: silent attempt →login_required→ interactive retry.AuthorizationCodeFlowclosed the popup on every callback message, so the retry had toopen()a new window — outside the original click's (already consumed) user activation. Popup blockers stop that, stranding the user in the "User interaction needed to launch authorization code flow in new window → Open new window" dialog on every first login. This is the "unnecessary accepting of prompts to redirect" users report.Change
Keep the popup open when the callback message carries one of the OIDC "user must interact" errors (
login_required/interaction_required/consent_required) — exactly the errors token providers retry interactively right away. The retry'sopen(uri, sameWindowName)then merely navigates the existing named window, which browsers always allow without user activation.Result: the user sees one popup that goes from the silent bounce straight to the IdP login page — no blocked window, no interstitial dialog, no extra click.
A
coderesponse or a terminal error (e.g.access_deniedwhen the user denies consent) still closes the window exactly as before; cancel/abort paths are unchanged.Verification
npm run build(tsc): clean.--disable-popup-blockingremoved) against a deployed pod manager + Solid-OIDC broker (https://app.solid-test.jeswr.org). Before: silent popup closes → second window needed. After: one popup,prompt=nonebounce navigates in place to the Keycloak login, flow completes with zero interstitial dialogs.Future work (out of scope here): running the
prompt=noneattempt in a hidden iframe would remove the popup flash for purely silent re-auth; that needs acallback.htmlcontract change (parentvsopener), so it is left for a separate discussion.🤖 Generated with Claude Code