mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-13 21:01:21 +08:00
Various PR changes
This commit is contained in:
parent
ac1aa0f5e7
commit
ea158be9ed
@ -1 +1 @@
|
||||
Please review the PR comments with the `gh` CLI and fix those issues that are valid and relevant. Resolve the comments when you fix them. Also resolve all those comments that no longer exist or have already been resolved. Leave those comments that are mostly bullshit unresolved. Report the result to me in detail. Do NOT automatically commit or stage the changes back to the PR!
|
||||
Please review the PR comments with the `gh` CLI and fix those issues that are valid and relevant. Resolve the comments when you fix them. Also resolve all those comments that no longer exist or have already been resolved. Leave those comments that are mostly bullshit unresolved. Also, create or modify tests to make sure the fixed behavior works as expected. Report the result to me in detail (for the most important issues, whether resolved or unresolved, mark them with a ‼️ emoji). Do NOT automatically commit or stage the changes back to the PR!
|
||||
|
||||
@ -1,3 +1,4 @@
|
||||
import { checkApiKeySet, throwCheckApiKeySetError } from "@/lib/internal-api-keys";
|
||||
import { isAcceptedNativeAppUrl, validateRedirectUrl } from "@/lib/redirect-urls";
|
||||
import { Tenancy } from "@/lib/tenancies";
|
||||
import { isRefreshTokenValid } from "@/lib/tokens";
|
||||
@ -156,10 +157,19 @@ export const POST = createSmartRouteHandler({
|
||||
id: user.id,
|
||||
refreshTokenId: refreshTokenObj.id,
|
||||
};
|
||||
const publishableClientKey = headers["x-stack-publishable-client-key"]?.[0] ?? publishableClientKeyNotNecessarySentinel;
|
||||
const keyCheck = await checkApiKeySet(tenancy.project.id, { publishableClientKey });
|
||||
if (keyCheck.status === "error") {
|
||||
throwCheckApiKeySetError(
|
||||
keyCheck.error,
|
||||
tenancy.project.id,
|
||||
new KnownErrors.InvalidPublishableClientKey(tenancy.project.id),
|
||||
);
|
||||
}
|
||||
const redirectUrl = await createCrossDomainAuthorizeRedirect({
|
||||
tenancy,
|
||||
user: userWithSession,
|
||||
publishableClientKey: headers["x-stack-publishable-client-key"]?.[0] ?? publishableClientKeyNotNecessarySentinel,
|
||||
publishableClientKey,
|
||||
body,
|
||||
});
|
||||
|
||||
|
||||
@ -8,7 +8,7 @@ import { createRefreshTokenObj, decodeAccessToken, generateAccessTokenFromRefres
|
||||
import { getPrismaClientForTenancy, globalPrismaClient } from "@/prisma-client";
|
||||
import { AuthorizationCode, AuthorizationCodeModel, Client, Falsey, RefreshToken, Token, User } from "@node-oauth/oauth2-server";
|
||||
import { KnownErrors } from "@stackframe/stack-shared";
|
||||
import { StackAssertionError, captureError, throwErr } from "@stackframe/stack-shared/dist/utils/errors";
|
||||
import { StackAssertionError, StatusError, captureError, throwErr } from "@stackframe/stack-shared/dist/utils/errors";
|
||||
import { getProjectBranchFromClientId } from ".";
|
||||
const PrismaClientKnownRequestError = Prisma.PrismaClientKnownRequestError;
|
||||
|
||||
@ -124,10 +124,7 @@ export class OAuthModel implements AuthorizationCodeModel {
|
||||
},
|
||||
});
|
||||
if (refreshTokenObj && refreshTokenObj.projectUserId !== user.id) {
|
||||
throw new StackAssertionError("Cross-domain handoff refresh token does not belong to the authenticated user", {
|
||||
refreshTokenProjectUserId: refreshTokenObj.projectUserId,
|
||||
userId: user.id,
|
||||
});
|
||||
throw new StatusError(401, "Cross-domain handoff refresh token does not belong to the authenticated user.");
|
||||
}
|
||||
if (refreshTokenObj && await isRefreshTokenValid({ tenancy, refreshTokenObj })) {
|
||||
return refreshTokenObj;
|
||||
@ -151,7 +148,7 @@ export class OAuthModel implements AuthorizationCodeModel {
|
||||
}
|
||||
|
||||
async saveToken(token: Token, client: Client, user: User): Promise<Token | Falsey> {
|
||||
const afterCallbackRedirectUrl = user.afterCallbackRedirectUrl;
|
||||
const afterCallbackRedirectUrl = user.afterCallbackRedirectUrl ?? null;
|
||||
|
||||
if (token.refreshToken) {
|
||||
const tenancy = await getSoleTenancyFromProjectBranch(...getProjectBranchFromClientId(client.id));
|
||||
|
||||
@ -80,7 +80,7 @@ Q: How should unsubscribe-link e2e tests avoid breakage from email theme/layout
|
||||
A: In `apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts`, avoid snapshotting the entire rendered HTML for transactional emails; assert stable behavior instead (email content present and `/api/v1/emails/unsubscribe-link` absent) so cosmetic wrapper/style changes do not fail the test.
|
||||
|
||||
Q: How do cross-domain auth handoffs avoid creating extra refresh-token sessions?
|
||||
A: The cross-domain authorize route must carry the current `refreshTokenId` through authorization-code exchange and OAuth token issuance must reuse that ID. We encode handoff metadata (`afterCallbackRedirectUrl`, `refreshTokenId`) into a prefixed payload stored in `afterCallbackRedirectUrl`, decode it in `OAuthModel._getOrCreateRefreshTokenObj`, and sanitize `after_callback_redirect_url` in `saveToken` so clients only receive the original redirect URL.
|
||||
A: The cross-domain authorize route must carry the current `refreshTokenId` through authorization-code exchange and OAuth token issuance must reuse that ID. Keep `afterCallbackRedirectUrl` URL-only and persist refresh-token linkage in `ProjectUserAuthorizationCode.grantedRefreshTokenId`; then return that as `user.refreshTokenId` in `getAuthorizationCode` so token issuance can reuse the same refresh-token row with ownership checks.
|
||||
|
||||
Q: Is there a manual demo page for cross-domain auth handoff verification?
|
||||
A: Yes — `examples/demo/src/app/cross-domain-handoff/page.tsx` provides one-click triggers for client sign-in/sign-up redirects, server protected-page redirects, and OAuth provider sign-in, plus runtime URL visibility for manual verification.
|
||||
@ -97,6 +97,9 @@ A: Cross-domain handoff should still run when handoff params indicate a differen
|
||||
Q: How should `app.urls.signIn`/`signOut` behave for hosted cross-domain flows?
|
||||
A: In browser contexts, `app.urls` should return redirect-ready handler URLs for `signIn`, `signUp`, `onboarding`, and `signOut`: include `after_auth_return_to`, preserve existing cross-domain handoff params, and for hosted sign-in/up/onboarding populate cross-domain callback targets (`/handler/oauth-callback` with `stack_cross_domain_auth=1`) so plain `router.push(app.urls.signIn)` / `<Link href={app.urls.signOut}>` keeps return-to-domain behavior.
|
||||
|
||||
Q: What should happen if hosted `after_auth_return_to` requires cross-domain handoff but URL params are missing?
|
||||
A: In `planRedirectToHandler` (`redirect-page-urls.ts`), do not throw immediately. Generate missing PKCE handoff `state`/`codeChallenge` via `getCrossDomainHandoffParams(currentUrl)` and default `afterCallbackRedirectUrl` to `currentUrl.toString()`, then continue with cross-domain authorize planning.
|
||||
|
||||
Q: How do we avoid duplicating redirect-back URL logic between `app.urls` and `redirectTo*`?
|
||||
A: Keep redirect math and handler policy decisions in `redirect-page-urls.ts` (for example `resolveAppUrlsForCurrentPage`, `resolveRedirectBackAwareHandlerUrlForRedirect`, `getHandlerRedirectPolicy`) and keep `client-app-impl` as orchestration only (state/cookies/network/redirect side effects). Redirect execution should start from raw resolved URLs (not `this.urls`) so `noRedirectBack` still works as expected.
|
||||
|
||||
@ -118,6 +121,9 @@ A: E2E JS tests import `@stackframe/js` from built `dist`, so new helper files c
|
||||
Q: How should dashboard pages update project config values?
|
||||
A: Do not call `project.updateConfig(...)` directly from dashboard pages; lint enforces using `useUpdateConfig()` from `apps/dashboard/src/lib/config-update.tsx` so pushable-config confirmation flows are handled consistently.
|
||||
|
||||
Q: How should EventTracker behave in test environments with partial DOM mocks?
|
||||
A: In `packages/template/src/lib/stack-app/apps/implementations/event-tracker.ts`, gate `start()` behind runtime capability checks (DOM listener APIs and screen dimensions), and patch `window.history` instead of global `history`. This prevents crashes like `Cannot read properties of undefined (reading 'width')` in non-browser test stubs while keeping browser behavior unchanged.
|
||||
|
||||
Q: How can the dashboard find resumable onboarding state without SDK type changes?
|
||||
A: Query `/internal/projects` via `stackAppInternalsSymbol` and read each project's `onboarding_status`; this avoids relying on `AdminOwnedProject` fields that may lag until generated package copies are rebuilt.
|
||||
|
||||
|
||||
@ -21,13 +21,15 @@ export function OAuthCallback({ fullPage }: { fullPage?: boolean }) {
|
||||
if (called.current) return;
|
||||
called.current = true;
|
||||
let hasRedirected = false;
|
||||
let callbackError: unknown = null;
|
||||
try {
|
||||
hasRedirected = await app.callOAuthCallback();
|
||||
} catch (e) {
|
||||
callbackError = e;
|
||||
captureError("<OAuthCallback />", e);
|
||||
setError(e);
|
||||
}
|
||||
if (!hasRedirected && (!error || envVars.NODE_ENV === 'production')) {
|
||||
if (!hasRedirected && (callbackError == null || envVars.NODE_ENV === "production")) {
|
||||
await app.redirectToSignIn({ noRedirectBack: true });
|
||||
}
|
||||
}), []);
|
||||
|
||||
@ -23,7 +23,9 @@ export async function signInWithOAuth(
|
||||
provider: options.provider,
|
||||
redirectUrl: constructRedirectUrl(options.redirectUrl, "redirectUrl"),
|
||||
errorRedirectUrl: constructRedirectUrl(options.errorRedirectUrl, "errorRedirectUrl"),
|
||||
afterCallbackRedirectUrl: options.afterCallbackRedirectUrl,
|
||||
afterCallbackRedirectUrl: options.afterCallbackRedirectUrl == null
|
||||
? undefined
|
||||
: constructRedirectUrl(options.afterCallbackRedirectUrl, "afterCallbackRedirectUrl"),
|
||||
codeChallenge,
|
||||
state,
|
||||
type: "authenticate",
|
||||
|
||||
@ -73,6 +73,8 @@ isReactServer = sc.isReactServer;
|
||||
const NextNavigation = scrambleDuringCompileTime(NextNavigationUnscrambled);
|
||||
// END_PLATFORM
|
||||
|
||||
const prefetchedCrossDomainHandoffTtlMs = 55 * 60 * 1000;
|
||||
|
||||
|
||||
const allClientApps = new Map<string, [checkString: string | undefined, app: StackClientApp<any, any>]>();
|
||||
|
||||
@ -374,6 +376,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
|
||||
private _anonymousSignUpInProgress: Promise<{ accessToken: string, refreshToken: string }> | null = null;
|
||||
private _prefetchedCrossDomainHandoffParams: CrossDomainHandoffParams | null = null;
|
||||
private _prefetchedCrossDomainHandoffParamsFetchedAt = 0;
|
||||
private _isPrefetchingCrossDomainHandoffParams = false;
|
||||
|
||||
protected async _createCookieHelper(overrideTokenStoreInit?: TokenStoreInit): Promise<CookieHelper> {
|
||||
@ -2162,18 +2165,22 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
protected _prefetchCrossDomainHandoffParamsIfNeeded() {
|
||||
const canWriteOauthVerifierCookie = this._tokenStoreInit === "cookie" || this._tokenStoreInit === "nextjs-cookie";
|
||||
if (
|
||||
typeof window === "undefined"
|
||||
!isBrowserLike()
|
||||
|| !canWriteOauthVerifierCookie
|
||||
|| this._isPrefetchingCrossDomainHandoffParams
|
||||
|| this._prefetchedCrossDomainHandoffParams != null
|
||||
|| this._getFreshPrefetchedCrossDomainHandoffParams() != null
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this._isPrefetchingCrossDomainHandoffParams = true;
|
||||
runAsynchronously(async () => {
|
||||
try {
|
||||
if (!isBrowserLike()) {
|
||||
return;
|
||||
}
|
||||
const { state, codeChallenge } = await saveVerifierAndState();
|
||||
this._prefetchedCrossDomainHandoffParams = { state, codeChallenge };
|
||||
this._prefetchedCrossDomainHandoffParamsFetchedAt = performance.now();
|
||||
} finally {
|
||||
this._isPrefetchingCrossDomainHandoffParams = false;
|
||||
}
|
||||
@ -2186,8 +2193,9 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
return fromQuery;
|
||||
}
|
||||
|
||||
if (this._prefetchedCrossDomainHandoffParams != null) {
|
||||
return this._prefetchedCrossDomainHandoffParams;
|
||||
const prefetched = this._getFreshPrefetchedCrossDomainHandoffParams();
|
||||
if (prefetched != null) {
|
||||
return prefetched;
|
||||
}
|
||||
|
||||
this._prefetchCrossDomainHandoffParamsIfNeeded();
|
||||
@ -2199,8 +2207,13 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
if (fromQuery != null) {
|
||||
return fromQuery;
|
||||
}
|
||||
const prefetched = this._getFreshPrefetchedCrossDomainHandoffParams();
|
||||
if (prefetched != null) {
|
||||
return prefetched;
|
||||
}
|
||||
const { state, codeChallenge } = await saveVerifierAndState();
|
||||
this._prefetchedCrossDomainHandoffParams = { state, codeChallenge };
|
||||
this._prefetchedCrossDomainHandoffParamsFetchedAt = performance.now();
|
||||
return { state, codeChallenge };
|
||||
}
|
||||
|
||||
@ -2209,6 +2222,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
urls: {
|
||||
...this._urlOptions,
|
||||
default: { type: "handler-component" },
|
||||
oauthCallback: { type: "handler-component" },
|
||||
},
|
||||
projectId: this.projectId,
|
||||
}).oauthCallback;
|
||||
@ -2238,6 +2252,9 @@ 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 result = await response.json();
|
||||
if (!("redirect_url" in result) || typeof result.redirect_url !== "string") {
|
||||
throw new StackAssertionError("Cross-domain authorization endpoint returned an invalid payload", { result });
|
||||
@ -2245,6 +2262,18 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
|
||||
return result.redirect_url;
|
||||
}
|
||||
|
||||
protected _getFreshPrefetchedCrossDomainHandoffParams(): CrossDomainHandoffParams | null {
|
||||
if (this._prefetchedCrossDomainHandoffParams == null) {
|
||||
return null;
|
||||
}
|
||||
if (performance.now() - this._prefetchedCrossDomainHandoffParamsFetchedAt > prefetchedCrossDomainHandoffTtlMs) {
|
||||
this._prefetchedCrossDomainHandoffParams = null;
|
||||
this._prefetchedCrossDomainHandoffParamsFetchedAt = 0;
|
||||
return null;
|
||||
}
|
||||
return this._prefetchedCrossDomainHandoffParams;
|
||||
}
|
||||
|
||||
protected async _getCurrentUrl() {
|
||||
if (this._redirectMethod === "none") {
|
||||
return null;
|
||||
|
||||
@ -31,8 +31,8 @@ export class EventTracker {
|
||||
private readonly _sessionReplaySegmentId: string;
|
||||
private readonly _deps: EventTrackerDeps;
|
||||
|
||||
private _originalPushState: typeof history.pushState | null = null;
|
||||
private _originalReplaceState: typeof history.replaceState | null = null;
|
||||
private _originalPushState: History["pushState"] | null = null;
|
||||
private _originalReplaceState: History["replaceState"] | null = null;
|
||||
|
||||
constructor(deps: EventTrackerDeps) {
|
||||
this._deps = deps;
|
||||
@ -42,6 +42,18 @@ export class EventTracker {
|
||||
start() {
|
||||
if (this._started) return;
|
||||
if (!isBrowserLike()) return;
|
||||
const screenObject = Reflect.get(window, "screen");
|
||||
if (
|
||||
typeof window.addEventListener !== "function"
|
||||
|| typeof window.removeEventListener !== "function"
|
||||
|| typeof document.addEventListener !== "function"
|
||||
|| typeof document.removeEventListener !== "function"
|
||||
|| typeof screenObject !== "object"
|
||||
|| typeof Reflect.get(screenObject, "width") !== "number"
|
||||
|| typeof Reflect.get(screenObject, "height") !== "number"
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this._started = true;
|
||||
|
||||
this._setupPageViewCapture();
|
||||
@ -99,20 +111,26 @@ export class EventTracker {
|
||||
private _setupPageViewCapture() {
|
||||
// Fire initial page-view
|
||||
this._capturePageView("initial");
|
||||
const historyObject = window["history"];
|
||||
const originalPushState = Reflect.get(historyObject, "pushState");
|
||||
const originalReplaceState = Reflect.get(historyObject, "replaceState");
|
||||
if (typeof originalPushState !== "function" || typeof originalReplaceState !== "function") {
|
||||
return;
|
||||
}
|
||||
|
||||
// Monkey-patch history.pushState
|
||||
this._originalPushState = history.pushState.bind(history);
|
||||
history.pushState = (...args: Parameters<typeof history.pushState>) => {
|
||||
this._originalPushState = (...args: Parameters<History["pushState"]>) => originalPushState.apply(historyObject, args);
|
||||
Reflect.set(historyObject, "pushState", (...args: Parameters<History["pushState"]>) => {
|
||||
this._originalPushState!(...args);
|
||||
this._capturePageView("push");
|
||||
};
|
||||
});
|
||||
|
||||
// Monkey-patch history.replaceState
|
||||
this._originalReplaceState = history.replaceState.bind(history);
|
||||
history.replaceState = (...args: Parameters<typeof history.replaceState>) => {
|
||||
this._originalReplaceState = (...args: Parameters<History["replaceState"]>) => originalReplaceState.apply(historyObject, args);
|
||||
Reflect.set(historyObject, "replaceState", (...args: Parameters<History["replaceState"]>) => {
|
||||
this._originalReplaceState!(...args);
|
||||
this._capturePageView("replace");
|
||||
};
|
||||
});
|
||||
|
||||
// Listen for popstate (back/forward navigation)
|
||||
window.addEventListener("popstate", this._onPopState);
|
||||
@ -205,12 +223,13 @@ export class EventTracker {
|
||||
}
|
||||
|
||||
// Restore history methods
|
||||
const historyObject = Reflect.get(window, "history");
|
||||
if (this._originalPushState) {
|
||||
history.pushState = this._originalPushState;
|
||||
Reflect.set(historyObject, "pushState", this._originalPushState);
|
||||
this._originalPushState = null;
|
||||
}
|
||||
if (this._originalReplaceState) {
|
||||
history.replaceState = this._originalReplaceState;
|
||||
Reflect.set(historyObject, "replaceState", this._originalReplaceState);
|
||||
this._originalReplaceState = null;
|
||||
}
|
||||
|
||||
|
||||
@ -257,16 +257,15 @@ export async function planRedirectToHandler(options: {
|
||||
if (crossDomainHandoff == null) {
|
||||
return { type: "redirect", url: redirectBackUrl };
|
||||
}
|
||||
const state = crossDomainHandoff.handoffParams.state;
|
||||
const codeChallenge = crossDomainHandoff.handoffParams.codeChallenge;
|
||||
const afterCallbackRedirectUrl = crossDomainHandoff.handoffParams.afterCallbackRedirectUrl;
|
||||
if (state == null || codeChallenge == null || afterCallbackRedirectUrl == null) {
|
||||
throw new StackAssertionError("Cross-domain handoff policy was selected but required params were missing", {
|
||||
state,
|
||||
codeChallenge,
|
||||
afterCallbackRedirectUrl,
|
||||
});
|
||||
let state = crossDomainHandoff.handoffParams.state;
|
||||
let codeChallenge = crossDomainHandoff.handoffParams.codeChallenge;
|
||||
let afterCallbackRedirectUrl = crossDomainHandoff.handoffParams.afterCallbackRedirectUrl;
|
||||
if (state == null || codeChallenge == null) {
|
||||
const generatedHandoffParams = await options.getCrossDomainHandoffParams(options.currentUrl);
|
||||
state ??= generatedHandoffParams.state;
|
||||
codeChallenge ??= generatedHandoffParams.codeChallenge;
|
||||
}
|
||||
afterCallbackRedirectUrl ??= options.currentUrl.toString();
|
||||
return {
|
||||
type: "cross-domain-authorize",
|
||||
redirectUri: crossDomainHandoff.redirectBackTarget.toString(),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user