From 54006e72a4db90ced5e2ed6e7b4dfb309738edeb Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 22 Jun 2026 14:00:00 +0800 Subject: [PATCH] fix(web): ISSUE-62 gate security settings by capability Signed-off-by: dongmucat <1127093059@qq.com> --- web/e2e/settings-security-capability.spec.ts | 102 ++++++++++++++++ web/src/i18n/locales/en.json | 2 + web/src/i18n/locales/zh.json | 2 + web/src/pages/settings/security.test.ts | 61 ---------- web/src/pages/settings/security.test.tsx | 119 +++++++++++++++++++ web/src/pages/settings/security.tsx | 60 ++++++---- web/src/shared/components/user-menu.test.ts | 18 --- web/src/shared/components/user-menu.test.tsx | 108 +++++++++++++++++ web/src/shared/components/user-menu.tsx | 5 +- 9 files changed, 371 insertions(+), 106 deletions(-) create mode 100644 web/e2e/settings-security-capability.spec.ts delete mode 100644 web/src/pages/settings/security.test.ts create mode 100644 web/src/pages/settings/security.test.tsx delete mode 100644 web/src/shared/components/user-menu.test.ts create mode 100644 web/src/shared/components/user-menu.test.tsx diff --git a/web/e2e/settings-security-capability.spec.ts b/web/e2e/settings-security-capability.spec.ts new file mode 100644 index 00000000..30d94ea8 --- /dev/null +++ b/web/e2e/settings-security-capability.spec.ts @@ -0,0 +1,102 @@ +import { expect, test, type Page } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' + +interface MockSessionUser { + userId: string + displayName: string + email: string + avatarUrl: string + oauthProvider: string + canChangePassword: boolean + platformRoles: string[] +} + +function apiEnvelope(data: unknown) { + return { + code: 0, + msg: 'OK', + data, + timestamp: new Date().toISOString(), + requestId: 'e2e-security-capability', + } +} + +async function mockSession(page: Page, user: MockSessionUser) { + await page.route('**/api/v1/auth/me', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(apiEnvelope(user)), + }) + }) + + await page.route('**/api/web/me/namespaces', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(apiEnvelope([])), + }) + }) + + await page.route('**/api/web/notifications/unread-count', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(apiEnvelope({ count: 0 })), + }) + }) + + await page.route('**/api/web/notifications/sse', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'text/event-stream', + body: ': ok\n\n', + }) + }) +} + +test.describe('Security Settings capability', () => { + test('shows the security menu entry and password form for local admin accounts', async ({ page }) => { + await setEnglishLocale(page) + await mockSession(page, { + userId: 'local-admin', + displayName: 'Local Admin', + email: 'local-admin@example.test', + avatarUrl: '', + oauthProvider: '', + canChangePassword: true, + platformRoles: ['USER', 'SUPER_ADMIN'], + }) + + await page.goto('/settings/security') + await expect(page.getByRole('heading', { name: 'Security Settings' })).toBeVisible() + await expect(page.getByLabel('Current Password')).toBeVisible() + await expect(page.getByLabel('New Password')).toBeVisible() + + await page.getByRole('button', { name: 'Local Admin' }).click() + await expect(page.getByRole('link', { name: 'Security Settings' })).toBeVisible() + }) + + test('hides the security menu entry and form when password changes are unavailable', async ({ page }) => { + await setEnglishLocale(page) + await mockSession(page, { + userId: 'oauth-only-user', + displayName: 'OAuth Only User', + email: 'oauth-only@example.test', + avatarUrl: '', + oauthProvider: 'github', + canChangePassword: false, + platformRoles: ['USER'], + }) + + await page.goto('/settings/security') + + await expect(page.getByRole('heading', { name: 'Security Settings' })).toBeVisible() + await expect(page.getByText('Password changes are unavailable for this account.')).toBeVisible() + await expect(page.getByLabel('Current Password')).toHaveCount(0) + await expect(page.getByRole('button', { name: 'Update Password' })).toHaveCount(0) + + await page.getByRole('button', { name: 'OAuth Only User' }).click() + await expect(page.getByRole('link', { name: 'Security Settings' })).toHaveCount(0) + }) +}) diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 56039107..6cf7fc71 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -743,6 +743,8 @@ "successTitle": "Password changed successfully", "successDescription": "Please sign in again with your new password.", "defaultError": "Failed to change password", + "unavailableTitle": "Password changes are unavailable for this account.", + "unavailableDescription": "This account signs in through an external identity provider or has no local password credential.", "submitting": "Submitting...", "submit": "Update Password" }, diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 1920b158..2086a5a4 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -743,6 +743,8 @@ "successTitle": "密码修改成功", "successDescription": "请使用新密码重新登录。", "defaultError": "修改密码失败", + "unavailableTitle": "此账号暂不可修改密码。", + "unavailableDescription": "此账号通过外部身份提供方登录，或尚未配置本地密码凭据。", "submitting": "提交中...", "submit": "更新密码" }, diff --git a/web/src/pages/settings/security.test.ts b/web/src/pages/settings/security.test.ts deleted file mode 100644 index 6c11ae40..00000000 --- a/web/src/pages/settings/security.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' - -vi.mock('@tanstack/react-router', () => ({ - useNavigate: () => vi.fn(), -})) - -vi.mock('@tanstack/react-query', () => ({ - useQueryClient: () => ({ setQueryData: vi.fn() }), -})) - -vi.mock('react-i18next', async () => { - const actual = await vi.importActual('react-i18next') - return { - ...actual, - useTranslation: () => ({ - t: (key: string) => key, - }), - } -}) - -vi.mock('@/api/client', () => ({ - ApiError: class ApiError extends Error { - status?: number - }, - authApi: { - changePassword: vi.fn(), - logout: vi.fn(), - }, -})) - -vi.mock('@/shared/lib/error-display', () => ({ - truncateErrorMessage: (v: string) => v, -})) - -vi.mock('@/shared/lib/toast', () => ({ - toast: { success: vi.fn(), error: vi.fn() }, -})) - -vi.mock('@/shared/ui/button', () => ({ - Button: ({ children }: { children: unknown }) => children, -})) - -vi.mock('@/shared/ui/card', () => ({ - Card: ({ children }: { children: unknown }) => children, - CardContent: ({ children }: { children: unknown }) => children, - CardDescription: ({ children }: { children: unknown }) => children, - CardHeader: ({ children }: { children: unknown }) => children, - CardTitle: ({ children }: { children: unknown }) => children, -})) - -vi.mock('@/shared/ui/input', () => ({ - Input: () => null, -})) - -import { SecuritySettingsPage } from './security' - -describe('SecuritySettingsPage', () => { - it('exports a named component function', () => { - expect(typeof SecuritySettingsPage).toBe('function') - }) -}) diff --git a/web/src/pages/settings/security.test.tsx b/web/src/pages/settings/security.test.tsx new file mode 100644 index 00000000..7f965912 --- /dev/null +++ b/web/src/pages/settings/security.test.tsx @@ -0,0 +1,119 @@ +import type { InputHTMLAttributes, ReactNode } from 'react' +import { renderToStaticMarkup } from 'react-dom/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const useAuthMock = vi.hoisted(() => vi.fn()) + +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => vi.fn(), +})) + +vi.mock('@tanstack/react-query', () => ({ + useQueryClient: () => ({ setQueryData: vi.fn() }), +})) + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next') + return { + ...actual, + useTranslation: () => ({ + t: (key: string) => key, + }), + } +}) + +vi.mock('@/api/client', () => ({ + ApiError: class ApiError extends Error { + status?: number + }, + authApi: { + changePassword: vi.fn(), + logout: vi.fn(), + }, +})) + +vi.mock('@/features/auth/use-auth', () => ({ + useAuth: useAuthMock, +})) + +vi.mock('@/shared/lib/error-display', () => ({ + truncateErrorMessage: (v: string) => v, +})) + +vi.mock('@/shared/lib/toast', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})) + +vi.mock('@/shared/ui/button', () => ({ + Button: ({ + children, + disabled, + type, + }: { + children: ReactNode + disabled?: boolean + type?: 'button' | 'submit' | 'reset' + }) => ( + + ), +})) + +vi.mock('@/shared/ui/card', () => ({ + Card: ({ children }: { children: ReactNode }) => children, + CardContent: ({ children }: { children: ReactNode }) => children, + CardDescription: ({ children }: { children: ReactNode }) => children, + CardHeader: ({ children }: { children: ReactNode }) => children, + CardTitle: ({ children }: { children: ReactNode }) => children, +})) + +vi.mock('@/shared/ui/input', () => ({ + Input: (props: InputHTMLAttributes) => , +})) + +import { SecuritySettingsPage } from './security' + +beforeEach(() => { + useAuthMock.mockReturnValue({ + user: { + userId: 'user-1', + displayName: 'Local User', + platformRoles: ['USER'], + canChangePassword: true, + }, + }) +}) + +describe('SecuritySettingsPage', () => { + it('exports a named component function', () => { + expect(typeof SecuritySettingsPage).toBe('function') + }) + + it('renders the password form when password changes are allowed', () => { + const html = renderToStaticMarkup() + + expect(html).toContain('security.currentPassword') + expect(html).toContain('security.newPassword') + expect(html).toContain('security.submit') + }) + + it('renders a read-only unavailable state when password changes are not allowed', () => { + useAuthMock.mockReturnValue({ + user: { + userId: 'oauth-user', + displayName: 'OAuth User', + oauthProvider: 'github', + platformRoles: ['USER'], + canChangePassword: false, + }, + }) + + const html = renderToStaticMarkup() + + expect(html).toContain('security.unavailableTitle') + expect(html).toContain('security.unavailableDescription') + expect(html).not.toContain('security.currentPassword') + expect(html).not.toContain('security.submit') + }) +}) diff --git a/web/src/pages/settings/security.tsx b/web/src/pages/settings/security.tsx index d9285910..37e85d56 100644 --- a/web/src/pages/settings/security.tsx +++ b/web/src/pages/settings/security.tsx @@ -3,6 +3,7 @@ import { useNavigate } from '@tanstack/react-router' import { useQueryClient } from '@tanstack/react-query' import { useTranslation } from 'react-i18next' import { ApiError, authApi } from '@/api/client' +import { useAuth } from '@/features/auth/use-auth' import { clearSessionScopedQueries } from '@/features/notification/notification-session' import { truncateErrorMessage } from '@/shared/lib/error-display' import { toast } from '@/shared/lib/toast' @@ -19,10 +20,12 @@ export function SecuritySettingsPage() { const { t } = useTranslation() const navigate = useNavigate() const queryClient = useQueryClient() + const { user } = useAuth() const [currentPassword, setCurrentPassword] = useState('') const [newPassword, setNewPassword] = useState('') const [errorMessage, setErrorMessage] = useState('') const [isSubmitting, setIsSubmitting] = useState(false) + const passwordChangeUnavailable = user?.canChangePassword === false /** * Submits the password change request and clears local auth state afterward, @@ -78,32 +81,39 @@ export function SecuritySettingsPage() { {t('security.subtitle')} -
-
- - setCurrentPassword(event.target.value)} - /> + {passwordChangeUnavailable ? ( +
+

{t('security.unavailableTitle')}

+

{t('security.unavailableDescription')}

-
- - setNewPassword(event.target.value)} - /> -
- {errorMessage ?

{errorMessage}

: null} - - + ) : ( +
+
+ + setCurrentPassword(event.target.value)} + /> +
+
+ + setNewPassword(event.target.value)} + /> +
+ {errorMessage ?

{errorMessage}

: null} + +
+ )}
diff --git a/web/src/shared/components/user-menu.test.ts b/web/src/shared/components/user-menu.test.ts deleted file mode 100644 index a1877caf..00000000 --- a/web/src/shared/components/user-menu.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { describe, expect, it } from 'vitest' -import * as mod from './user-menu' - -/** - * UserMenu is a React component that renders a hover/click dropdown menu with - * role-based navigation links (dashboard, reviews, admin, etc.) and logout. - * Internal helpers (hasRole, closeMenu, handleMouseEnter/Leave) and the - * menuItemClassName constant are scoped inside the component function. - * There are no exported pure helpers or constants to test here. - * - * We verify the module shape so downstream consumers break fast - * if the export contract changes. - */ -describe('user-menu module exports', () => { - it('exports the UserMenu component', () => { - expect(mod.UserMenu).toBeTypeOf('function') - }) -}) diff --git a/web/src/shared/components/user-menu.test.tsx b/web/src/shared/components/user-menu.test.tsx new file mode 100644 index 00000000..3487bd8b --- /dev/null +++ b/web/src/shared/components/user-menu.test.tsx @@ -0,0 +1,108 @@ +import type { ReactNode } from 'react' +import { renderToStaticMarkup } from 'react-dom/server' +import { describe, expect, it, vi } from 'vitest' +import * as mod from './user-menu' +import { UserMenu } from './user-menu' + +vi.mock('react', async () => { + const actual = await vi.importActual('react') + return { + ...actual, + useState: (initialValue: unknown) => [ + typeof initialValue === 'boolean' ? true : initialValue, + vi.fn(), + ], + } +}) + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next') + return { + ...actual, + useTranslation: () => ({ + t: (key: string) => key, + }), + } +}) + +vi.mock('@tanstack/react-router', () => ({ + Link: ({ + children, + className, + onClick, + to, + }: { + children: ReactNode + className?: string + onClick?: () => void + to: string + }) => ( + { + event.preventDefault() + onClick?.() + }} + > + {children} + + ), +})) + +vi.mock('@tanstack/react-query', () => ({ + useQueryClient: () => ({ + setQueryData: vi.fn(), + }), +})) + +vi.mock('@/api/client', () => ({ + authApi: { + logout: vi.fn(), + }, +})) + +vi.mock('@/shared/hooks/use-namespace-queries', () => ({ + useMyNamespaces: () => ({ data: [] }), +})) + +/** + * UserMenu is a React component that renders a hover/click dropdown menu with + * role-based navigation links (dashboard, reviews, admin, etc.) and logout. + */ +describe('user-menu module exports', () => { + it('exports the UserMenu component', () => { + expect(mod.UserMenu).toBeTypeOf('function') + }) +}) + +describe('UserMenu security settings visibility', () => { + it('shows security settings when password changes are allowed, independent of OAuth provider', () => { + const html = renderToStaticMarkup( + , + ) + + expect(html).toContain('user.menu.security') + }) + + it('hides security settings when password changes are not allowed, even for a local-looking account', () => { + const html = renderToStaticMarkup( + , + ) + + expect(html).not.toContain('user.menu.security') + }) +}) diff --git a/web/src/shared/components/user-menu.tsx b/web/src/shared/components/user-menu.tsx index 1cb10a31..d3eb3d39 100644 --- a/web/src/shared/components/user-menu.tsx +++ b/web/src/shared/components/user-menu.tsx @@ -14,6 +14,7 @@ interface User { avatarUrl?: string platformRoles?: string[] oauthProvider?: string + canChangePassword?: boolean } interface UserMenuProps { @@ -37,7 +38,7 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) { const isAuditor = hasRole('AUDITOR') || hasRole('SUPER_ADMIN') const isSuperAdmin = hasRole('SUPER_ADMIN') const reviewCenterVisible = canAccessReviewCenter(user.platformRoles, myNamespaces) - const isLocalAccount = !user.oauthProvider + const canChangePassword = user.canChangePassword === true const open = isHovered || isClickOpen const clearCloseTimer = () => { @@ -200,7 +201,7 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) { {t('user.menu.notifications')} - {isLocalAccount ? ( + {canChangePassword ? ( {t('user.menu.security')}