From 1aa8d7bc850e151d52fe5c9d7deaafd0ed666298 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:08:49 +0800 Subject: [PATCH] fix(auth): guard registration OAuth hint and return paths Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- web/e2e/auth-entry.spec.ts | 15 +++++++++++++++ web/src/pages/register.test.tsx | 21 ++++++++++++++++++++- web/src/pages/register.tsx | 19 ++++++++++++------- web/src/shared/lib/auth-route.test.ts | 4 ++++ web/src/shared/lib/auth-route.ts | 4 ++++ 5 files changed, 55 insertions(+), 8 deletions(-) diff --git a/web/e2e/auth-entry.spec.ts b/web/e2e/auth-entry.spec.ts index 4fa2a7b6..9793b03a 100644 --- a/web/e2e/auth-entry.spec.ts +++ b/web/e2e/auth-entry.spec.ts @@ -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({ diff --git a/web/src/pages/register.test.tsx b/web/src/pages/register.test.tsx index e6b820ae..ddf36df2 100644 --- a/web/src/pages/register.test.tsx +++ b/web/src/pages/register.test.tsx @@ -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: () => OAuth buttons, +})) + +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() + + expect(html).toContain('register.oauthHint') + expect(html).toContain('OAuth buttons') + authMethodsFixture.methods = [] }) }) diff --git a/web/src/pages/register.tsx b/web/src/pages/register.tsx index 18fc234e..cc46649d 100644 --- a/web/src/pages/register.tsx +++ b/web/src/pages/register.tsx @@ -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(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() {

-
-

- {t('register.oauthHint')} -

- -
+ {authMethodsLoading || hasExternalMethods ? ( +
+

+ {t('register.oauthHint')} +

+ +
+ ) : null} ) diff --git a/web/src/shared/lib/auth-route.test.ts b/web/src/shared/lib/auth-route.test.ts index 695e6dff..8444d78d 100644 --- a/web/src/shared/lib/auth-route.test.ts +++ b/web/src/shared/lib/auth-route.test.ts @@ -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('/') }) diff --git a/web/src/shared/lib/auth-route.ts b/web/src/shared/lib/auth-route.ts index 492be432..f6c9fa2e 100644 --- a/web/src/shared/lib/auth-route.ts +++ b/web/src/shared/lib/auth-route.ts @@ -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) {