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('')]);