From e2dc5f5ee015916724d1643906f83a2a77dd3223 Mon Sep 17 00:00:00 2001 From: Mantra <87142457+mantrakp04@users.noreply.github.com> Date: Tue, 28 Apr 2026 16:33:59 -0700 Subject: [PATCH] [codex] fix OAuth redirect contract (#1393) ## Summary - Route browser OAuth redirects through the configured `redirectMethod` instead of hardcoded `window.location` calls. - Keep OAuth redirect APIs pending after navigation starts, including custom redirect methods. - Add `cliAuthConfirm` handler URL metadata and custom-page prompt coverage. - Update SDK spec text for browser OAuth callback and `returnTo` behavior. ## Root Cause OAuth helpers previously combined URL construction with direct browser navigation. That bypassed configured redirect methods and made it too easy for public redirect APIs to resolve after navigation started. ## Impact Browser SDK consumers get consistent redirect behavior across built-in and custom navigation methods. `returnTo` is handled as the post-callback destination while the OAuth callback URL remains fixed to the configured handler route. ## Validation - `pnpm test run packages/template/src/lib/auth.test.ts` - `pnpm test run apps/e2e/tests/js/oauth.test.ts` - `pnpm -C packages/template lint` - `pnpm -C apps/e2e lint` - `pnpm -C packages/template typecheck` - `pnpm -C apps/e2e typecheck` ## Summary by CodeRabbit * **New Features** * Added CLI authorization confirmation page/flow for terminal-based auth. * Added optional returnTo parameter for OAuth to control post-auth redirects. * Exposed configurable redirect behavior so apps follow the chosen redirect method. * **Bug Fixes** * OAuth callback now uses app navigation/queued redirects and shows a fallback link instead of forcing location.assign. * **Tests** * Added unit and e2e tests covering OAuth URL generation, scope handling, and CLI auth confirmation. --- apps/e2e/tests/js/oauth.test.ts | 53 ++++++++++++++++ .../src/components-page/oauth-callback.tsx | 18 ++++-- .../components-page/stack-handler-client.tsx | 40 +++++++++++- packages/template/src/lib/auth.test.ts | 63 +++++++++++++++++++ packages/template/src/lib/auth.ts | 9 +-- .../apps/implementations/client-app-impl.ts | 32 ++++++---- .../stack-app/apps/interfaces/client-app.ts | 2 + sdks/spec/src/apps/client-app.spec.md | 6 +- 8 files changed, 197 insertions(+), 26 deletions(-) create mode 100644 packages/template/src/lib/auth.test.ts diff --git a/apps/e2e/tests/js/oauth.test.ts b/apps/e2e/tests/js/oauth.test.ts index ebd1b478b..d2d55272f 100644 --- a/apps/e2e/tests/js/oauth.test.ts +++ b/apps/e2e/tests/js/oauth.test.ts @@ -17,6 +17,7 @@ it("adds provider_scope from oauthScopesOnSignIn for authenticate flow", async ( }, { client: { + redirectMethod: "window", oauthScopesOnSignIn: { github: ["repo"], }, @@ -52,4 +53,56 @@ it("adds provider_scope from oauthScopesOnSignIn for authenticate flow", async ( expect(scope).toBe("user:email repo"); }, { timeout: 40_000 }); +it("does not resolve signInWithOAuth after a custom redirectMethod starts navigation", async ({ expect }) => { + const navigatedUrls: string[] = []; + const { clientApp } = await createApp( + { + config: { + oauthProviders: [ + { + id: "github", + type: "standard", + clientId: "test_client_id", + clientSecret: "test_client_secret", + }, + ], + }, + }, + { + client: { + redirectMethod: { + useNavigate: () => (url) => { + navigatedUrls.push(url); + }, + navigate: (url) => { + navigatedUrls.push(url); + }, + }, + }, + } + ); + const previousWindow = globalThis.window; + const previousDocument = globalThis.document; + globalThis.document = { cookie: "", createElement: () => ({}) } as any; + globalThis.window = { + location: { + href: localRedirectUrl, + }, + } as any; + + try { + const redirectResult = clientApp.signInWithOAuth("github").then(() => "resolved"); + const result = await Promise.race([ + redirectResult, + new Promise((resolve) => setTimeout(() => resolve("pending"), 5000)), + ]); + + expect(navigatedUrls).toHaveLength(1); + expect(new URL(navigatedUrls[0]).pathname).toBe("/login/oauth/authorize"); + expect(result).toBe("pending"); + } finally { + globalThis.window = previousWindow; + globalThis.document = previousDocument; + } +}, { timeout: 40_000 }); diff --git a/packages/template/src/components-page/oauth-callback.tsx b/packages/template/src/components-page/oauth-callback.tsx index da800b4a0..8310b033f 100644 --- a/packages/template/src/components-page/oauth-callback.tsx +++ b/packages/template/src/components-page/oauth-callback.tsx @@ -8,6 +8,7 @@ import { useEffect, useRef, useState } from "react"; import { useStackApp } from ".."; import { MaybeFullPage } from "../components/elements/maybe-full-page"; import { StyledLink } from "../components/link"; +import { stackAppInternalsSymbol } from "../lib/stack-app"; import { useTranslation } from "../lib/translations"; export function OAuthCallback({ fullPage }: { fullPage?: boolean }) { @@ -15,10 +16,19 @@ export function OAuthCallback({ fullPage }: { fullPage?: boolean }) { const app = useStackApp(); const called = useRef(false); const [showRedirectLink, setShowRedirectLink] = useState(false); + const [redirectUrl, setRedirectUrl] = useState(null); useEffect(() => runAsynchronously(async () => { if (called.current) return; called.current = true; + const redirectToError = async (url: URL) => { + const urlString = url.toString(); + if (app[stackAppInternalsSymbol].getRedirectMethod() === "none") { + setRedirectUrl(urlString); + return; + } + await app[stackAppInternalsSymbol].redirectToUrl(urlString, { replace: true }); + }; try { const hasRedirected = await app.callOAuthCallback(); if (!hasRedirected) { @@ -30,13 +40,13 @@ export function OAuthCallback({ fullPage }: { fullPage?: boolean }) { errorUrl.searchParams.set("errorCode", e.errorCode); errorUrl.searchParams.set("message", e.message); errorUrl.searchParams.set("details", JSON.stringify(e.details ?? {})); - window.location.replace(errorUrl.toString()); + await redirectToError(errorUrl); return; } captureError("", e); - window.location.replace(new URL(app.urls.error, window.location.href).toString()); + await redirectToError(new URL(app.urls.error, window.location.href)); } - }), []); + }), [app]); useEffect(() => { setTimeout(() => setShowRedirectLink(true), 3000); @@ -56,7 +66,7 @@ export function OAuthCallback({ fullPage }: { fullPage?: boolean }) {
- {showRedirectLink ?

{t('If you are not redirected automatically, ')}{t("click here")}

: null} + {showRedirectLink || redirectUrl != null ?

{t('If you are not redirected automatically, ')}{t("click here")}

: null} ); diff --git a/packages/template/src/components-page/stack-handler-client.tsx b/packages/template/src/components-page/stack-handler-client.tsx index 82c28c93e..55cfe9fe0 100644 --- a/packages/template/src/components-page/stack-handler-client.tsx +++ b/packages/template/src/components-page/stack-handler-client.tsx @@ -5,6 +5,9 @@ import { FilterUndefined, filterUndefined } from "@stackframe/stack-shared/dist/ import { getRelativePart } from "@stackframe/stack-shared/dist/utils/urls"; import { notFound, redirect, RedirectType, usePathname, useSearchParams } from 'next/navigation'; // THIS_LINE_PLATFORM next import { useMemo } from 'react'; +/* IF_PLATFORM react +import { useEffect, useRef } from 'react'; +// END_PLATFORM */ import { SignIn, SignUp, StackServerApp } from ".."; import { useStackApp } from "../lib/hooks"; import { HandlerUrls, StackClientApp, stackAppInternalsSymbol } from "../lib/stack-app"; @@ -230,8 +233,12 @@ export function StackHandlerClient(props: BaseHandlerProps & Partial const currentLocation = pathname; const searchParamsSource = searchParamsFromHook; /* ELSE_IF_PLATFORM react + const navigate = stackApp.useNavigate(); + const navigateRef = useRef(navigate); + navigateRef.current = navigate; const currentLocation = props.location ?? window.location.pathname; const searchParamsSource = new URLSearchParams(window.location.search); + const redirectTargets: (string | undefined)[] = []; END_PLATFORM */ const { path, searchParams, handlerPath } = useMemo(() => { @@ -278,7 +285,7 @@ export function StackHandlerClient(props: BaseHandlerProps & Partial // IF_PLATFORM next redirect(toAbsoluteOrRelativeRedirectTarget(urlObj), RedirectType.replace); /* ELSE_IF_PLATFORM react - window.location.href = toAbsoluteOrRelativeRedirectTarget(urlObj); + redirectTargets.push(toAbsoluteOrRelativeRedirectTarget(urlObj)); END_PLATFORM */ }; @@ -312,11 +319,38 @@ export function StackHandlerClient(props: BaseHandlerProps & Partial // IF_PLATFORM next redirect(result.redirect, RedirectType.replace); /* ELSE_IF_PLATFORM react - window.location.href = result.redirect; - return null; + redirectTargets.push(result.redirect); END_PLATFORM */ } + /* IF_PLATFORM react + const redirectTarget = redirectTargets[0]; + const shouldRenderRedirectFallback = redirectTarget != null && stackApp[stackAppInternalsSymbol].getRedirectMethod() === "none"; + useEffect(() => { + if (redirectTarget == null || shouldRenderRedirectFallback) { + return; + } + navigateRef.current(redirectTarget); + }, [redirectTarget, shouldRenderRedirectFallback]); + + if (redirectTarget != null && shouldRenderRedirectFallback) { + return ( + window.location.assign(redirectTarget)} + > + Continue to the next page. + + ); + } + + if (redirectTarget != null) { + return null; + } + END_PLATFORM */ + return result; } diff --git a/packages/template/src/lib/auth.test.ts b/packages/template/src/lib/auth.test.ts new file mode 100644 index 000000000..713c36529 --- /dev/null +++ b/packages/template/src/lib/auth.test.ts @@ -0,0 +1,63 @@ +// @vitest-environment jsdom + +import { StackClientInterface } from "@stackframe/stack-shared"; +import { describe, expect, it, vi } from "vitest"; +import { getNewOAuthProviderOrScopeUrl } from "./auth"; + +vi.mock("./cookie", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + saveVerifierAndState: async () => ({ + codeChallenge: "", + state: "", + }), + }; +}); + +describe("getNewOAuthProviderOrScopeUrl", () => { + it("returns the OAuth URL without performing navigation", async () => { + window.history.replaceState({}, "", "/account?after_auth_return_to=%2Fsettings"); + + const iface = new StackClientInterface({ + clientVersion: "test", + getBaseUrl: () => "https://api.example.com", + getApiUrls: () => ["https://api.example.com"], + extraRequestHeaders: {}, + projectId: "00000000-0000-4000-8000-000000000000", + publishableClientKey: "pck_test", + }); + const session = iface.createSession({ refreshToken: null, accessToken: null }); + + const location = await getNewOAuthProviderOrScopeUrl( + iface, + { + provider: "github", + redirectUrl: "/handler/oauth-callback", + errorRedirectUrl: "/handler/error", + providerScope: "repo user", + }, + session, + ); + + const url = new URL(location); + expect(`${url.origin}${url.pathname}`).toBe("https://api.example.com/api/v1/auth/oauth/authorize/github"); + expect(Object.fromEntries(url.searchParams.entries())).toMatchInlineSnapshot(` + { + "after_callback_redirect_url": "http://localhost:3000/account?after_auth_return_to=%2Fsettings", + "client_id": "00000000-0000-4000-8000-000000000000", + "client_secret": "pck_test", + "code_challenge": "", + "code_challenge_method": "S256", + "error_redirect_url": "http://localhost:3000/handler/error?after_auth_return_to=%2Fsettings", + "grant_type": "authorization_code", + "provider_scope": "repo user", + "redirect_uri": "http://localhost:3000/handler/oauth-callback?after_auth_return_to=%2Fsettings", + "response_type": "code", + "scope": "legacy", + "state": "", + "type": "link", + } + `); + }); +}); diff --git a/packages/template/src/lib/auth.ts b/packages/template/src/lib/auth.ts index 2a986af6e..a695b4ad4 100644 --- a/packages/template/src/lib/auth.ts +++ b/packages/template/src/lib/auth.ts @@ -1,12 +1,11 @@ import { KnownError, StackClientInterface } from "@stackframe/stack-shared"; import { InternalSession } from "@stackframe/stack-shared/dist/sessions"; import { StackAssertionError, throwErr } from "@stackframe/stack-shared/dist/utils/errors"; -import { neverResolve } from "@stackframe/stack-shared/dist/utils/promises"; import { Result } from "@stackframe/stack-shared/dist/utils/results"; import { deindent } from "@stackframe/stack-shared/dist/utils/strings"; import { constructRedirectUrl } from "../utils/url"; import { consumeVerifierAndStateCookie, saveVerifierAndState } from "./cookie"; -export async function addNewOAuthProviderOrScope( +export async function getNewOAuthProviderOrScopeUrl( iface: StackClientInterface, options: { provider: string, @@ -15,9 +14,9 @@ export async function addNewOAuthProviderOrScope( providerScope?: string, }, session: InternalSession, -) { +): Promise { const { codeChallenge, state } = await saveVerifierAndState(); - const location = await iface.getOAuthUrl({ + return await iface.getOAuthUrl({ provider: options.provider, redirectUrl: constructRedirectUrl(options.redirectUrl, "redirectUrl"), errorRedirectUrl: constructRedirectUrl(options.errorRedirectUrl, "errorRedirectUrl"), @@ -28,8 +27,6 @@ export async function addNewOAuthProviderOrScope( session, providerScope: options.providerScope, }); - window.location.assign(location); - await neverResolve(); } /** diff --git a/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts b/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts index 1671e16fe..1097699f1 100644 --- a/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts +++ b/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts @@ -41,7 +41,7 @@ import * as NextNavigationUnscrambled from "next/navigation"; // import the enti import React, { useCallback, useMemo } from "react"; // THIS_LINE_PLATFORM react-like import type * as yup from "yup"; import { constructRedirectUrl } from "../../../../utils/url"; -import { addNewOAuthProviderOrScope, callOAuthCallback } from "../../../auth"; +import { getNewOAuthProviderOrScopeUrl, callOAuthCallback } from "../../../auth"; import { CookieHelper, createBrowserCookieHelper, createCookieHelper, createPlaceholderCookieHelper, deleteCookie, deleteCookieClient, isSecure as isSecureCookieContext, saveVerifierAndState, setOrDeleteCookie, setOrDeleteCookieClient } from "../../../cookie"; import { envVars } from "../../../env"; import { ApiKey, ApiKeyCreationOptions, ApiKeyUpdateOptions, apiKeyCreationOptionsToCrud } from "../../api-keys"; @@ -220,7 +220,7 @@ export class _StackClientAppImplIncomplete { return await this._interface.authorizeOAuth({ provider, - redirectUrl: constructRedirectUrl(options?.returnTo ?? this.urls.oauthCallback, "redirectUrl"), + redirectUrl: constructRedirectUrl(this.urls.oauthCallback, "redirectUrl"), errorRedirectUrl: constructRedirectUrl(this.urls.error, "errorRedirectUrl"), afterCallbackRedirectUrl, type: "authenticate", @@ -2844,7 +2850,7 @@ export class _StackClientAppImplIncomplete { return await this._interface.sendClientRequest(path, requestOptions, await this._getSession(), requestType); }, + getRedirectMethod: () => this._redirectMethod ?? throwErr("Redirect method should have been initialized in the Stack client app constructor"), + redirectToUrl: async (url: string | URL, options?: { replace?: boolean }) => { + await this._redirectTo({ url, ...options }); + }, refreshOwnedProjects: async () => { await this._refreshOwnedProjects(await this._getSession()); }, diff --git a/packages/template/src/lib/stack-app/apps/interfaces/client-app.ts b/packages/template/src/lib/stack-app/apps/interfaces/client-app.ts index 6a6d20f2a..10d8e6a67 100644 --- a/packages/template/src/lib/stack-app/apps/interfaces/client-app.ts +++ b/packages/template/src/lib/stack-app/apps/interfaces/client-app.ts @@ -118,6 +118,8 @@ export type StackClientApp>, addRequestListener(listener: RequestListener): () => void, sendRequest(path: string, requestOptions: RequestInit, requestType?: "client" | "server" | "admin"): Promise, + getRedirectMethod(): RedirectMethod, + redirectToUrl(url: string | URL, options?: { replace?: boolean }): Promise, signInWithTokens(tokens: { accessToken: string, refreshToken: string }): Promise, }, } diff --git a/sdks/spec/src/apps/client-app.spec.md b/sdks/spec/src/apps/client-app.spec.md index 3972a5a98..8d2de04ec 100644 --- a/sdks/spec/src/apps/client-app.spec.md +++ b/sdks/spec/src/apps/client-app.spec.md @@ -52,6 +52,7 @@ Use an OAuth library (e.g., oauth4webapi) to handle PKCE and state management. Arguments: provider: string - OAuth provider ID (e.g., "google", "github", "microsoft") + options.returnTo: string [BROWSER-ONLY, optional] - URL to redirect to after the OAuth callback completes options.presentationContextProvider: platform-specific [NATIVE-ONLY] - iOS/macOS: ASWebAuthenticationPresentationContextProviding @@ -66,7 +67,8 @@ Note: Additional provider scopes are configured via oauthScopesOnSignIn construc Implementation: 1. Construct full redirect URLs using a fixed callback scheme: - Native apps: "stack-auth-mobile-oauth-url://success" and "stack-auth-mobile-oauth-url://error" - - Browser: Use window.location to construct absolute URLs + - Browser: Use the configured OAuth callback handler URL as redirect_uri and window.location to construct absolute URLs + - Browser: If options.returnTo is provided, pass it as afterCallbackRedirectUrl, not as redirect_uri 2. Call getOAuthUrl() with the constructed URLs to get: - Authorization URL @@ -79,7 +81,7 @@ Implementation: - Mobile/other: in-memory (passed directly to callback handler) 4. Open the authorization URL: - - Browser: window.location.assign(authorization_url) + - Browser: perform redirect according to redirectMethod - iOS/macOS: ASWebAuthenticationSession with callbackURLScheme: "stack-auth-mobile-oauth-url" - Android: Custom Tabs with callback URL registered as deep link - Desktop: Open system browser with registered URL scheme for callback