mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-19 21:00:40 +08:00
fix(sdk): stop nested cross-domain auth from restarting the redirect chain on the hosted domain (#1581)
## What users see
Setting up a new project with hosted components, clicking **Sign in**
sometimes throws the browser into a redirect ping-pong between the app
and the hosted components site — anywhere from 5 to 9+ cross-domain
redirects — before the sign-in page finally renders. Reproduced live
against production:
Captured redirect chain from that recording (one line per navigation, ~1
per second):
```
localhost:3000/ ← click "Sign in"
HOSTED /handler/sign-in?...nested_refresh_token_id ← start session handoff
localhost:3000/?redirect_uri=...&state=S1 ← bounce: "prove the session"
HOSTED /handler/sign-in?...&code=... ← code delivered... then RESTART ↩
localhost:3000/?redirect_uri=...&state=S2 ← bounce again (fresh state!)
HOSTED /handler/sign-in?...&code=... ← code delivered... RESTART ↩
localhost:3000/?redirect_uri=...&state=S3 ← again
HOSTED /handler/sign-in?...&code=... ← again
localhost:3000/?redirect_uri=...&state=S4 ← again
HOSTED /handler/sign-in?...&code=... ← exchange finally wins the race
HOSTED /handler/sign-in (clean URL) ← sign-in form renders
```
The designed handshake is only 3 cross-domain redirects. Everything past
that is one bug restarting the chain over and over.
## The bug
When a page on the hosted domain loads with a one-time `code`, the
`StackClientApp` constructor schedules **two** async startup flows
back-to-back:
1. `callOAuthCallback` — which **synchronously strips `code` + `state`
from the URL** (`history.replaceState`) before starting its network
token exchange, and
2. `_maybeHandleNestedCrossDomainAuth` — which has a guard for exactly
this situation ("a real OAuth callback wins"), implemented as *"is
`code`+`state` in the URL?"*
Flow 1 runs first. By the time flow 2 reads `window.location`, the
params it's guarding on are already gone — so it concludes no OAuth
callback is happening, sees the (un-stripped) nested handoff marker, and
bounces back to the app domain to request a *new* code, cancelling the
in-flight exchange:
```mermaid
sequenceDiagram
participant A as Your app (a.com)
participant B as Hosted sign-in (b.com)
A->>B: 1. go to sign-in ("I have session X")
B->>A: 2. "prove it" (state, code_challenge)
A->>B: 3. one-time code for session X
Note over B: callOAuthCallback strips code+state from URL,<br/>starts token exchange (network)
Note over B: nested handler runs next, checks URL for code+state…<br/>already gone → guard defeated ❌
B->>A: 2'. "prove it" AGAIN (fresh state) — exchange cancelled
A->>B: 3'. another one-time code
Note over B: …loop repeats until the exchange happens to<br/>finish before the re-bounce navigation commits
```
Whether each cycle escapes is a coin flip between two competing
navigations (the exchange's success redirect vs. the handler's
re-bounce), which is why the loop count varies run to run and the issue
reproduces so inconsistently.
## The fix
Capture the URL once, at construction time — before anything can mutate
it — and let the nested handler consult that snapshot in addition to the
live URL:
- The constructor now captures `new URL(window.location.href)` when
scheduling the nested-auth resolution and passes it in.
- `_maybeHandleNestedCrossDomainAuth(urlAtConstructionTime?)` stands
down if **either** the live URL **or** the construction-time URL carries
`code` + `state`.
A stripped callback still counts as a callback, so the handler no longer
re-bounces while the exchange is in flight. Every other path is
unchanged: the handler still reads all of its working params from the
live URL (the strip never touches the nested params), hop-1/hop-2 pages
have no `code` in either snapshot, and ordinary social-login callbacks
never had this race (the component-driven flow strips long after the
handler has run).
Note this fix removes the *restarts*. The remaining 3-redirect baseline
for signed-out users is a separate design issue (the analytics-created
anonymous session triggering the handoff at all) and is intentionally
out of scope here.
## Tests
- New: `does not re-bounce nested cross-domain auth after the OAuth
callback consumed code+state from the URL` — pins both guards
(mutation-tested: reverting either fix line fails it).
- New: `passes the construction-time URL to the nested cross-domain auth
handler` — pins the eager capture; fails if the URL is read lazily at
handler run time.
- Full cross-domain suite passes (the `signOut` timeout in that file is
a pre-existing flake on `dev`, reproducible without this change).
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Fixes a race in nested cross-domain auth that caused repeated redirects
between the app and the hosted sign-in. We now snapshot the URL at
construction so OAuth callbacks are respected even after `code` and
`state` are stripped.
- **Bug Fixes**
- Capture `window.location` at construction and pass it to
`_maybeHandleNestedCrossDomainAuth`.
- Handler stands down if `code` and `state` exist in the live or
captured URL.
- Stops the redirect ping‑pong; the 3‑redirect baseline remains
unchanged.
- Keeps reading nested params from the live URL; no other paths changed.
- Adds tests to pin the race and the construction‑time URL behavior.
<sup>Written for commit f312baa54c.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/hexclave/hexclave/pull/1581?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved cross-domain OAuth authentication handling to prevent
unnecessary redirects after OAuth callback processing.
* **Tests**
* Added test coverage for nested cross-domain OAuth authentication
scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
BilalG1
GitHub
parent
a092be6dc3
commit
f4c13db079
@ -325,6 +325,108 @@ describe("StackClientApp cross-domain auth", () => {
|
||||
expect(refreshedRawRefreshTokens).toEqual(["new-refresh-token"]);
|
||||
});
|
||||
|
||||
it("does not re-bounce nested cross-domain auth after the OAuth callback consumed code+state from the URL", async () => {
|
||||
const projectId = "00000000-0000-4000-8000-000000000008";
|
||||
const previousWindow = globalThis.window;
|
||||
const previousDocument = globalThis.document;
|
||||
|
||||
const strippedUrl = new URL(`https://${projectId}.example-stack-hosted.test/handler/sign-in`);
|
||||
strippedUrl.searchParams.set("stack_nested_cross_domain_auth_refresh_token_id", "source-refresh-token-id");
|
||||
strippedUrl.searchParams.set("stack_nested_cross_domain_auth_callback_url", "https://demo.stack-auth.com/");
|
||||
const urlAtConstructionTime = new URL(strippedUrl);
|
||||
urlAtConstructionTime.searchParams.set("code", "one-time-code");
|
||||
urlAtConstructionTime.searchParams.set("state", "nested-oauth-state");
|
||||
|
||||
// Construct before installing the window mock so the constructor does not schedule its own
|
||||
// nested-auth resolution; the assertions below drive the handler explicitly.
|
||||
const clientApp = new StackClientApp({
|
||||
baseUrl: "http://localhost:12345",
|
||||
projectId,
|
||||
publishableClientKey: "stack-pk-test",
|
||||
tokenStore: "memory",
|
||||
redirectMethod: "window",
|
||||
noAutomaticPrefetch: true,
|
||||
});
|
||||
|
||||
globalThis.document = createMockDocument();
|
||||
globalThis.window = {
|
||||
location: {
|
||||
href: strippedUrl.toString(),
|
||||
replace: () => {
|
||||
throw new Error("INTENTIONAL_TEST_ABORT");
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
|
||||
vi.spyOn(clientApp as any, "_fetchCurrentRefreshTokenIdIfSignedIn").mockResolvedValue(null);
|
||||
vi.spyOn(clientApp as any, "_getCrossDomainHandoffParamsForRedirect").mockResolvedValue({
|
||||
state: "fresh-nested-state",
|
||||
codeChallenge: "fresh-nested-code-challenge",
|
||||
});
|
||||
vi.spyOn(clientApp as any, "_isTrusted").mockResolvedValue(true);
|
||||
|
||||
try {
|
||||
// Without the construction-time URL, the handler re-bounces (location.replace aborts).
|
||||
await expect((clientApp as any)._maybeHandleNestedCrossDomainAuth()).rejects.toThrowError("INTENTIONAL_TEST_ABORT");
|
||||
// With it, the in-flight OAuth callback wins and the handler stands down.
|
||||
await expect((clientApp as any)._maybeHandleNestedCrossDomainAuth(urlAtConstructionTime)).resolves.toBe(false);
|
||||
// The live-URL guard must also stand down on its own when code+state are still present.
|
||||
(globalThis.window as any).location.href = urlAtConstructionTime.toString();
|
||||
await expect((clientApp as any)._maybeHandleNestedCrossDomainAuth()).resolves.toBe(false);
|
||||
} finally {
|
||||
globalThis.window = previousWindow;
|
||||
globalThis.document = previousDocument;
|
||||
}
|
||||
});
|
||||
|
||||
it("passes the construction-time URL to the nested cross-domain auth handler", async () => {
|
||||
const projectId = "00000000-0000-4000-8000-000000000009";
|
||||
const previousWindow = globalThis.window;
|
||||
const previousDocument = globalThis.document;
|
||||
|
||||
const callbackUrl = new URL(`https://${projectId}.example-stack-hosted.test/handler/sign-in`);
|
||||
callbackUrl.searchParams.set("stack_nested_cross_domain_auth_refresh_token_id", "source-refresh-token-id");
|
||||
callbackUrl.searchParams.set("code", "one-time-code");
|
||||
callbackUrl.searchParams.set("state", "nested-oauth-state");
|
||||
const strippedUrl = new URL(callbackUrl);
|
||||
strippedUrl.searchParams.delete("code");
|
||||
strippedUrl.searchParams.delete("state");
|
||||
|
||||
globalThis.document = createMockDocument();
|
||||
globalThis.window = {
|
||||
location: {
|
||||
href: callbackUrl.toString(),
|
||||
},
|
||||
} as any;
|
||||
|
||||
const nestedAuthSpy = vi.spyOn(StackClientApp.prototype as any, "_maybeHandleNestedCrossDomainAuth").mockResolvedValue(false);
|
||||
|
||||
try {
|
||||
new StackClientApp({
|
||||
baseUrl: "http://localhost:12345",
|
||||
projectId,
|
||||
publishableClientKey: "stack-pk-test",
|
||||
tokenStore: "memory",
|
||||
redirectMethod: "window",
|
||||
noAutomaticPrefetch: true,
|
||||
});
|
||||
|
||||
// Simulate consumeOAuthCallbackQueryParams stripping code+state before microtasks run.
|
||||
(globalThis.window as any).location.href = strippedUrl.toString();
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(nestedAuthSpy).toHaveBeenCalledTimes(1);
|
||||
const urlArgument = nestedAuthSpy.mock.calls[0][0] as URL;
|
||||
expect(urlArgument).toBeInstanceOf(URL);
|
||||
expect(urlArgument.searchParams.get("code")).toBe("one-time-code");
|
||||
expect(urlArgument.searchParams.get("state")).toBe("nested-oauth-state");
|
||||
} finally {
|
||||
nestedAuthSpy.mockRestore();
|
||||
globalThis.window = previousWindow;
|
||||
globalThis.document = previousDocument;
|
||||
}
|
||||
});
|
||||
|
||||
it("uses direct sign-out instead of hosted sign-out redirects when code execution is available", async () => {
|
||||
const clientApp = new StackClientApp({
|
||||
baseUrl: "http://localhost:12345",
|
||||
|
||||
@ -718,8 +718,12 @@ export class _HexclaveClientAppImplIncomplete<HasTokenStore extends boolean, Pro
|
||||
}
|
||||
|
||||
if (isBrowserLike()) {
|
||||
// The OAuth callback resolution scheduled above synchronously strips `code` and `state`
|
||||
// from the URL before its token exchange, so the nested handler must decide based on the
|
||||
// URL the page was loaded with, not whatever is in the address bar when it runs.
|
||||
const urlAtConstructionTime = new URL(window.location.href);
|
||||
this._trackPendingAuthResolution(async () => {
|
||||
await this._maybeHandleNestedCrossDomainAuth();
|
||||
await this._maybeHandleNestedCrossDomainAuth(urlAtConstructionTime);
|
||||
});
|
||||
}
|
||||
|
||||
@ -890,11 +894,15 @@ export class _HexclaveClientAppImplIncomplete<HasTokenStore extends boolean, Pro
|
||||
return targetUrl.toString();
|
||||
}
|
||||
|
||||
protected async _maybeHandleNestedCrossDomainAuth(): Promise<boolean> {
|
||||
protected async _maybeHandleNestedCrossDomainAuth(urlAtConstructionTime?: URL): Promise<boolean> {
|
||||
if (typeof window === "undefined") return false;
|
||||
const currentUrl = new URL(window.location.href);
|
||||
// A real OAuth callback wins over nested handoff detection on the final return to b.com.
|
||||
// The OAuth callback resolution strips `code` and `state` from the live URL before this
|
||||
// runs, so the check must also consult the URL captured at construction time — otherwise
|
||||
// we'd re-bounce to the source domain while the token exchange is still in flight.
|
||||
if (currentUrl.searchParams.has("code") && currentUrl.searchParams.has("state")) return false;
|
||||
if (urlAtConstructionTime != null && urlAtConstructionTime.searchParams.has("code") && urlAtConstructionTime.searchParams.has("state")) return false;
|
||||
const refreshTokenId = currentUrl.searchParams.get(nestedCrossDomainAuthQueryParams.refreshTokenId);
|
||||
if (refreshTokenId == null) return false;
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user