fix(ENG-950): don't cancel OAuth attempt on cross-origin popup false-close#177
Merged
Merged
Conversation
A cross-origin OAuth popup under Cross-Origin-Opener-Policy reports `window.closed === true` once it navigates to the provider, even while the user is still authenticating. The popup-close watcher's COOP guard is first-tick-only, so popups that open same-origin and then redirect to the provider slip past it: the watcher reads the popup as closed ~1-2s in, polls once (still pending), and cancels the connection attempt — bouncing the user back to the form (the "second click" bug). Defer to the poll loop on any detected close instead of cancelling, as the first-tick COOP path already does. Genuine closes resolve via the poll timeout or the explicit Cancel button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adjusts the OAuth popup “close detection” logic in the integration picker to avoid incorrectly cancelling an in-progress OAuth attempt when the popup navigates cross-origin under Cross-Origin-Opener-Policy (COOP) and window.closed can falsely report true.
Changes:
- Stop cancelling the connection attempt when the popup reports closed while the attempt is still pending; instead, defer to the existing polling loop to resolve authenticated/error/cancelled/expired/timeout.
- Keep a “final poll” when the popup reports closed so a just-completed authentication is still detected immediately.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adefreitas
approved these changes
Jul 1, 2026
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.
Summary
Fixes the connect "second click" bug (ENG-950): under the openid-client (
feat_oidc_client) auth path, clicking Connect on an OAuth2 provider returned the user to the form on the first attempt; a second click was needed to complete.Root cause
src/modules/integration-picker/hooks/useIntegrationPicker.ts→startPopupWatcherpollsconnectWindow.current.closedevery second. Its Cross-Origin-Opener-Policy (COOP) guard is first-tick only: if the popup reads closed at +1s it assumes COOP isolation, disables close-detection, and lets the poll loop drive.But the OAuth popup opens same-origin (
${baseUrl}/connect/oauth2/...) and only then 302-redirects to the provider. So at +1s it still reads open, the COOP guard doesn't trip, and a tick later — once it navigates cross-origin under COOP —window.closedflips totrueeven though the popup is open and the user is still on the consent screen. The watcher treats that as a user-close, polls once (stillpending), and cancels the connection attempt → returns to the form.Evidence (staging)
A real failing Contentful flow shows the attempt cancelled ~2s after the popup opened, ~4.5s before the provider even called back:
Fix
In the close-detected branch, don't cancel a still-pending attempt — defer to the poll loop (
authenticated/error/cancelled/expired/ timeout), exactly as the first-tick COOP path already does. The success-catching final poll is kept, so an auth that completes right as the popup closes is still picked up immediately.Tradeoff
A genuinely-abandoned popup now shows "loading" until the poll resolves or times out (the same behaviour COOP-isolated popups already have today), rather than bouncing back immediately. The explicit Cancel button still aborts instantly.
Verification
npm run lint— cleannpm run build— green;'use client'banner intact ondist/index.esm.jsanddist/index.jsnpm testis a stub); a vitest test for the watcher would be a good follow-up.Note
This fixes the frontend "second click" only. Contentful & QuickBooks Online additionally fail at the backend token exchange (
invalid_client) under the OC path — tracked separately in ENG-1045 — so those two won't fully link from this change alone.🤖 Generated with Claude Code