From 3055f931f7a7a032ca384b6980774c81f0cc11df Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 7 Sep 2026 18:29:35 -0400 Subject: [PATCH] feat(oauth): read the returnTo a sign-in asked for and land on it startOAuthLogin has taken a returnTo since OAuth landed here and nothing ever read one back. The auth server validated it against the configured origins and signed it into the state, but finishOAuthLogin was typed as a bare MessageResult, so the whole callback body was discarded and an adopter had no way to learn where the flow had been asked to end up. finishOAuthLogin now resolves to FinishOAuthLoginResult, the completed response minus its session material, for the same reason LoginStartResult drops it: sessions are carried by cookies, so there is no reason to hand an adopter raw tokens. The bundled OAuthCallback view lands there instead of always going to /. This is the gap the magic link redirect closed in 0.10.0: the headless client could reach the feature and an application using AuthRoutes could not. The view only follows a destination on its own origin. That is not the guard against an open redirect, which the auth server applied before signing the state. It is that these views route with react-router, which cannot leave the application. An adopter that wants to reads returnTo off the result and navigates itself. --- .changeset/lucky-swans-arrive.md | 28 ++++++++++++ package-lock.json | 12 +++--- package.json | 2 +- src/AuthProvider.tsx | 3 +- src/client/createSeamlessAuthClient.ts | 20 ++++++++- src/index.ts | 2 + src/session/createAuthSession.ts | 3 +- src/views/OAuthCallback.tsx | 44 +++++++++++++++---- tests/OAuthCallback.test.tsx | 60 ++++++++++++++++++++++++++ 9 files changed, 154 insertions(+), 20 deletions(-) create mode 100644 .changeset/lucky-swans-arrive.md diff --git a/.changeset/lucky-swans-arrive.md b/.changeset/lucky-swans-arrive.md new file mode 100644 index 0000000..93e71cf --- /dev/null +++ b/.changeset/lucky-swans-arrive.md @@ -0,0 +1,28 @@ +--- +'@seamless-auth/react': minor +--- + +Read the `returnTo` an OAuth sign-in asked for, and land on it. + +`startOAuthLogin` has taken a `returnTo` since OAuth landed here, and nothing ever read one +back. The auth server validated it against the configured origins and signed it into the +state, but `finishOAuthLogin` was typed as a bare `MessageResult`, so the whole callback body +was discarded and an adopter had no way to learn where the flow had been asked to end up. + +`finishOAuthLogin` now resolves to `FinishOAuthLoginResult`, the completed OAuth response +minus its session material, for the same reason `LoginStartResult` drops it: sessions are +carried by cookies, so there is no reason to hand an adopter raw tokens. The new field on it +is `returnTo`, absent when the caller asked for nothing. + +The bundled `OAuthCallback` view lands there instead of always going to `/`. This is the same +gap the magic link redirect closed in 0.10.0: the headless client could reach the feature and +an application using `AuthRoutes` could not, which is the audience least likely to be wiring +up its own client. + +The view only follows a destination on its own origin. That is not the guard against an open +redirect, which the auth server already applied before signing the state; it is that these +views route with react-router, which cannot leave the application. An adopter that wants to +send someone to another origin reads `returnTo` off the result and navigates itself. + +Requires `@seamless-auth/types` 0.20.0, which carries the response field and holds both +`returnTo` fields to a scheme that can be a link destination. diff --git a/package-lock.json b/package-lock.json index c4db83a..a8d9991 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,15 +1,15 @@ { "name": "@seamless-auth/react", - "version": "0.9.0", + "version": "0.10.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@seamless-auth/react", - "version": "0.9.0", + "version": "0.10.0", "license": "AGPL-3.0-only", "dependencies": { - "@seamless-auth/types": "^0.16.0", + "@seamless-auth/types": "^0.20.0", "@simplewebauthn/browser": "^13.1.0", "eslint-plugin-license-header": "^0.9.0", "libphonenumber-js": "^1.12.7", @@ -3066,9 +3066,9 @@ "license": "MIT" }, "node_modules/@seamless-auth/types": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.16.0.tgz", - "integrity": "sha512-7W8w850S5B+6nekeIMG9Cmmouo6ABnoQ7iUpO62GG08HGx2Ur5j14pFB2CFs0MQWIQss5mcMkNPSY2NfB0UoNA==", + "version": "0.20.0", + "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.20.0.tgz", + "integrity": "sha512-e21oZDg1Ssf6zbAt1qswOSEKFC4kDX4JTLOKJKdkBgsy+f64Ii4xdKJcdLmvz1fErcN5I3Lzz6g0d8HnMmfoyg==", "license": "AGPL-3.0-only", "dependencies": { "zod": "^4.3.6" diff --git a/package.json b/package.json index 0e0a79c..b30fd95 100644 --- a/package.json +++ b/package.json @@ -102,7 +102,7 @@ "typescript-eslint": "^8.46.1" }, "dependencies": { - "@seamless-auth/types": "^0.16.0", + "@seamless-auth/types": "^0.20.0", "@simplewebauthn/browser": "^13.1.0", "eslint-plugin-license-header": "^0.9.0", "libphonenumber-js": "^1.12.7", diff --git a/src/AuthProvider.tsx b/src/AuthProvider.tsx index 405d653..a98bda9 100644 --- a/src/AuthProvider.tsx +++ b/src/AuthProvider.tsx @@ -7,6 +7,7 @@ import { CurrentUserResult, FinishOAuthLoginInput, + FinishOAuthLoginResult, LoginStartResult, MessageResult, OAuthProvidersResult, @@ -55,7 +56,7 @@ export interface AuthContextType { ) => Promise>; finishOAuthLogin: ( input: FinishOAuthLoginInput - ) => Promise>; + ) => Promise>; stepUpStatus: StepUpStatus | null; updateCredential: (credential: Credential) => Promise>; deleteCredential: (credentialId: string) => Promise>; diff --git a/src/client/createSeamlessAuthClient.ts b/src/client/createSeamlessAuthClient.ts index da48e29..82b3861 100644 --- a/src/client/createSeamlessAuthClient.ts +++ b/src/client/createSeamlessAuthClient.ts @@ -24,6 +24,7 @@ import type { LogoutScope as LogoutScopeShape, MeResponse, MessageResponse, + OAuthLoginSuccessResponse, OAuthProvidersResponse, OrganizationEnvelopeResponse, OrganizationListResponse, @@ -150,6 +151,21 @@ export interface FinishOAuthLoginInput { state: string; } +/** + * The completed OAuth response minus its session material, for the same reason + * `LoginStartResult` drops it: sessions are carried by cookies. + * + * What this does surface is `returnTo`, the destination the caller asked for when it + * started the flow. It comes back out of the state the auth server signed, so it is + * the value that was validated against the configured origins at the start and not + * one introduced at the callback. Absent when the caller asked for nothing, so treat + * that as "use my own default" rather than as a failure. + */ +export type FinishOAuthLoginResult = Omit< + OAuthLoginSuccessResponse, + 'token' | 'refreshToken' | 'sub' +>; + /** Response body for endpoints that only acknowledge the request. */ export type MessageResult = MessageResponse; @@ -265,7 +281,7 @@ export interface SeamlessAuthClient { ) => Promise>; finishOAuthLogin: ( input: FinishOAuthLoginInput - ) => Promise>; + ) => Promise>; registerPasskey: ( input: PasskeyMetadata | RegisterPasskeyOptions ) => Promise>; @@ -644,7 +660,7 @@ export const createSeamlessAuthClient = ( ), finishOAuthLogin: input => - requestResult( + requestResult( fetchWithAuth(`/oauth/${encodeURIComponent(input.providerId)}/callback`, { method: 'POST', body: JSON.stringify({ diff --git a/src/index.ts b/src/index.ts index faabc0d..0c02e75 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,7 @@ import { CreateOrganizationInput, CurrentUserResult, FinishOAuthLoginInput, + FinishOAuthLoginResult, LoginInput, LoginMethod, LoginStartResult, @@ -92,6 +93,7 @@ export type { CreateOrganizationInput, CurrentUserResult, FinishOAuthLoginInput, + FinishOAuthLoginResult, LoginInput, LoginMethod, LoginStartResult, diff --git a/src/session/createAuthSession.ts b/src/session/createAuthSession.ts index e947b7e..c2c0cf8 100644 --- a/src/session/createAuthSession.ts +++ b/src/session/createAuthSession.ts @@ -8,6 +8,7 @@ import { createSeamlessAuthClient, CurrentUserResult, FinishOAuthLoginInput, + FinishOAuthLoginResult, LoginStartResult, MessageResult, OAuthProvidersResult, @@ -59,7 +60,7 @@ export interface AuthSessionActions { ) => Promise>; finishOAuthLogin: ( input: FinishOAuthLoginInput - ) => Promise>; + ) => Promise>; refreshStepUpStatus: () => Promise>; verifyStepUpWithPasskey: () => Promise>; verifyStepUpWithPasskeyPrf: ( diff --git a/src/views/OAuthCallback.tsx b/src/views/OAuthCallback.tsx index 859ae9b..42626f0 100644 --- a/src/views/OAuthCallback.tsx +++ b/src/views/OAuthCallback.tsx @@ -14,6 +14,30 @@ import styles from '@/styles/verifyMagiclink.module.css'; const GENERIC_ERROR = 'We could not complete sign-in. Please try again.'; +/** + * The in-app path a `returnTo` names, or null. + * + * The auth server validated the destination against its configured origins before + * signing it into the state, so this is not the guard against an open redirect. It is + * a narrower question: these bundled views route with react-router, which can only + * move within this application, so a destination on another origin is not somewhere + * this component can send anyone. An adopter that wants to leave the app reads + * `returnTo` off the client result and navigates itself. + */ +function inAppPath(returnTo: string | undefined): string | null { + if (!returnTo) return null; + + try { + const target = new URL(returnTo, window.location.origin); + + if (target.origin !== window.location.origin) return null; + + return `${target.pathname}${target.search}${target.hash}`; + } catch { + return null; + } +} + const CODE_ERRORS: Record = { oauth_missing_email: 'Your provider account did not share an email address. Add an email to that account and make it visible, then try again.', @@ -43,16 +67,18 @@ const OAuthCallback: React.FC = () => { return; } - void finishOAuthLogin({ providerId, code, state }).then(({ error: finishError }) => { - if (finishError) { - const code = getOAuthErrorCode(finishError); - setError(code ? CODE_ERRORS[code] : GENERIC_ERROR); - return; - } + void finishOAuthLogin({ providerId, code, state }).then( + ({ data, error: finishError }) => { + if (finishError) { + const code = getOAuthErrorCode(finishError); + setError(code ? CODE_ERRORS[code] : GENERIC_ERROR); + return; + } - sessionStorage.removeItem(OAUTH_PROVIDER_STORAGE_KEY); - navigate('/'); - }); + sessionStorage.removeItem(OAUTH_PROVIDER_STORAGE_KEY); + navigate(inAppPath(data?.returnTo) ?? '/'); + } + ); }, [finishOAuthLogin, navigate, searchParams]); return ( diff --git a/tests/OAuthCallback.test.tsx b/tests/OAuthCallback.test.tsx index 9b0858b..a8eef69 100644 --- a/tests/OAuthCallback.test.tsx +++ b/tests/OAuthCallback.test.tsx @@ -49,6 +49,66 @@ describe('OAuthCallback', () => { expect(window.sessionStorage.getItem('seamless:oauth:provider')).toBeNull(); }); + // The destination was validated against the configured origins and signed into the + // state by the auth server, so it comes back as a URL rather than a path. + test('lands on the returnTo the flow asked for', async () => { + finishOAuthLogin.mockResolvedValue({ + data: { + message: 'Success', + returnTo: `${window.location.origin}/dashboard?tab=billing#top`, + }, + error: null, + }); + window.sessionStorage.setItem('seamless:oauth:provider', 'mock'); + (useSearchParams as jest.Mock).mockReturnValue([ + new URLSearchParams('code=abc&state=xyz'), + ]); + + render(); + + await waitFor(() => + expect(navigate).toHaveBeenCalledWith('/dashboard?tab=billing#top') + ); + }); + + // These views route with react-router, which cannot leave the application. An adopter + // that wants to is expected to read returnTo off the client result and navigate itself. + test('falls back home when the returnTo is on another origin', async () => { + finishOAuthLogin.mockResolvedValue({ + data: { message: 'Success', returnTo: 'https://elsewhere.example/dashboard' }, + error: null, + }); + window.sessionStorage.setItem('seamless:oauth:provider', 'mock'); + (useSearchParams as jest.Mock).mockReturnValue([ + new URLSearchParams('code=abc&state=xyz'), + ]); + + render(); + + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/')); + }); + + // The auth server refuses these before signing the state, so this is defence in depth + // against a malformed or hostile response rather than the primary guard. A + // javascript: URL parses but has no origin, so the origin check rejects it too. + test.each(['//', 'http://', 'javascript:alert(1)'])( + 'falls back home for a %s returnTo', + async returnTo => { + finishOAuthLogin.mockResolvedValue({ + data: { message: 'Success', returnTo }, + error: null, + }); + window.sessionStorage.setItem('seamless:oauth:provider', 'mock'); + (useSearchParams as jest.Mock).mockReturnValue([ + new URLSearchParams('code=abc&state=xyz'), + ]); + + render(); + + await waitFor(() => expect(navigate).toHaveBeenCalledWith('/')); + } + ); + test('shows an error when the callback params are missing', async () => { (useSearchParams as jest.Mock).mockReturnValue([new URLSearchParams('')]);