fix: reuse the popup for the interactive retry — no more blocked second window / 'Open new window' dialog - #13
fix: reuse the popup for the interactive retry — no more blocked second window / 'Open new window' dialog#13jeswr wants to merge 1 commit into
Conversation
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.
|
|
@langsamu thanks — answering your three points, with behavioural evidence rather than guessing where it's a runtime question. 1. Taking the second commit to 2. The UI element duplicating the token provider's error-message checks. This is a fair observation and I don't want to paper over it — it is genuine duplication. Both 3. "Is it correct to start with Short answer: yes, starting with
So "some require interaction, others don't" is the same server behaving differently depending on whether you already had a session — which is exactly the win Evidence (Playwright, on branch I sent an unauthenticated
All three decline the silent attempt, and the only difference is which of the three error strings they use (CSS/ESS say Two accuracy caveats I want to be straight about:
Also recorded the discovery metadata the provider keys its behaviour off (refresh_token grant, offline_access scope, PKCE, DPoP algs) for all three in |
…Solid server matrix Extends the PR #13 evidence (was CSS/ESS/broker) to every DISTINCT known Solid server codebase the server-agnostic reactive-fetch app could target: - CSS (solidcommunity.net, solidweb.me, + a local full-control instance) - Pivot (teamid.live) - ESS (login.inrupt.com) - NSS (solidweb.org, datapod.igrant.io) - Trinpod / TwinPod-Server (trinpod.us, trinpod.eu) - Manas, PHP Solid Server (pdsinterop), use.id, inrupt.net — recorded as unverified/unreachable (no public OIDC endpoint), skipped with a reason. oidc.ts gains a codebase-tagged MATRIX and a silentAttempt that CLASSIFIES the response (interaction-error / html-login / code / other) instead of only reading the redirect error. New table-driven specs (prompt-none-matrix, discovery-matrix) iterate the matrix, tolerate unreachable hosts, and assert observed behaviour. KEY FINDING: NSS and Trinpod IGNORE prompt=none and serve an interactive HTML login page (HTTP 200, no OIDC error redirect) rather than returning one of {login_required, interaction_required, consent_required}. Recorded as a divergence, not a failure — flagged for the library's retry classifier. Original specs (prompt-none.spec, discovery.spec, warm-session.spec) unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
prompt=none evidence — expanded across the full Solid server matrixFollowing up on the earlier 3-server evidence: I broadened the probe to every distinct known Solid server codebase the reactive-fetch app could be pointed at (it's server-agnostic — the user types their issuer at runtime, so the target set is "all known Solid IdPs"). All probes are credential-free (dynamic public-client registration + an unauthenticated Tests pushed to
Codebases covered (live): CSS, Pivot, ESS, NSS, Trinpod — plus a local full-control CSS as a self-hosted control. So: is it correct to start with
|
The flow always tries prompt=none first, so a fresh login is a silent attempt, a login_required, then an interactive retry. Closing the popup on every callback meant that retry had to open a new window, outside the original click's already spent user activation, so popup blockers stopped it and the user was stranded in the "open new window" dialog. Keeping the window open for the interaction errors lets the retry navigate it by name, which needs no activation. A code or a terminal error such as access_denied still closes it, and the cancel and abort paths are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1ce81c1 to
5e06ac1
Compare
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" dialog on every first login.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. The retry'sopen(uri, sameWindowName)then merely navigates the existing named window, which browsers allow without user activation.The user sees one popup that goes from the silent bounce straight to the IdP login page.
An authorization code or a terminal error such as
access_deniedstill closes the window as before; the cancel and abort paths are unchanged.Note on scope
This branch originally also carried the fix that filters callback messages by
message.source. That has since landed onmainindependently, so it has been dropped from this branch — the rebase leaves a single commit, +18 lines.Verification
tsc: clean.prompt=nonebounce navigates in place to the Keycloak login, and the flow completes with no interstitial dialog.Future work, out of scope: running the
prompt=noneattempt in a hidden iframe would remove the popup flash for purely silent re-auth, but that needs acallback.htmlcontract change (parentvsopener).🤖 Generated with Claude Code