fix(auth): guard registration OAuth hint and return paths

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-24 10:08:49 +08:00
parent 5a8b796d03
commit 1aa8d7bc85
5 changed files with 55 additions and 8 deletions

View file

@ -53,6 +53,21 @@ test.describe('Auth Entry', () => {
await expect(page.getByRole('button', { name: 'Organization login' })).toHaveCount(0)
})
test('keeps registration usable without configured OAuth methods', async ({ page }) => {
await page.route('**/api/v1/auth/methods*', async (route) => {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ code: 0, msg: 'ok', data: [] }),
})
})
await page.goto('/register')
await expect(page.getByRole('button', { name: 'Register & Login' })).toBeVisible()
await expect(page.getByText('Sign in directly with your existing OAuth account')).toHaveCount(0)
})
test('keeps configured session bootstrap available in the organization view', async ({ page }) => {
await page.route('**/runtime-config.js', async (route) => {
await route.fulfill({

View file

@ -1,5 +1,10 @@
import { describe, expect, it, vi } from 'vitest'
const authMethodsFixture = vi.hoisted(() => ({
methods: [] as Array<{ id: string, methodType: string }>,
isLoading: false,
}))
vi.mock('@tanstack/react-router', () => ({
Link: ({ children }: { children: unknown }) => children,
useNavigate: () => vi.fn(),
@ -21,7 +26,11 @@ vi.mock('@/features/auth/auth-shell', () => ({
}))
vi.mock('@/features/auth/login-button', () => ({
LoginButton: () => null,
AuthMethodButtonList: () => <span>OAuth buttons</span>,
}))
vi.mock('@/features/auth/use-auth-methods', () => ({
useAuthMethods: () => ({ data: authMethodsFixture.methods, isLoading: authMethodsFixture.isLoading }),
}))
vi.mock('@/features/auth/use-local-auth', () => ({
@ -69,5 +78,15 @@ describe('RegisterPage', () => {
expect(html).toContain('register.title')
expect(html).toContain('register.subtitle')
expect(html).toContain('register.submit')
expect(html).not.toContain('register.oauthHint')
})
it('shows OAuth entry only when providers are advertised', () => {
authMethodsFixture.methods = [{ id: 'github', methodType: 'OAUTH_REDIRECT' }]
const html = renderToStaticMarkup(<RegisterPage />)
expect(html).toContain('register.oauthHint')
expect(html).toContain('OAuth buttons')
authMethodsFixture.methods = []
})
})

View file

@ -3,7 +3,8 @@ import { useState } from 'react'
import { useTranslation } from 'react-i18next'
import { ApiError } from '@/api/client'
import { AuthShell } from '@/features/auth/auth-shell'
import { LoginButton } from '@/features/auth/login-button'
import { AuthMethodButtonList } from '@/features/auth/login-button'
import { useAuthMethods } from '@/features/auth/use-auth-methods'
import { useLocalRegister } from '@/features/auth/use-local-auth'
import { Button } from '@/shared/ui/button'
import { Input } from '@/shared/ui/input'
@ -62,6 +63,8 @@ export function RegisterPage() {
const [formError, setFormError] = useState<string | null>(null)
const returnTo = resolveAuthReturnTo(search.returnTo)
const { data: authMethods, isLoading: authMethodsLoading } = useAuthMethods(returnTo)
const hasExternalMethods = authMethods?.some((method) => method.methodType === 'OAUTH_REDIRECT')
function validateUsername(value: string) {
const trimmed = value.trim()
@ -262,12 +265,14 @@ export function RegisterPage() {
</p>
</form>
<div className="space-y-3 border-t border-border/70 pt-5">
<p className="text-sm text-muted-foreground">
{t('register.oauthHint')}
</p>
<LoginButton returnTo={returnTo} compact />
</div>
{authMethodsLoading || hasExternalMethods ? (
<div className="space-y-3 border-t border-border/70 pt-5">
<p className="text-sm text-muted-foreground">
{t('register.oauthHint')}
</p>
<AuthMethodButtonList methods={authMethods} isLoading={authMethodsLoading} compact />
</div>
) : null}
</div>
</AuthShell>
)

View file

@ -8,6 +8,10 @@ describe('auth-route', () => {
expect(resolveAuthReturnTo(undefined)).toBe('/')
expect(resolveAuthReturnTo('//example.com')).toBe('/')
expect(resolveAuthReturnTo('/\\example.com')).toBe('/')
expect(resolveAuthReturnTo('/\n/evil.example')).toBe('/')
expect(resolveAuthReturnTo('/\t/evil.example')).toBe('/')
expect(resolveAuthReturnTo('/\r/evil.example')).toBe('/')
expect(resolveAuthReturnTo('/\u007fevil.example')).toBe('/')
expect(resolveAuthReturnTo('https://example.com')).toBe('/')
})

View file

@ -15,6 +15,10 @@ export function isSafeAuthReturnTo(value: unknown): value is string {
&& value.startsWith('/')
&& !value.startsWith('//')
&& !value.includes('\\')
&& !Array.from(value).some((character) => {
const code = character.charCodeAt(0)
return code < 32 || code === 127
})
}
export function resolveAuthReturnTo(value: unknown) {