fix(auth): preserve OAuth continuation through sign-in

This commit is contained in:
saltbo
2026-07-30 15:06:47 -04:00
parent 954aae5f9e
commit af6e8d4c81
6 changed files with 173 additions and 25 deletions
+11 -4
View File
@@ -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',
)
})
})
// ---------------------------------------------------------------------------
+11 -2
View File
@@ -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'))
}
+64
View File
@@ -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<string, string>()
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)
})
})
+69
View File
@@ -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')
)
}
+5 -5
View File
@@ -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')
})
})
+13 -14
View File
@@ -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() {
<h1 className="text-2xl font-bold">{siteName || DEFAULT_SITE_NAME}</h1>
<p className="text-muted-foreground">{t('auth.signInSubtitle')}</p>
</div>
<OAuthButtons showLastUsed />
<OAuthButtons
showLastUsed
callbackURL={callbackURL}
onSignIn={() => clearSignInRedirect(window.sessionStorage)}
/>
{showDivider && (
<div className="flex items-center gap-3">
<Separator className="flex-1" />