Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/lucky-swans-arrive.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 6 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion src/AuthProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import {
CurrentUserResult,
FinishOAuthLoginInput,
FinishOAuthLoginResult,
LoginStartResult,
MessageResult,
OAuthProvidersResult,
Expand Down Expand Up @@ -55,7 +56,7 @@ export interface AuthContextType {
) => Promise<SeamlessAuthResult<StartOAuthLoginResult>>;
finishOAuthLogin: (
input: FinishOAuthLoginInput
) => Promise<SeamlessAuthResult<MessageResult>>;
) => Promise<SeamlessAuthResult<FinishOAuthLoginResult>>;
stepUpStatus: StepUpStatus | null;
updateCredential: (credential: Credential) => Promise<SeamlessAuthResult<Credential>>;
deleteCredential: (credentialId: string) => Promise<SeamlessAuthResult<MessageResult>>;
Expand Down
20 changes: 18 additions & 2 deletions src/client/createSeamlessAuthClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
LogoutScope as LogoutScopeShape,
MeResponse,
MessageResponse,
OAuthLoginSuccessResponse,
OAuthProvidersResponse,
OrganizationEnvelopeResponse,
OrganizationListResponse,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -265,7 +281,7 @@ export interface SeamlessAuthClient {
) => Promise<SeamlessAuthResult<StartOAuthLoginResult>>;
finishOAuthLogin: (
input: FinishOAuthLoginInput
) => Promise<SeamlessAuthResult<MessageResult>>;
) => Promise<SeamlessAuthResult<FinishOAuthLoginResult>>;
registerPasskey: (
input: PasskeyMetadata | RegisterPasskeyOptions
) => Promise<SeamlessAuthResult<PasskeyRegistrationData>>;
Expand Down Expand Up @@ -644,7 +660,7 @@ export const createSeamlessAuthClient = (
),

finishOAuthLogin: input =>
requestResult<MessageResult>(
requestResult<FinishOAuthLoginResult>(
fetchWithAuth(`/oauth/${encodeURIComponent(input.providerId)}/callback`, {
method: 'POST',
body: JSON.stringify({
Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
CreateOrganizationInput,
CurrentUserResult,
FinishOAuthLoginInput,
FinishOAuthLoginResult,
LoginInput,
LoginMethod,
LoginStartResult,
Expand Down Expand Up @@ -92,6 +93,7 @@ export type {
CreateOrganizationInput,
CurrentUserResult,
FinishOAuthLoginInput,
FinishOAuthLoginResult,
LoginInput,
LoginMethod,
LoginStartResult,
Expand Down
3 changes: 2 additions & 1 deletion src/session/createAuthSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
createSeamlessAuthClient,
CurrentUserResult,
FinishOAuthLoginInput,
FinishOAuthLoginResult,
LoginStartResult,
MessageResult,
OAuthProvidersResult,
Expand Down Expand Up @@ -59,7 +60,7 @@ export interface AuthSessionActions {
) => Promise<SeamlessAuthResult<StartOAuthLoginResult>>;
finishOAuthLogin: (
input: FinishOAuthLoginInput
) => Promise<SeamlessAuthResult<MessageResult>>;
) => Promise<SeamlessAuthResult<FinishOAuthLoginResult>>;
refreshStepUpStatus: () => Promise<SeamlessAuthResult<StepUpStatus>>;
verifyStepUpWithPasskey: () => Promise<SeamlessAuthResult<StepUpStatus>>;
verifyStepUpWithPasskeyPrf: (
Expand Down
44 changes: 35 additions & 9 deletions src/views/OAuthCallback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<OAuthErrorCode, string> = {
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.',
Expand Down Expand Up @@ -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 (
Expand Down
60 changes: 60 additions & 0 deletions tests/OAuthCallback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<OAuthCallback />);

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(<OAuthCallback />);

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(<OAuthCallback />);

await waitFor(() => expect(navigate).toHaveBeenCalledWith('/'));
}
);

test('shows an error when the callback params are missing', async () => {
(useSearchParams as jest.Mock).mockReturnValue([new URLSearchParams('')]);

Expand Down
Loading