From 5a8b796d03510c513e3f47af53344177669d25a1 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 24 Sep 2026 09:48:56 +0800 Subject: [PATCH] fix(auth): defer organization discovery until backend contract Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- web/e2e/auth-entry.spec.ts | 52 +++++++++++- web/src/api/client.ts | 9 -- web/src/api/types.ts | 13 --- .../auth/enterprise-discovery-entry.test.tsx | 61 ------------- .../auth/enterprise-discovery-entry.tsx | 85 ------------------- .../features/auth/session-bootstrap-entry.tsx | 2 +- web/src/i18n/locales/en.json | 6 -- web/src/i18n/locales/ru.json | 6 -- web/src/i18n/locales/zh.json | 6 -- web/src/pages/login.test.tsx | 21 ++--- web/src/pages/login.tsx | 12 +-- 11 files changed, 62 insertions(+), 211 deletions(-) delete mode 100644 web/src/features/auth/enterprise-discovery-entry.test.tsx delete mode 100644 web/src/features/auth/enterprise-discovery-entry.tsx diff --git a/web/e2e/auth-entry.spec.ts b/web/e2e/auth-entry.spec.ts index aeeec995..4fa2a7b6 100644 --- a/web/e2e/auth-entry.spec.ts +++ b/web/e2e/auth-entry.spec.ts @@ -1,7 +1,7 @@ import { expect, test } from '@playwright/test' import { setEnglishLocale } from './helpers/auth-fixtures' -test.describe('Auth Entry (Real API)', () => { +test.describe('Auth Entry', () => { test.beforeEach(async ({ page }) => { await setEnglishLocale(page) }) @@ -18,4 +18,54 @@ test.describe('Auth Entry (Real API)', () => { await page.getByRole('link', { name: 'Sign up now' }).click() await expect(page).toHaveURL('/register?returnTo=%2Fdashboard%2Ftokens') }) + + test('shows configured OAuth methods without exposing unsupported organization discovery', 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: [ + { + id: 'github', + methodType: 'OAUTH_REDIRECT', + provider: 'github', + displayName: 'GitHub', + actionUrl: '/oauth2/authorization/github', + }, + { + id: 'enterprise-discovery', + methodType: 'ENTERPRISE_DISCOVERY', + provider: 'enterprise', + displayName: 'Organization login', + actionUrl: '/api/v1/auth/login-discovery', + }, + ], + }), + }) + }) + + await page.goto('/login') + + await expect(page.getByRole('button', { name: 'GitHub' })).toBeVisible() + await expect(page.getByRole('button', { name: 'Organization login' })).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({ + status: 200, + contentType: 'application/javascript', + body: 'window.__SKILLHUB_RUNTIME_CONFIG__ = { authSessionBootstrapEnabled: "true", authSessionBootstrapProvider: "proxy", authSessionBootstrapAuto: "false" }', + }) + }) + + await page.goto('/login') + await page.getByRole('button', { name: 'Organization login' }).click() + + await expect(page.getByRole('button', { name: 'Log in with work account' })).toBeVisible() + await expect(page.getByLabel('Password', { exact: true })).toBeHidden() + }) }) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 2aa8ae26..a724997e 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -31,7 +31,6 @@ import type { PagedResponse, ReportDisposition, AuthMethod, - EnterpriseLoginDiscovery, OAuthProvider, User, ManagedNamespace, @@ -363,14 +362,6 @@ export const authApi = { })) }, - async discoverEnterpriseLogin(identifier: string, returnTo?: string): Promise { - return fetchJson('/api/v1/auth/login-discovery', { - method: 'POST', - headers: await ensureCsrfHeaders({ 'Content-Type': 'application/json' }), - body: JSON.stringify({ identifier, returnTo }), - }) - }, - async localLogin(request: LocalLoginRequest): Promise { return fetchJson('/api/v1/auth/local/login', { method: 'POST', diff --git a/web/src/api/types.ts b/web/src/api/types.ts index df58d751..bbc37549 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -23,19 +23,6 @@ export interface AuthMethod { actionUrl: string } -export interface EnterpriseLoginDiscovery { - publicMethods: AuthMethod[] - organizations: Array<{ - slug: string - displayName: string - loginOptions: Array<{ - displayName: string - methodType: string - actionUrl: string - }> - }> -} - export type ApiToken = Omit & { id: number name: string diff --git a/web/src/features/auth/enterprise-discovery-entry.test.tsx b/web/src/features/auth/enterprise-discovery-entry.test.tsx deleted file mode 100644 index fb917a4c..00000000 --- a/web/src/features/auth/enterprise-discovery-entry.test.tsx +++ /dev/null @@ -1,61 +0,0 @@ -/** @vitest-environment jsdom */ -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' -import { EnterpriseDiscoveryEntry } from './enterprise-discovery-entry' - -const discoverEnterpriseLogin = vi.hoisted(() => vi.fn()) - -vi.mock('@/api/client', () => ({ - authApi: { discoverEnterpriseLogin }, -})) - -function renderEntry() { - const queryClient = new QueryClient({ defaultOptions: { mutations: { retry: false } } }) - return render( - - - , - ) -} - -describe('EnterpriseDiscoveryEntry', () => { - afterEach(() => { - cleanup() - vi.clearAllMocks() - }) - - it('shows the organization identifier field without another expansion step', () => { - renderEntry() - - expect(screen.getByPlaceholderText('login.organizationIdentifier')).toBeTruthy() - }) - - it('uses server discovery and displays only returned organization login options', async () => { - discoverEnterpriseLogin.mockResolvedValue({ - publicMethods: [], - organizations: [{ - slug: 'team', - displayName: 'Team', - loginOptions: [{ displayName: 'Company OIDC', methodType: 'ENTERPRISE_REDIRECT', actionUrl: '/api/v1/auth/enterprise/test/start' }], - }], - }) - renderEntry() - - fireEvent.change(screen.getByPlaceholderText('login.organizationIdentifier'), { target: { value: 'team' } }) - fireEvent.click(screen.getByRole('button', { name: 'login.organizationContinue' })) - - await waitFor(() => expect(screen.getByText('Company OIDC')).toBeTruthy()) - expect(discoverEnterpriseLogin).toHaveBeenCalledWith('team', '/dashboard') - }) - - it('shows a neutral result when the server has no available organization method', async () => { - discoverEnterpriseLogin.mockResolvedValue({ publicMethods: [], organizations: [] }) - renderEntry() - - fireEvent.change(screen.getByPlaceholderText('login.organizationIdentifier'), { target: { value: 'unknown' } }) - fireEvent.click(screen.getByRole('button', { name: 'login.organizationContinue' })) - - await waitFor(() => expect(screen.getByText('login.organizationNotFound')).toBeTruthy()) - }) -}) diff --git a/web/src/features/auth/enterprise-discovery-entry.tsx b/web/src/features/auth/enterprise-discovery-entry.tsx deleted file mode 100644 index 7f033ed1..00000000 --- a/web/src/features/auth/enterprise-discovery-entry.tsx +++ /dev/null @@ -1,85 +0,0 @@ -import { useState } from 'react' -import { useMutation } from '@tanstack/react-query' -import { useTranslation } from 'react-i18next' -import { ArrowRight, Building2 } from 'lucide-react' -import { authApi } from '@/api/client' -import { withBasePath } from '@/shared/lib/base-path' -import { Button } from '@/shared/ui/button' -import { Input } from '@/shared/ui/input' - -interface EnterpriseDiscoveryEntryProps { - returnTo: string -} - -/** The server resolves the organization and returns only its available login connections. */ -export function EnterpriseDiscoveryEntry({ returnTo }: EnterpriseDiscoveryEntryProps) { - const { t } = useTranslation() - const [identifier, setIdentifier] = useState('') - const discovery = useMutation({ - mutationFn: (value: string) => authApi.discoverEnterpriseLogin(value, returnTo), - meta: { skipGlobalErrorHandler: true }, - }) - - return ( -
-
{ - event.preventDefault() - const value = identifier.trim() - if (value) discovery.mutate(value) - }} - > -
- -
-
-
-

{t('login.organizationLoginHint')}

- -
- - {discovery.isError ?

{discovery.error.message}

: null} - {discovery.data && discovery.data.organizations.length === 0 ? ( -

{t('login.organizationNotFound')}

- ) : null} - {discovery.data?.organizations.map((organization) => ( -
-

{organization.displayName}

- {organization.loginOptions.map((option) => ( - - ))} -
- ))} -
- ) -} diff --git a/web/src/features/auth/session-bootstrap-entry.tsx b/web/src/features/auth/session-bootstrap-entry.tsx index 6d6a186d..bcc0054f 100644 --- a/web/src/features/auth/session-bootstrap-entry.tsx +++ b/web/src/features/auth/session-bootstrap-entry.tsx @@ -52,7 +52,7 @@ export function SessionBootstrapEntry({ onAuthenticated, methodDisplayName, comp return (
- {discoveryMethod ? ( - - ) : null} - {bootstrapConfig.enabled ? ( -