Repository navigation
fix(oauth): stop treating a foreign window.opener as a bind flow #6425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Calcium-Ion
merged 2 commits into
QuantumNous:main
from
neimaravila:fix/oauth-callback-mode
Jul 31, 2026
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| /* | ||
| Copyright (C) 2023-2026 QuantumNous | ||
|
|
||
| This program is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Affero General Public License as | ||
| published by the Free Software Foundation, either version 3 of the | ||
| License, or (at your option) any later version. | ||
|
|
||
| This program is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Affero General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Affero General Public License | ||
| along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| For commercial licensing, please contact support@quantumnous.com | ||
| */ | ||
| import assert from 'node:assert/strict' | ||
| import { describe, test } from 'node:test' | ||
|
|
||
| import { | ||
| markOAuthBindPopup, | ||
| resolveOAuthCallbackMode, | ||
| } from './oauth-callback-mode' | ||
|
|
||
| function fakeStorage(initial: Record<string, string> = {}) { | ||
| const data = new Map(Object.entries(initial)) | ||
| return { | ||
| getItem: (key: string) => data.get(key) ?? null, | ||
| setItem: (key: string, value: string) => void data.set(key, value), | ||
| removeItem: (key: string) => void data.delete(key), | ||
| snapshot: () => Object.fromEntries(data), | ||
| } | ||
| } | ||
|
|
||
| const openOpener = { closed: false } | ||
|
|
||
| describe('resolveOAuthCallbackMode', () => { | ||
| test('bind popup we opened is treated as a bind flow', () => { | ||
| const storage = fakeStorage() | ||
| markOAuthBindPopup(storage, 'oidc') | ||
|
|
||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { opener: openOpener, storage }), | ||
| 'bind' | ||
| ) | ||
| }) | ||
|
|
||
| // Regression: a tab opened from an external link (Slack, e-mail, another | ||
| // site) keeps a live window.opener across the cross-origin round trip to the | ||
| // identity provider. Treating that opener as proof of a bind flow made every | ||
| // such login hang on the binding screen until the 30s handshake deadline. | ||
| test('login redirect in a tab with a foreign opener stays a login flow', () => { | ||
| const storage = fakeStorage() | ||
|
|
||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { opener: openOpener, storage }), | ||
| 'login' | ||
| ) | ||
| }) | ||
|
|
||
| test('bind marker for another provider does not hijack this callback', () => { | ||
| const storage = fakeStorage() | ||
| markOAuthBindPopup(storage, 'github') | ||
|
|
||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { opener: openOpener, storage }), | ||
| 'login' | ||
| ) | ||
| }) | ||
|
|
||
| test('bind marker without an opener falls back to login', () => { | ||
| const storage = fakeStorage() | ||
| markOAuthBindPopup(storage, 'oidc') | ||
|
|
||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { opener: null, storage }), | ||
| 'login' | ||
| ) | ||
| }) | ||
|
|
||
| test('closed opener falls back to login', () => { | ||
| const storage = fakeStorage() | ||
| markOAuthBindPopup(storage, 'oidc') | ||
|
|
||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { | ||
| opener: { closed: true }, | ||
| storage, | ||
| }), | ||
| 'login' | ||
| ) | ||
| }) | ||
|
|
||
| test('missing storage degrades to login instead of throwing', () => { | ||
| assert.equal( | ||
| resolveOAuthCallbackMode('oidc', { opener: openOpener, storage: null }), | ||
| 'login' | ||
| ) | ||
| }) | ||
| }) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /* | ||
| Copyright (C) 2023-2026 QuantumNous | ||
|
|
||
| This program is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Affero General Public License as | ||
| published by the Free Software Foundation, either version 3 of the | ||
| License, or (at your option) any later version. | ||
|
|
||
| This program is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Affero General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Affero General Public License | ||
| along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| For commercial licensing, please contact support@quantumnous.com | ||
| */ | ||
|
|
||
| /** | ||
| * Tells apart the two OAuth callbacks that land on the same `/oauth/:provider` | ||
| * route: an account **bind**, which runs inside a popup we opened, and a plain | ||
| * **login** redirect, which runs in the user's own tab. | ||
| * | ||
| * `window.opener` alone cannot make that call. Any tab opened from an external | ||
| * link (`target="_blank"`, Slack, mail clients, another site) carries a live | ||
| * opener, and that opener survives the cross-origin round trip to the identity | ||
| * provider. Such a login callback used to be misread as a bind, so it posted a | ||
| * handshake to a window that speaks no such protocol and sat on the binding | ||
| * screen until the deadline elapsed. | ||
| * | ||
| * The popup we open for a bind is same-origin (`about:blank`) before it is sent | ||
| * to the provider, so we stamp its own sessionStorage. That stamp rides along | ||
| * through the provider round trip and is scoped to the popup alone, which makes | ||
| * it positive proof of a bind flow. | ||
| */ | ||
|
|
||
| const OAUTH_BIND_FLOW_KEY = 'oauth_bind_flow' | ||
|
|
||
| /** Minimal shape of `sessionStorage`, kept structural so tests can fake it. */ | ||
| export interface OAuthModeStorage { | ||
| getItem: (key: string) => string | null | ||
| setItem: (key: string, value: string) => void | ||
| removeItem: (key: string) => void | ||
| } | ||
|
|
||
| /** Minimal shape of `window.opener`. */ | ||
| export interface OAuthModeOpener { | ||
| closed: boolean | ||
| } | ||
|
|
||
| export interface OAuthCallbackModeContext { | ||
| opener: OAuthModeOpener | null | undefined | ||
| storage: OAuthModeStorage | null | undefined | ||
| } | ||
|
|
||
| export type OAuthCallbackMode = 'login' | 'bind' | ||
|
|
||
| /** | ||
| * Stamp a freshly opened, still same-origin popup as an OAuth bind flow. | ||
| * Call this before navigating the popup to the provider. | ||
| */ | ||
| export function markOAuthBindPopup( | ||
| storage: OAuthModeStorage | null | undefined, | ||
| provider: string | ||
| ): void { | ||
| try { | ||
| storage?.setItem(OAUTH_BIND_FLOW_KEY, provider) | ||
| } catch { | ||
| // A blocked or full sessionStorage only costs us the bind stamp; the | ||
| // callback then behaves like a login, which is the safe direction. | ||
| } | ||
| } | ||
|
|
||
| /** Clear the stamp once the bind flow is over. */ | ||
| export function clearOAuthBindPopupMark( | ||
| storage: OAuthModeStorage | null | undefined | ||
| ): void { | ||
| try { | ||
| storage?.removeItem(OAUTH_BIND_FLOW_KEY) | ||
| } catch { | ||
| // Nothing actionable: the stamp dies with the popup anyway. | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolve how a callback on `/oauth/:provider` should be handled. | ||
| * | ||
| * A bind requires both halves of the evidence: our own stamp for this exact | ||
| * provider, and a live opener to hand the result back to. Anything else is a | ||
| * login, which is also the safe default — a login callback recovers on its own, | ||
| * while a wrongly assumed bind can only time out. | ||
| */ | ||
| export function resolveOAuthCallbackMode( | ||
| provider: string, | ||
| { opener, storage }: OAuthCallbackModeContext | ||
| ): OAuthCallbackMode { | ||
| if (!opener || opener.closed) return 'login' | ||
|
|
||
| let marked: string | null = null | ||
| try { | ||
| marked = storage?.getItem(OAUTH_BIND_FLOW_KEY) ?? null | ||
| } catch { | ||
| return 'login' | ||
| } | ||
|
|
||
| return marked && marked === provider ? 'bind' : 'login' | ||
| } |
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.