diff --git a/src/components/oauth-buttons.test.ts b/src/components/oauth-buttons.test.ts index d946a72b..601be6ac 100644 --- a/src/components/oauth-buttons.test.ts +++ b/src/components/oauth-buttons.test.ts @@ -91,17 +91,24 @@ describe('OAuthButtons — provider data contract', () => { }) // --------------------------------------------------------------------------- -// OAuth sign-in handler — callbackURL is always '/files' +// OAuth sign-in handler — callbackURL defaults to '/files' and can continue +// an authorization request supplied by the sign-in page. // --------------------------------------------------------------------------- -function buildOAuthCallbackUrl(): string { - return '/files' +function buildOAuthCallbackUrl(callbackURL = '/files'): string { + return callbackURL } describe('OAuthButtons — OAuth callback URL', () => { - it('OAuth sign-in callback URL is "/files"', () => { + it('defaults the OAuth sign-in callback URL to "/files"', () => { expect(buildOAuthCallbackUrl()).toBe('/files') }) + + it('uses the supplied authorization continuation', () => { + expect(buildOAuthCallbackUrl('/api/auth/oauth2/authorize?state=oauth-state')).toBe( + '/api/auth/oauth2/authorize?state=oauth-state', + ) + }) }) // --------------------------------------------------------------------------- diff --git a/src/components/oauth-buttons.tsx b/src/components/oauth-buttons.tsx index 07e1b3e3..8a95b328 100644 --- a/src/components/oauth-buttons.tsx +++ b/src/components/oauth-buttons.tsx @@ -10,7 +10,15 @@ export function useOAuthProviders() { return { providers: data?.auth.providers ?? [], isLoading } } -export function OAuthButtons({ showLastUsed = false }: { showLastUsed?: boolean }) { +export function OAuthButtons({ + showLastUsed = false, + callbackURL = '/files', + onSignIn, +}: { + showLastUsed?: boolean + callbackURL?: string + onSignIn?: () => void +}) { const { t } = useTranslation() const [error, setError] = useState('') const { providers, isLoading } = useOAuthProviders() @@ -20,7 +28,8 @@ export function OAuthButtons({ showLastUsed = false }: { showLastUsed?: boolean async function handleOAuth(providerId: string) { setError('') - const result = await authClient.signIn.social({ provider: providerId, callbackURL: '/files' }) + onSignIn?.() + const result = await authClient.signIn.social({ provider: providerId, callbackURL }) if (result.error) { setError(result.error.message ?? t('auth.signInFailed')) } diff --git a/src/lib/sign-in-redirect.test.ts b/src/lib/sign-in-redirect.test.ts new file mode 100644 index 00000000..cfbd9cc7 --- /dev/null +++ b/src/lib/sign-in-redirect.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest' +import { clearSignInRedirect, loadSignInRedirect, resolveSignInRedirect } from './sign-in-redirect' + +const ORIGIN = 'https://zpan.example' + +describe('resolveSignInRedirect', () => { + it('accepts an explicit same-origin redirect', () => { + expect(resolveSignInRedirect('?redirect=%2Fdevice%3Fuser_code%3DABCD', ORIGIN)).toBe('/device?user_code=ABCD') + }) + + it('rejects an explicit cross-origin redirect', () => { + expect(resolveSignInRedirect('?redirect=https%3A%2F%2Fevil.example%2Fcallback', ORIGIN)).toBeNull() + }) + + it('continues a signed OAuth authorization request without reserializing it', () => { + const query = + 'response_type=code&redirect_uri=https%3A%2F%2Fbroker.example%2Fcallback&scope=objects%3Aread+openid' + + '&state=oauth-state&client_id=client-1&code_challenge=challenge&code_challenge_method=S256' + + '&ba_param=client_id&ba_param=state&sig=abc%2Fdef%3D' + + expect(resolveSignInRedirect(`?${query}`, ORIGIN)).toBe(`/api/auth/oauth2/authorize?${query}`) + }) + + it('ignores an incomplete OAuth-looking query', () => { + expect(resolveSignInRedirect('?response_type=code&client_id=client-1', ORIGIN)).toBeNull() + }) + + it('returns null for an ordinary sign-in request', () => { + expect(resolveSignInRedirect('', ORIGIN)).toBeNull() + }) +}) + +describe('sign-in redirect session storage', () => { + it('stores and restores the exact OAuth continuation within the tab session', () => { + const values = new Map() + const storage = { + getItem: (key: string) => values.get(key) ?? null, + removeItem: (key: string) => values.delete(key), + setItem: (key: string, value: string) => values.set(key, value), + } + const query = + '?response_type=code&redirect_uri=https%3A%2F%2Fbroker.example%2Fcallback&state=oauth-state' + + '&client_id=client-1&code_challenge=challenge&code_challenge_method=S256&sig=abc%2Fdef%3D' + const expected = `/api/auth/oauth2/authorize?${query.slice(1)}` + + expect(loadSignInRedirect(query, ORIGIN, storage)).toBe(expected) + expect(loadSignInRedirect('', ORIGIN, storage)).toBe(expected) + + clearSignInRedirect(storage) + expect(loadSignInRedirect('', ORIGIN, storage)).toBeNull() + }) + + it('removes a stored cross-origin redirect', () => { + const values = new Map([['zpan.sign-in.redirect', 'https://evil.example/callback']]) + const storage = { + getItem: (key: string) => values.get(key) ?? null, + removeItem: (key: string) => values.delete(key), + setItem: (key: string, value: string) => values.set(key, value), + } + + expect(loadSignInRedirect('', ORIGIN, storage)).toBeNull() + expect(values.size).toBe(0) + }) +}) diff --git a/src/lib/sign-in-redirect.ts b/src/lib/sign-in-redirect.ts new file mode 100644 index 00000000..e2a5627c --- /dev/null +++ b/src/lib/sign-in-redirect.ts @@ -0,0 +1,69 @@ +const OAUTH_AUTHORIZE_PATH = '/api/auth/oauth2/authorize' +const SIGN_IN_REDIRECT_STORAGE_KEY = 'zpan.sign-in.redirect' + +interface SignInRedirectStorage { + getItem(key: string): string | null + removeItem(key: string): void + setItem(key: string, value: string): void +} + +export function loadSignInRedirect(search: string, origin: string, storage: SignInRedirectStorage): string | null { + const redirect = resolveSignInRedirect(search, origin) + if (redirect) { + storage.setItem(SIGN_IN_REDIRECT_STORAGE_KEY, redirect) + return redirect + } + + const stored = storage.getItem(SIGN_IN_REDIRECT_STORAGE_KEY) + if (!stored) return null + + try { + const parsed = new URL(stored, origin) + if (parsed.origin !== origin) { + storage.removeItem(SIGN_IN_REDIRECT_STORAGE_KEY) + return null + } + return parsed.pathname + parsed.search + parsed.hash + } catch { + storage.removeItem(SIGN_IN_REDIRECT_STORAGE_KEY) + return null + } +} + +export function clearSignInRedirect(storage: SignInRedirectStorage): void { + storage.removeItem(SIGN_IN_REDIRECT_STORAGE_KEY) +} + +export function resolveSignInRedirect(search: string, origin: string): string | null { + const rawSearch = search.startsWith('?') ? search.slice(1) : search + const params = new URLSearchParams(rawSearch) + const explicitRedirect = params.get('redirect') + + if (explicitRedirect) { + try { + const parsed = new URL(explicitRedirect, origin) + if (parsed.origin !== origin) return null + return parsed.pathname + parsed.search + parsed.hash + } catch { + return null + } + } + + if (!isOAuthAuthorizationContinuation(params)) return null + + // Better Auth signs the authorization query before redirecting an + // unauthenticated user here. Preserve the exact bytes and parameter order. + return `${OAUTH_AUTHORIZE_PATH}?${rawSearch}` +} + +function isOAuthAuthorizationContinuation(params: URLSearchParams): boolean { + return ( + params.get('response_type') === 'code' && + params.has('client_id') && + params.has('redirect_uri') && + params.has('state') && + params.has('code_challenge') && + params.get('code_challenge_method') === 'S256' && + params.has('sig') + ) +} diff --git a/src/routes/(auth)/sign-in.test.ts b/src/routes/(auth)/sign-in.test.ts index 7b33595c..38219182 100644 --- a/src/routes/(auth)/sign-in.test.ts +++ b/src/routes/(auth)/sign-in.test.ts @@ -85,14 +85,14 @@ describe('SignIn — sign-up link visibility', () => { }) // --------------------------------------------------------------------------- -// OAuth callback URL used for social sign-in +// Default callback URL used when no continuation is present // --------------------------------------------------------------------------- -const SIGN_IN_CALLBACK_URL = '/files' +const DEFAULT_SIGN_IN_CALLBACK_URL = '/files' -describe('SignIn — OAuth and form callback URL', () => { - it('callback URL for sign-in is "/files"', () => { - expect(SIGN_IN_CALLBACK_URL).toBe('/files') +describe('SignIn — default callback URL', () => { + it('uses "/files" for an ordinary sign-in', () => { + expect(DEFAULT_SIGN_IN_CALLBACK_URL).toBe('/files') }) }) diff --git a/src/routes/(auth)/sign-in.tsx b/src/routes/(auth)/sign-in.tsx index 378fa694..a2f52b7a 100644 --- a/src/routes/(auth)/sign-in.tsx +++ b/src/routes/(auth)/sign-in.tsx @@ -12,6 +12,7 @@ import { Separator } from '@/components/ui/separator' import { useSiteConfig } from '@/hooks/use-site-config' import { authClient, signIn } from '@/lib/auth-client' import { isCredentialLoginMethod } from '@/lib/last-login-method' +import { clearSignInRedirect, loadSignInRedirect } from '@/lib/sign-in-redirect' export const Route = createFileRoute('/(auth)/sign-in')({ component: SignIn, @@ -20,17 +21,10 @@ export const Route = createFileRoute('/(auth)/sign-in')({ function SignIn() { const { t } = useTranslation() const navigate = useNavigate() - const redirectTo: string | null = (() => { - const raw = new URLSearchParams(window.location.search).get('redirect') - if (!raw) return null - try { - const parsed = new URL(raw, window.location.origin) - if (parsed.origin !== window.location.origin) return null - return parsed.pathname + parsed.search + parsed.hash - } catch { - return null - } - })() + const [redirectTo] = useState(() => + loadSignInRedirect(window.location.search, window.location.origin, window.sessionStorage), + ) + const callbackURL = redirectTo ?? '/files' const { data: siteConfig } = useSiteConfig() const authSignupMode = siteConfig?.auth.signupMode const captcha = siteConfig?.auth.captcha @@ -53,8 +47,8 @@ function SignIn() { const isEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(identity) const fetchOptions = captcha?.enabled ? { headers: { 'x-captcha-response': captchaToken } } : undefined const result = isEmail - ? await signIn.email({ email: identity, password, callbackURL: '/files', fetchOptions }) - : await signIn.username({ username: identity, password, callbackURL: '/files', fetchOptions }) + ? await signIn.email({ email: identity, password, callbackURL, fetchOptions }) + : await signIn.username({ username: identity, password, callbackURL, fetchOptions }) if (result.error) { setError( @@ -65,6 +59,7 @@ function SignIn() { return } if (redirectTo) { + clearSignInRedirect(window.sessionStorage) window.location.href = redirectTo } else { navigate({ to: '/files' }) @@ -83,7 +78,11 @@ function SignIn() {

{siteName || DEFAULT_SITE_NAME}

{t('auth.signInSubtitle')}

- + clearSignInRedirect(window.sessionStorage)} + /> {showDivider && (