fix(oauth): stop treating a foreign window.opener as a bind flow

[neimaravila]

📝 变更描述 / Description

/oauth/:provider decides between an account bind and a plain login with a single signal:

const mode: 'login' | 'bind' =
  typeof window !== 'undefined' && window.opener ? 'bind' : 'login'

window.opener cannot carry that meaning. Any tab opened from an external link (target="_blank", Slack, Teams, mail clients, another site) has a live opener, and that opener survives the cross-origin round trip to the identity provider.

So an ordinary login callback in such a tab is misread as a bind: it posts the bind handshake to a window that speaks no such protocol, renders the "binding your account" screen, and hangs until the 30s deadline in startOAuthBindResponseDeadline fires with "OAuth binding timed out". The backend is never called at all — GET /api/oauth/:provider is never issued.

This is not browser-specific or user-specific: it hits every user who reaches the site through a link instead of typing the URL.

Root cause reproduced against a live Keycloak realm:

step	observed
tab opened via window.open (= target="_blank")	window.opener truthy
navigate cross-origin to Keycloak, then back to /oauth/oidc?code=…	window.opener still truthy, opener.closed === false
resulting mode	bind — wrong, this was a login

The change

A bind now needs positive proof instead of an inference. The popup opened for a bind is same-origin (about:blank) before it is sent to the provider, so it is stamped in its own sessionStorage. The stamp survives the provider round trip and is scoped to that popup alone, so a login tab can never carry it.

Resolution rules (resolveOAuthCallbackMode): a bind requires both a live opener and our stamp for that exact provider. Everything else resolves to login, deliberately — a login callback recovers on its own, whereas a wrongly assumed bind can only time out.

The decision is extracted into web/src/features/auth/lib/oauth-callback-mode.ts so it is unit-testable; the storage/opener dependencies are structural, matching the existing style in oauth-bind-window.ts.

Affects every provider sharing this callback: OIDC, GitHub, Discord, LinuxDO and custom providers.

🚀 变更类型 / Type of change

• 🐛 Bug 修复 (Bug fix)

🔗 关联任务 / Related Issue

• No existing issue found; reproduction steps are in Proof of Work below.

✅ 提交前检查项 / Checklist

• 人工确认: Description written and verified by me; AI assistance disclosed below.
• 非重复提交: Searched open PRs/issues; no existing fix for this callback-mode detection.
• Bug fix 说明: Reproduced deterministically; this is a defect, not a design trade-off.
• 变更理解: Root cause traced end to end, from opener semantics to the 30s handshake deadline.
• 范围聚焦: 4 files, no unrelated changes.
• 本地验证: See Proof of Work.
• 安全合规: No credentials; the stamp holds only a provider slug.

📸 运行证明 / Proof of Work

Reproduction of the root cause (real Keycloak round trip, browser console):

tab opened via window.open      -> openerTruthy: true
after Keycloak round trip       -> openerTruthy: true, openerClosed: false
mode computed by current code   -> "bind"     (should be "login")

Stamp behaves as required (same browser, same round trip):

marker survived round trip      -> "oidc"
marker in the login tab         -> null        (isolated to the popup)

End-to-end against a build of this branch — the previously hanging scenario (tab with a foreign opener → OIDC callback) now takes the login path immediately:

[GET] /api/oauth/oidc?code=…&state=… => [403]

403 is expected here (synthetic state); the point is the request is issued, where before the fix no request was made and the UI sat on the binding screen for 30s.

Checks

bun test    -> 86 pass, 0 fail (16 files), including 6 new cases
tsgo -b     -> clean

New tests cover: bind popup detected as bind; login in a tab with a foreign opener stays login (the regression); marker for another provider ignored; marker without opener; closed opener; storage unavailable.

---

🤖 AI assistance disclosure

I am not one of this repository's core contributors. This fix was AI-assisted: the root cause was found by instrumenting the real callback in a browser, and I reviewed the diagnosis, the change and the tests before submitting.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved OAuth callback handling by distinguishing login and account-binding flows using the provider and authorization state.
  • Added safeguards for unavailable, blocked, or unwritable browser session storage.
  • Prevented account-binding popups from proceeding when required state tracking fails.
  • Added explicit handling for Telegram account-binding callbacks.

• Tests

  • Added coverage for valid, stale, mismatched, missing, and inaccessible OAuth session markers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9a617cd1-f9f3-4b1b-9022-b83744505125

📥 Commits

Reviewing files that changed from the base of the PR and between 94fd100 and ebcd5a0.

📒 Files selected for processing (4)

• web/src/features/auth/lib/__tests__/oauth-callback-mode.test.ts
• web/src/features/auth/lib/oauth-callback-mode.ts
• web/src/features/profile/components/tabs/account-bindings-tab.tsx
• web/src/routes/oauth/$provider.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• web/src/routes/oauth/$provider.tsx

---

Walkthrough

OAuth bind popups now record provider-and-state markers in sessionStorage. The callback route uses those markers, opener validity, and callback state to select bind or login mode. Storage failures fall back safely to login, and tests cover matching and invalid callback scenarios.

Changes

OAuth callback mode

Layer / File(s)	Summary
Callback mode contract and resolver web/src/features/auth/lib/oauth-callback-mode.ts, web/src/features/auth/lib/__tests__/oauth-callback-mode.test.ts	The storage contract now supports nullable and failure-safe access. Bind markers include the provider and OAuth state. Callback resolution requires matching markers and a live opener. Tests cover valid, stale, mismatched, unavailable, and unwritable storage.
Binding popup marker wiring web/src/features/profile/components/tabs/account-bindings-tab.tsx	The binding popup records its provider and state before navigation. Initialization stops and closes the popup when storage marking fails.
OAuth callback route integration web/src/routes/oauth/$provider.tsx	The route extracts a shared callback state, handles Telegram bind callbacks explicitly, and uses the resolver for other browser callbacks. The effect dependencies use the shared state.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• QuantumNous/new-api#6543: Extends the same OAuth bind-mode detection with opener validation and provider/state markers.

Poem

A rabbit stamps the popup trail,
With provider and state in a session-scale.
Matching markers make binds appear,
Mismatches turn to login clear.
Safe storage keeps the flow in sight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main OAuth callback fix: a foreign window.opener no longer triggers bind-flow detection.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

web/src/features/auth/lib/oauth-callback-mode.test.ts (2)

27-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add an explicit return type to fakeStorage.

This new helper annotates its parameter but not its return value. Use OAuthModeStorage & { snapshot: () => Record<string, string> } (with a type-only import) to satisfy the repository’s TypeScript typing rule.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/features/auth/lib/oauth-callback-mode.test.ts` around lines 27 - 35,
Update fakeStorage with an explicit return type of OAuthModeStorage & {
snapshot: () => Record<string, string> }, adding a type-only import for
OAuthModeStorage from the existing auth storage module.

Source: Coding guidelines

---

96-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the storage-exception fallback.

The current test only covers storage: null; add a fake whose getItem throws to verify the resolver returns login when storage access fails.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/features/auth/lib/oauth-callback-mode.test.ts` around lines 96 - 100,
Extend the `resolveOAuthCallbackMode` test for missing storage with a fake
storage object whose `getItem` method throws, and assert the resolver returns
`login` rather than propagating the exception. Keep the existing `storage: null`
case intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/src/routes/oauth/`$provider.tsx:
- Around line 72-78: Guard the sessionStorage property reads before invoking the
OAuth helpers: in web/src/routes/oauth/$provider.tsx lines 72-78, update the
resolveOAuthCallbackMode call to safely handle a thrown window.sessionStorage
access and use the intended login-safe fallback; in
web/src/features/profile/components/tabs/account-bindings-tab.tsx lines 166-169,
apply the same protection to the popup.sessionStorage read passed to
markOAuthBindPopup. Ensure blocked storage does not escape from either property
access.

---

Nitpick comments:
In `@web/src/features/auth/lib/oauth-callback-mode.test.ts`:
- Around line 27-35: Update fakeStorage with an explicit return type of
OAuthModeStorage & { snapshot: () => Record<string, string> }, adding a
type-only import for OAuthModeStorage from the existing auth storage module.
- Around line 96-100: Extend the `resolveOAuthCallbackMode` test for missing
storage with a fake storage object whose `getItem` method throws, and assert the
resolver returns `login` rather than propagating the exception. Keep the
existing `storage: null` case intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fb93aa4d-f517-4e19-aa1f-f41bc6fedfdb

📥 Commits

Reviewing files that changed from the base of the PR and between 1721144 and 94fd100.

📒 Files selected for processing (4)

• web/src/features/auth/lib/oauth-callback-mode.test.ts
• web/src/features/auth/lib/oauth-callback-mode.ts
• web/src/features/profile/components/tabs/account-bindings-tab.tsx
• web/src/routes/oauth/$provider.tsx

[Calcium-Ion]

We’re about to squash and merge this PR. Which identity would you prefer for the squashed commit: Neimar Avila <neimar.avila@vsgroup.com.br> or your GitHub account, [neimaravila](https://github.com/neimaravila)?