Skip to content

fix(ENG-950): don't cancel OAuth attempt on cross-origin popup false-close#177

Merged
CameronCarlin merged 1 commit into
mainfrom
fix/eng-950-hub-popup-close-false-cancel
Jul 1, 2026
Merged

fix(ENG-950): don't cancel OAuth attempt on cross-origin popup false-close#177
CameronCarlin merged 1 commit into
mainfrom
fix/eng-950-hub-popup-close-false-cancel

Conversation

@CameronCarlin

Copy link
Copy Markdown
Contributor

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.tsstartPopupWatcher polls connectWindow.current.closed every 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.closed flips to true even though the popup is open and the user is still on the consent screen. The watcher treats that as a user-close, polls once (still pending), 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:

13:58:44.7  302 → Contentful (popup opens)
13:58:46.6  DELETE /hub/connection_attempts/9c663562 → 204   ← cancelled mid-auth
13:58:51.1  GET /connect/oauth2/contentful/callback?code=…   ← provider returns (too late)

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 — clean
  • npm run build — green; 'use client' banner intact on dist/index.esm.js and dist/index.js
  • No test infrastructure in this repo (npm test is 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

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>
Copilot AI review requested due to automatic review settings June 30, 2026 14:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@CameronCarlin CameronCarlin merged commit 809e9d2 into main Jul 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants