mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-04 21:04:37 +08:00
Fix various cross-domain auth bugs
This commit is contained in:
parent
197ad09eea
commit
1effedbc42
@ -521,5 +521,14 @@ A: Only auto-start hosted OAuth callback handling when the current URL has `code
|
||||
## Q: Should built-with hosted handler domains be manually configured as trusted domains?
|
||||
A: No. Treat the hosted handler origin for the project, such as `https://<project-id>.built-with-stack-auth.com` or the origin derived from `NEXT_PUBLIC_STACK_HOSTED_HANDLER_URL_TEMPLATE`, as an implicit trusted redirect domain on both client and backend validation paths. The hosted template must put `{projectId}` in the hostname so every project has its own origin; path-based templates like `https://host/{projectId}/{hostedPath}` are not safe for implicit origin trust.
|
||||
|
||||
## Q: How should post-auth redirects mint cross-domain auth codes right after sign-in?
|
||||
A: When a sign-in/sign-up/OAuth callback immediately redirects through the cross-domain authorize endpoint, pass the freshly returned `{ accessToken, refreshToken }` as an override token store into the redirect path. Do not rely on browser cookies having already flushed; otherwise the request can pair a new access token with an old refresh token and fail the backend refresh-token/session consistency check.
|
||||
|
||||
## Q: How should mixed-token cross-domain auth regressions be tested?
|
||||
A: Add a source-level template test that creates a client app with a stale persistent token store, calls `_createCrossDomainAuthRedirectUrl` with `overrideTokenStoreInit`, and patches only `sendClientRequest` on the real client interface so session creation remains intact. The assertion should inspect the session passed to the interface and require the fresh refresh token, proving the cross-domain authorize request does not use stale persisted tokens.
|
||||
|
||||
## Q: When should cross-domain auth call `captureError`?
|
||||
A: Do not call `captureError` for normal cross-domain auth failures such as stale/deleted cookies, untrusted redirect URLs, invalid or mismatched refresh tokens, missing handoff params, or interrupted auth flows. Reserve `captureError` for states that definitely imply a Stack developer mistake; ordinary auth failures should just return or throw their normal user-facing errors.
|
||||
|
||||
## Q: How should the npm publish workflow create the post-publish dev version bump?
|
||||
A: The workflow needs a full checkout using the fine-grained `NPM_PUBLISH_VERSION_UPDATE_PR_PAT` secret. It then fetches `origin/dev`, checks out `dev`, creates a non-interactive patch changeset, runs `pnpm changeset version`, copies the generated `packages/template/package.json` version line back into `packages/template/package-template.json`, and commit/pushes `chore: update package versions`. Because direct pushes to `dev` are blocked by repository rules requiring PRs and the `all-good` status check, the PAT's owning user or bot account must be added to the ruleset bypass list with "Always allow" rather than "For pull requests only".
|
||||
|
||||
@ -244,7 +244,10 @@ it("does not await pending auth resolutions when post-callback redirect mints a
|
||||
await expect((clientApp as any)._redirectToHandler(
|
||||
"afterSignIn",
|
||||
{ replace: true },
|
||||
{ awaitPendingAuthResolutions: false },
|
||||
{
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: { accessToken: "fresh-access-token", refreshToken: "fresh-refresh-token" },
|
||||
},
|
||||
)).rejects.toThrowError("INTENTIONAL_TEST_ABORT");
|
||||
} finally {
|
||||
globalThis.window = previousWindow;
|
||||
@ -253,6 +256,7 @@ it("does not await pending auth resolutions when post-callback redirect mints a
|
||||
|
||||
expect(createCrossDomainAuthRedirectUrlSpy).toHaveBeenCalledWith(expect.objectContaining({
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: { accessToken: "fresh-access-token", refreshToken: "fresh-refresh-token" },
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
@ -0,0 +1,61 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { StackClientApp } from "../interfaces/client-app";
|
||||
|
||||
describe("StackClientApp cross-domain auth", () => {
|
||||
it("uses the fresh post-auth refresh token when minting a cross-domain handoff", async () => {
|
||||
const clientApp = new StackClientApp({
|
||||
baseUrl: "http://localhost:12345",
|
||||
projectId: "00000000-0000-4000-8000-000000000000",
|
||||
publishableClientKey: "stack-pk-test",
|
||||
tokenStore: {
|
||||
accessToken: "stale-access-token",
|
||||
refreshToken: "stale-refresh-token",
|
||||
},
|
||||
redirectMethod: "none",
|
||||
noAutomaticPrefetch: true,
|
||||
});
|
||||
|
||||
const clientInterface = Reflect.get(clientApp, "_interface");
|
||||
const originalSendClientRequest = Reflect.get(clientInterface, "sendClientRequest");
|
||||
const capturedRefreshTokens: string[] = [];
|
||||
|
||||
Reflect.set(clientInterface, "sendClientRequest", async (_path: unknown, _requestOptions: unknown, session: unknown) => {
|
||||
const getRefreshToken = Reflect.get(session ?? {}, "getRefreshToken");
|
||||
if (typeof getRefreshToken !== "function") {
|
||||
throw new Error("Expected cross-domain auth to pass a session to the client interface.");
|
||||
}
|
||||
const refreshToken = getRefreshToken.call(session);
|
||||
const refreshTokenString = Reflect.get(refreshToken ?? {}, "token");
|
||||
if (typeof refreshTokenString !== "string") {
|
||||
throw new Error("Expected cross-domain auth to pass a refresh-token-backed session.");
|
||||
}
|
||||
capturedRefreshTokens.push(refreshTokenString);
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => ({ redirect_url: "https://example.com/handler/oauth-callback?code=handoff-code&state=handoff-state" }),
|
||||
};
|
||||
});
|
||||
|
||||
try {
|
||||
const createCrossDomainAuthRedirectUrl = Reflect.get(clientApp, "_createCrossDomainAuthRedirectUrl");
|
||||
if (typeof createCrossDomainAuthRedirectUrl !== "function") {
|
||||
throw new Error("Expected StackClientApp to expose _createCrossDomainAuthRedirectUrl in tests.");
|
||||
}
|
||||
|
||||
await expect(createCrossDomainAuthRedirectUrl.call(clientApp, {
|
||||
redirectUri: "https://example.com/handler/oauth-callback",
|
||||
state: "handoff-state",
|
||||
codeChallenge: "abcdefghijklmnopqrstuvwxyzABCDEFG_0123456789-._~",
|
||||
afterCallbackRedirectUrl: "https://example.com/account-settings",
|
||||
overrideTokenStoreInit: {
|
||||
accessToken: "fresh-access-token",
|
||||
refreshToken: "fresh-refresh-token",
|
||||
},
|
||||
})).resolves.toBe("https://example.com/handler/oauth-callback?code=handoff-code&state=handoff-state");
|
||||
} finally {
|
||||
Reflect.set(clientInterface, "sendClientRequest", originalSendClientRequest);
|
||||
}
|
||||
|
||||
expect(capturedRefreshTokens).toEqual(["fresh-refresh-token"]);
|
||||
});
|
||||
});
|
||||
@ -818,8 +818,11 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
return currentUrl.toString();
|
||||
}
|
||||
|
||||
protected async _getCurrentRefreshTokenIdIfSignedIn(options?: { awaitPendingAuthResolutions?: boolean }): Promise<string | null> {
|
||||
const session = await this._getSession(undefined, options);
|
||||
protected async _getCurrentRefreshTokenIdIfSignedIn(options?: {
|
||||
awaitPendingAuthResolutions?: boolean,
|
||||
overrideTokenStoreInit?: TokenStoreInit,
|
||||
}): Promise<string | null> {
|
||||
const session = await this._getSession(options?.overrideTokenStoreInit, options);
|
||||
const tokens = await session.getOrFetchLikelyValidTokens(0, null);
|
||||
if (tokens?.refreshToken == null) {
|
||||
return null;
|
||||
@ -831,6 +834,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
url: string,
|
||||
currentUrl: URL,
|
||||
awaitPendingAuthResolutions?: boolean,
|
||||
overrideTokenStoreInit?: TokenStoreInit,
|
||||
}): Promise<string> {
|
||||
const targetUrl = new URL(options.url, options.currentUrl);
|
||||
if (targetUrl.origin === options.currentUrl.origin) {
|
||||
@ -839,6 +843,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
|
||||
const refreshTokenId = await this._getCurrentRefreshTokenIdIfSignedIn({
|
||||
awaitPendingAuthResolutions: options.awaitPendingAuthResolutions,
|
||||
overrideTokenStoreInit: options.overrideTokenStoreInit,
|
||||
});
|
||||
if (refreshTokenId == null) {
|
||||
return options.url;
|
||||
@ -913,7 +918,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
const currentRefreshTokenId = await this._getCurrentRefreshTokenIdIfSignedIn({ awaitPendingAuthResolutions: false });
|
||||
if (currentRefreshTokenId === refreshTokenId) return false;
|
||||
const callbackUrlString = currentUrl.searchParams.get(nestedCrossDomainAuthQueryParams.callbackUrl);
|
||||
if (callbackUrlString == null) throw new StackAssertionError("Nested cross-domain auth URL is missing callback URL");
|
||||
if (callbackUrlString == null) {
|
||||
throw new StackAssertionError("Nested cross-domain auth URL is missing callback URL");
|
||||
}
|
||||
if (isRelative(callbackUrlString)) {
|
||||
throw new Error("Nested cross-domain auth callback URL must be absolute.");
|
||||
}
|
||||
@ -1466,6 +1473,16 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
this._currentUserCache.getOrWait([newSession], "write-only").catch(() => {});
|
||||
}
|
||||
|
||||
protected _getTokenStoreInitForFreshTokens(tokens: { accessToken: string | null, refreshToken: string }): TokenStoreInit | undefined {
|
||||
if (tokens.accessToken == null) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
accessToken: tokens.accessToken,
|
||||
refreshToken: tokens.refreshToken,
|
||||
};
|
||||
}
|
||||
|
||||
protected _hasPersistentTokenStore(overrideTokenStoreInit?: TokenStoreInit): this is StackClientApp<true, ProjectId> {
|
||||
return (overrideTokenStoreInit !== undefined ? overrideTokenStoreInit : this._tokenStoreInit) !== null;
|
||||
}
|
||||
@ -2760,8 +2777,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
codeChallenge: string,
|
||||
afterCallbackRedirectUrl: string,
|
||||
awaitPendingAuthResolutions?: boolean,
|
||||
overrideTokenStoreInit?: TokenStoreInit,
|
||||
}): Promise<string> {
|
||||
const session = await this._getSession(undefined, { awaitPendingAuthResolutions: options.awaitPendingAuthResolutions });
|
||||
const session = await this._getSession(options.overrideTokenStoreInit, { awaitPendingAuthResolutions: options.awaitPendingAuthResolutions });
|
||||
const response = await this._interface.sendClientRequest(
|
||||
"/auth/oauth/cross-domain/authorize",
|
||||
{
|
||||
@ -2780,7 +2798,8 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
session,
|
||||
);
|
||||
if (!response.ok) {
|
||||
throw new StackAssertionError(`Cross-domain authorization endpoint failed: ${response.status} ${await response.text()}`);
|
||||
const responseBody = await response.text();
|
||||
throw new StackAssertionError(`Cross-domain authorization endpoint failed: ${response.status} ${responseBody}`);
|
||||
}
|
||||
const result = await response.json();
|
||||
if (!("redirect_url" in result) || typeof result.redirect_url !== "string") {
|
||||
@ -2862,7 +2881,10 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
protected async _redirectToHandler(
|
||||
handlerName: keyof HandlerUrls,
|
||||
options?: RedirectToOptions,
|
||||
internalOptions?: { awaitPendingAuthResolutions?: boolean },
|
||||
internalOptions?: {
|
||||
awaitPendingAuthResolutions?: boolean,
|
||||
overrideTokenStoreInit?: TokenStoreInit,
|
||||
},
|
||||
) {
|
||||
const rawUrls = getUrls(this._urlOptions, { projectId: this.projectId });
|
||||
const rawHandlerUrl = rawUrls[handlerName];
|
||||
@ -2889,6 +2911,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
codeChallenge: plan.codeChallenge,
|
||||
afterCallbackRedirectUrl: plan.afterCallbackRedirectUrl,
|
||||
awaitPendingAuthResolutions: internalOptions?.awaitPendingAuthResolutions,
|
||||
overrideTokenStoreInit: internalOptions?.overrideTokenStoreInit,
|
||||
});
|
||||
await this._redirectTo({ url: crossDomainRedirectUrl, ...options });
|
||||
return;
|
||||
@ -2899,6 +2922,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
url: plan.url,
|
||||
currentUrl,
|
||||
awaitPendingAuthResolutions: internalOptions?.awaitPendingAuthResolutions,
|
||||
overrideTokenStoreInit: internalOptions?.overrideTokenStoreInit,
|
||||
})
|
||||
: plan.url;
|
||||
await this._redirectIfTrusted(redirectUrl, options);
|
||||
@ -3334,7 +3358,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
if (result.status === 'ok') {
|
||||
await this._signInToAccountWithTokens(result.data);
|
||||
if (!options.noRedirect) {
|
||||
await this.redirectToAfterSignIn({ replace: true });
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, {
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
}
|
||||
return Result.ok(undefined);
|
||||
} else {
|
||||
@ -3396,7 +3422,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
if (result.status === 'ok') {
|
||||
await this._signInToAccountWithTokens(result.data);
|
||||
if (!options.noRedirect) {
|
||||
await this.redirectToAfterSignUp({ replace: true });
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, {
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
}
|
||||
return Result.ok(undefined);
|
||||
} else {
|
||||
@ -3444,9 +3472,15 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
await this._signInToAccountWithTokens(result.data);
|
||||
if (!(options?.noRedirect)) {
|
||||
if (result.data.newUser) {
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, { awaitPendingAuthResolutions: false });
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, {
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
} else {
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, { awaitPendingAuthResolutions: false });
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, {
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
}
|
||||
}
|
||||
return Result.ok(undefined);
|
||||
@ -3584,9 +3618,13 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
await this._signInToAccountWithTokens(result.data);
|
||||
if (!(options?.noRedirect)) {
|
||||
if (result.data.newUser) {
|
||||
await this.redirectToAfterSignUp({ replace: true });
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, {
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
} else {
|
||||
await this.redirectToAfterSignIn({ replace: true });
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, {
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
}
|
||||
}
|
||||
return Result.ok(undefined);
|
||||
@ -3627,7 +3665,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
|
||||
if (result.status === 'ok') {
|
||||
await this._signInToAccountWithTokens(result.data);
|
||||
await this.redirectToAfterSignIn({ replace: true });
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, {
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
return Result.ok(undefined);
|
||||
} else {
|
||||
return Result.error(result.error);
|
||||
@ -3675,10 +3715,16 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
await this._redirectTo({ url: result.data.afterCallbackRedirectUrl, replace: true });
|
||||
return true;
|
||||
} else if (result.data.newUser) {
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, { awaitPendingAuthResolutions: false });
|
||||
await this._redirectToHandler("afterSignUp", { replace: true }, {
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
return true;
|
||||
} else {
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, { awaitPendingAuthResolutions: false });
|
||||
await this._redirectToHandler("afterSignIn", { replace: true }, {
|
||||
awaitPendingAuthResolutions: false,
|
||||
overrideTokenStoreInit: this._getTokenStoreInitForFreshTokens(result.data),
|
||||
});
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user