diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 7a7e46d63..b4fb0cd16 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -485,23 +485,20 @@ Session 中存储以下字段: "code": 0, "msg": "获取成功", "data": { - "userId": 42, + "userId": "usr_42", "displayName": "zhangsan", "email": "zhangsan@company.com", "avatarUrl": "https://...", - "oauthProvider": "github", - "platformRoles": ["SKILL_ADMIN", "AUDITOR"], - "namespaces": [ - { "slug": "ai-team", "role": "ADMIN" }, - { "slug": "global", "role": "MEMBER" } - ] + "oauthProvider": "local", + "canChangePassword": true, + "platformRoles": ["SKILL_ADMIN", "AUDITOR"] }, "timestamp": "2026-03-12T06:00:00Z", "requestId": "req-123" } ``` -前端权限判定基于 `platformRoles` + `namespaces[].role`,后端通过 `role_permission` 表查询权限码。 +前端平台级权限判定基于 `platformRoles`;是否展示修改密码入口和表单基于后端返回的 `canChangePassword`。后端通过 `role_permission` 表查询权限码。 统一约束: - `/api/v1/auth/me`、`/api/v1/auth/providers` 等 JSON 响应必须统一使用 `code/msg/data/timestamp/requestId` 外层结构。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AuthController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AuthController.java index 1552f6f2a..4ba7b27ba 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AuthController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AuthController.java @@ -16,6 +16,7 @@ import com.iflytek.skillhub.dto.DirectLoginRequest; import com.iflytek.skillhub.dto.SessionBootstrapRequest; import com.iflytek.skillhub.auth.exception.AuthFlowException; +import com.iflytek.skillhub.service.AuthMeResponseAssembler; import com.iflytek.skillhub.service.AuthMethodCatalog; import com.iflytek.skillhub.service.DirectAuthService; import com.iflytek.skillhub.service.SessionBootstrapService; @@ -56,6 +57,7 @@ public class AuthController extends BaseApiController { private final UserRoleBindingRepository userRoleBindingRepository; private final PlatformSessionService platformSessionService; private final UserAccountRepository userAccountRepository; + private final AuthMeResponseAssembler authMeResponseAssembler; public AuthController(ApiResponseFactory responseFactory, AuthMethodCatalog authMethodCatalog, @@ -64,7 +66,8 @@ public AuthController(ApiResponseFactory responseFactory, AuthFailureThrottleService authFailureThrottleService, UserRoleBindingRepository userRoleBindingRepository, PlatformSessionService platformSessionService, - UserAccountRepository userAccountRepository) { + UserAccountRepository userAccountRepository, + AuthMeResponseAssembler authMeResponseAssembler) { super(responseFactory); this.authMethodCatalog = authMethodCatalog; this.sessionBootstrapService = sessionBootstrapService; @@ -73,6 +76,7 @@ public AuthController(ApiResponseFactory responseFactory, this.userRoleBindingRepository = userRoleBindingRepository; this.platformSessionService = platformSessionService; this.userAccountRepository = userAccountRepository; + this.authMeResponseAssembler = authMeResponseAssembler; } /** @@ -111,7 +115,7 @@ public ApiResponse me(@AuthenticationPrincipal PlatformPrincipal freshRoles); platformSessionService.establishSession(principal, request, false); } - return ok("response.success.read", AuthMeResponse.from(principal)); + return ok("response.success.read", authMeResponseAssembler.from(principal)); } /** @@ -146,7 +150,7 @@ public ApiResponse bootstrapSession(@Valid @RequestBody SessionB HttpServletRequest httpRequest) { return ok( "response.success.read", - AuthMeResponse.from(sessionBootstrapService.bootstrap(request.provider(), httpRequest)) + authMeResponseAssembler.from(sessionBootstrapService.bootstrap(request.provider(), httpRequest)) ); } @@ -178,7 +182,7 @@ public ApiResponse directLogin(@Valid @RequestBody DirectLoginRe authFailureThrottleService.resetIdentifier(category, request.username()); return ok( "response.success.read", - AuthMeResponse.from(principal) + authMeResponseAssembler.from(principal) ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java index 8442939df..17e54fbee 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/LocalAuthController.java @@ -17,6 +17,7 @@ import com.iflytek.skillhub.metrics.SkillHubMetrics; import com.iflytek.skillhub.ratelimit.RateLimit; import com.iflytek.skillhub.security.AuthFailureThrottleService; +import com.iflytek.skillhub.service.AuthMeResponseAssembler; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import org.springframework.http.HttpStatus; @@ -38,19 +39,22 @@ public class LocalAuthController extends BaseApiController { private final PlatformSessionService platformSessionService; private final AuthFailureThrottleService authFailureThrottleService; private final PasswordResetService passwordResetService; + private final AuthMeResponseAssembler authMeResponseAssembler; public LocalAuthController(ApiResponseFactory responseFactory, LocalAuthService localAuthService, SkillHubMetrics skillHubMetrics, PlatformSessionService platformSessionService, AuthFailureThrottleService authFailureThrottleService, - PasswordResetService passwordResetService) { + PasswordResetService passwordResetService, + AuthMeResponseAssembler authMeResponseAssembler) { super(responseFactory); this.localAuthService = localAuthService; this.skillHubMetrics = skillHubMetrics; this.platformSessionService = platformSessionService; this.authFailureThrottleService = authFailureThrottleService; this.passwordResetService = passwordResetService; + this.authMeResponseAssembler = authMeResponseAssembler; } @PostMapping("/register") @@ -60,7 +64,7 @@ public ApiResponse register(@Valid @RequestBody LocalRegisterReq PlatformPrincipal principal = localAuthService.register(request.username(), request.password(), request.email()); skillHubMetrics.incrementUserRegister(); platformSessionService.establishSession(principal, httpRequest); - return ok("response.success.created", AuthMeResponse.from(principal)); + return ok("response.success.created", authMeResponseAssembler.from(principal)); } @PostMapping("/login") @@ -84,7 +88,7 @@ public ApiResponse login(@Valid @RequestBody LocalLoginRequest r authFailureThrottleService.resetIdentifier("local", request.username()); skillHubMetrics.recordLocalLogin(true); platformSessionService.establishSession(principal, httpRequest); - return ok("response.success.read", AuthMeResponse.from(principal)); + return ok("response.success.read", authMeResponseAssembler.from(principal)); } @PostMapping("/change-password") diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuthMeResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuthMeResponse.java index 470b2fdbe..d54478840 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuthMeResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/AuthMeResponse.java @@ -10,15 +10,17 @@ public record AuthMeResponse( String email, String avatarUrl, String oauthProvider, + boolean canChangePassword, Set platformRoles ) { - public static AuthMeResponse from(PlatformPrincipal principal) { + public static AuthMeResponse from(PlatformPrincipal principal, boolean canChangePassword) { return new AuthMeResponse( principal.userId(), principal.displayName(), principal.email() != null ? principal.email() : "", principal.avatarUrl() != null ? principal.avatarUrl() : "", principal.oauthProvider(), + canChangePassword, principal.platformRoles() ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMeResponseAssembler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMeResponseAssembler.java new file mode 100644 index 000000000..60b0066b1 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AuthMeResponseAssembler.java @@ -0,0 +1,27 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.local.LocalCredentialRepository; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.dto.AuthMeResponse; +import org.springframework.stereotype.Service; + +/** + * Builds the current-user API response with account capabilities derived from + * authoritative backend state. + */ +@Service +public class AuthMeResponseAssembler { + + private final LocalCredentialRepository localCredentialRepository; + + public AuthMeResponseAssembler(LocalCredentialRepository localCredentialRepository) { + this.localCredentialRepository = localCredentialRepository; + } + + public AuthMeResponse from(PlatformPrincipal principal) { + return AuthMeResponse.from( + principal, + localCredentialRepository.existsByUserId(principal.userId()) + ); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java index a25d31048..d79d368ad 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthControllerTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.local.LocalCredentialRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.user.UserAccount; @@ -64,6 +65,9 @@ class AuthControllerTest { @MockBean private UserRoleBindingRepository userRoleBindingRepository; + @MockBean + private LocalCredentialRepository localCredentialRepository; + @Test void meShouldReturnUnauthorizedForAnonymousRequest() throws Exception { mockMvc.perform(get("/api/v1/auth/me")) @@ -77,6 +81,7 @@ void meShouldReturnCurrentPrincipal() throws Exception { given(userAccountRepository.findById("user-42")) .willReturn(java.util.Optional.of(new UserAccount("user-42", "tester", "tester@example.com", "https://example.com/avatar.png"))); given(userRoleBindingRepository.findByUserId("user-42")).willReturn(List.of()); + given(localCredentialRepository.existsByUserId("user-42")).willReturn(false); PlatformPrincipal principal = new PlatformPrincipal( "user-42", @@ -102,6 +107,7 @@ void meShouldReturnCurrentPrincipal() throws Exception { .andExpect(jsonPath("$.data.userId").value("user-42")) .andExpect(jsonPath("$.data.displayName").value("tester")) .andExpect(jsonPath("$.data.oauthProvider").value("github")) + .andExpect(jsonPath("$.data.canChangePassword").value(false)) .andExpect(jsonPath("$.data.platformRoles[0]").value("USER")) .andExpect(jsonPath("$.timestamp").isNotEmpty()) .andExpect(jsonPath("$.requestId").isNotEmpty()); @@ -115,6 +121,7 @@ void meShouldRefreshSessionWhenDisplayNameChanges() throws Exception { var user = new UserAccount("user-42", "UpdatedName", "tester@example.com", "https://example.com/avatar.png"); given(userAccountRepository.findById("user-42")).willReturn(java.util.Optional.of(user)); given(userRoleBindingRepository.findByUserId("user-42")).willReturn(List.of()); + given(localCredentialRepository.existsByUserId("user-42")).willReturn(true); PlatformPrincipal principal = new PlatformPrincipal( "user-42", @@ -134,7 +141,8 @@ void meShouldRefreshSessionWhenDisplayNameChanges() throws Exception { mockMvc.perform(get("/api/v1/auth/me").with(authentication(auth))) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.displayName").value("UpdatedName")); // should return DB value + .andExpect(jsonPath("$.data.displayName").value("UpdatedName")) // should return DB value + .andExpect(jsonPath("$.data.canChangePassword").value(true)); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DirectAuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DirectAuthControllerTest.java index 8878e286e..97db51d47 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DirectAuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/DirectAuthControllerTest.java @@ -7,6 +7,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import com.iflytek.skillhub.auth.local.LocalCredentialRepository; import com.iflytek.skillhub.auth.local.LocalAuthService; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; @@ -52,6 +53,9 @@ class DirectAuthControllerTest { @MockBean private UserRoleBindingRepository userRoleBindingRepository; + @MockBean + private LocalCredentialRepository localCredentialRepository; + @Test void directLoginShouldAuthenticateViaConfiguredProvider() throws Exception { PlatformPrincipal principal = new PlatformPrincipal( @@ -67,6 +71,7 @@ void directLoginShouldAuthenticateViaConfiguredProvider() throws Exception { given(userAccountRepository.findById("usr_direct_1")) .willReturn(java.util.Optional.of(new UserAccount("usr_direct_1", "direct-user", null, null))); given(userRoleBindingRepository.findByUserId("usr_direct_1")).willReturn(List.of()); + given(localCredentialRepository.existsByUserId("usr_direct_1")).willReturn(true); MockHttpSession session = (MockHttpSession) mockMvc.perform(post("/api/v1/auth/direct/login") .with(csrf()) @@ -77,6 +82,7 @@ void directLoginShouldAuthenticateViaConfiguredProvider() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.userId").value("usr_direct_1")) + .andExpect(jsonPath("$.data.canChangePassword").value(true)) .andReturn() .getRequest() .getSession(false); @@ -84,7 +90,8 @@ void directLoginShouldAuthenticateViaConfiguredProvider() throws Exception { mockMvc.perform(get("/api/v1/auth/me").session(session)) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.userId").value("usr_direct_1")); + .andExpect(jsonPath("$.data.userId").value("usr_direct_1")) + .andExpect(jsonPath("$.data.canChangePassword").value(true)); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java index 440fe4bf9..2425acde0 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java @@ -12,6 +12,7 @@ import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.local.LocalAuthService; +import com.iflytek.skillhub.auth.local.LocalCredentialRepository; import com.iflytek.skillhub.auth.local.PasswordResetService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -55,6 +56,9 @@ class LocalAuthControllerTest { @MockBean private PasswordResetService passwordResetService; + @MockBean + private LocalCredentialRepository localCredentialRepository; + @Test void login_returnsCurrentUserEnvelope() throws Exception { PlatformPrincipal principal = new PlatformPrincipal( @@ -66,6 +70,7 @@ void login_returnsCurrentUserEnvelope() throws Exception { Set.of("SUPER_ADMIN") ); given(localAuthService.login("alice", "Abcd123!")).willReturn(principal); + given(localCredentialRepository.existsByUserId("usr_1")).willReturn(true); mockMvc.perform(post("/api/v1/auth/local/login") .with(csrf()) @@ -76,7 +81,8 @@ void login_returnsCurrentUserEnvelope() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.userId").value("usr_1")) - .andExpect(jsonPath("$.data.oauthProvider").value("local")); + .andExpect(jsonPath("$.data.oauthProvider").value("local")) + .andExpect(jsonPath("$.data.canChangePassword").value(true)); verify(skillHubMetrics).recordLocalLogin(true); verify(skillHubMetrics, never()).recordLocalLogin(false); verify(authFailureThrottleService).resetIdentifier("local", "alice"); @@ -93,6 +99,7 @@ void register_returnsCreatedEnvelope() throws Exception { Set.of() ); given(localAuthService.register("bob", "Abcd123!", "bob@example.com")).willReturn(principal); + given(localCredentialRepository.existsByUserId("usr_2")).willReturn(true); mockMvc.perform(post("/api/v1/auth/local/register") .with(csrf()) @@ -102,7 +109,8 @@ void register_returnsCreatedEnvelope() throws Exception { """)) .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.displayName").value("bob")); + .andExpect(jsonPath("$.data.displayName").value("bob")) + .andExpect(jsonPath("$.data.canChangePassword").value(true)); verify(skillHubMetrics).incrementUserRegister(); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SessionBootstrapControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SessionBootstrapControllerTest.java index b842efbc7..36fd80bf2 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SessionBootstrapControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SessionBootstrapControllerTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.bootstrap.PassiveSessionAuthenticator; +import com.iflytek.skillhub.auth.local.LocalCredentialRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -48,12 +49,16 @@ class SessionBootstrapControllerTest { @MockBean private UserRoleBindingRepository userRoleBindingRepository; + @MockBean + private LocalCredentialRepository localCredentialRepository; + @Test void sessionBootstrapShouldEstablishSessionWhenAuthenticatorSucceeds() throws Exception { given(namespaceMemberRepository.findByUserId("sso-user-1")).willReturn(List.of()); given(userAccountRepository.findById("sso-user-1")) .willReturn(Optional.of(new UserAccount("sso-user-1", "Private SSO User", null, null))); given(userRoleBindingRepository.findByUserId("sso-user-1")).willReturn(List.of()); + given(localCredentialRepository.existsByUserId("sso-user-1")).willReturn(false); MockHttpSession session = (MockHttpSession) mockMvc.perform(post("/api/v1/auth/session/bootstrap") .with(csrf()) @@ -65,6 +70,7 @@ void sessionBootstrapShouldEstablishSessionWhenAuthenticatorSucceeds() throws Ex .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.userId").value("sso-user-1")) .andExpect(jsonPath("$.data.displayName").value("Private SSO User")) + .andExpect(jsonPath("$.data.canChangePassword").value(false)) .andReturn() .getRequest() .getSession(false); @@ -73,7 +79,8 @@ void sessionBootstrapShouldEstablishSessionWhenAuthenticatorSucceeds() throws Ex .andExpect(status().isOk()) .andExpect(jsonPath("$.code").value(0)) .andExpect(jsonPath("$.data.userId").value("sso-user-1")) - .andExpect(jsonPath("$.data.oauthProvider").value("private-sso")); + .andExpect(jsonPath("$.data.oauthProvider").value("private-sso")) + .andExpect(jsonPath("$.data.canChangePassword").value(false)); } @Test diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalCredentialRepository.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalCredentialRepository.java index 8346b9c23..a80d44acd 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalCredentialRepository.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalCredentialRepository.java @@ -15,4 +15,6 @@ public interface LocalCredentialRepository extends JpaRepository findByUserId(String userId); boolean existsByUsernameIgnoreCase(String username); + + boolean existsByUserId(String userId); } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java index 6b9f43446..b6eaf5afc 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java @@ -230,6 +230,20 @@ void login_withoutExplicitRoles_defaultsToUser() { assertThat(principal.platformRoles()).containsExactly("USER"); } + @Test + void changePassword_withoutLocalCredential_rejectsRequest() { + given(credentialRepository.findByUserId("oauth-only")).willReturn(Optional.empty()); + + assertThatThrownBy(() -> service.changePassword("oauth-only", "old", "Newpass123!")) + .isInstanceOf(AuthFlowException.class) + .hasMessageContaining("error.auth.local.notEnabled") + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + + verify(passwordEncoder, never()).matches(any(), any()); + verify(credentialRepository, never()).save(any(LocalCredential.class)); + } + @Test void register_rejectsInvalidEmailFormat() { given(credentialRepository.existsByUsernameIgnoreCase("alice")).willReturn(false); diff --git a/web/e2e/settings-pages.spec.ts b/web/e2e/settings-pages.spec.ts index de2abd6ec..38f287606 100644 --- a/web/e2e/settings-pages.spec.ts +++ b/web/e2e/settings-pages.spec.ts @@ -1,11 +1,13 @@ import { expect, test } from '@playwright/test' import { setEnglishLocale } from './helpers/auth-fixtures' -import { registerSession } from './helpers/session' +import { createFreshSession } from './helpers/session' test.describe('Settings Pages (Real API)', () => { + test.use({ baseURL: 'http://127.0.0.1:3000' }) + test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) - await registerSession(page, testInfo) + await createFreshSession(page, testInfo) }) test('opens profile settings page', async ({ page }) => { diff --git a/web/e2e/settings-security-capability.spec.ts b/web/e2e/settings-security-capability.spec.ts new file mode 100644 index 000000000..b2d45910c --- /dev/null +++ b/web/e2e/settings-security-capability.spec.ts @@ -0,0 +1,68 @@ +import { expect, test, type Page } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' +import { csrfHeaders } from './helpers/csrf' +import { loginWithCredentials } from './helpers/session' + +function getOptionalEnv(name: string): string | undefined { + const value = process.env[name]?.trim() + return value ? value : undefined +} + +function adminCredentials() { + return { + username: getOptionalEnv('E2E_ADMIN_USERNAME') ?? getOptionalEnv('BOOTSTRAP_ADMIN_USERNAME') ?? 'admin', + password: getOptionalEnv('E2E_ADMIN_PASSWORD') ?? getOptionalEnv('BOOTSTRAP_ADMIN_PASSWORD') ?? 'ChangeMe!2026', + } +} + +async function currentDisplayName(page: Page, headers?: Record): Promise { + const response = await page.context().request.get('/api/v1/auth/me', { headers }) + expect(response.ok()).toBeTruthy() + const body = await response.json() as { data: { displayName: string } } + return body.data.displayName +} + +test.describe('Security Settings capability (Real API)', () => { + test.use({ baseURL: 'http://127.0.0.1:3000' }) + + test('shows the security menu entry and password form for local admin accounts', async ({ page }, testInfo) => { + await setEnglishLocale(page) + await loginWithCredentials(page, adminCredentials(), testInfo) + const displayName = await currentDisplayName(page) + + 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: displayName }).click() + await expect(page.getByRole('link', { name: 'Security Settings' })).toBeVisible() + }) + + test('hides the security menu entry and rejects password changes without a local credential', async ({ page }) => { + await setEnglishLocale(page) + await page.context().setExtraHTTPHeaders({ + 'X-Mock-User-Id': 'local-user', + }) + const displayName = await currentDisplayName(page, { 'X-Mock-User-Id': 'local-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: displayName }).click() + await expect(page.getByRole('link', { name: 'Security Settings' })).toHaveCount(0) + + const response = await page.context().request.post('/api/v1/auth/local/change-password', { + data: { + currentPassword: 'Passw0rd!123', + newPassword: 'N3wPassw0rd!123', + }, + headers: await csrfHeaders(page, { 'X-Mock-User-Id': 'local-user' }), + }) + expect(response.status()).toBe(400) + }) +}) diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index a141fb207..68d7ceb9a 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -3846,6 +3846,7 @@ export interface components { email?: string; avatarUrl?: string; oauthProvider?: string; + canChangePassword?: boolean; platformRoles?: string[]; }; LocalRegisterRequest: { diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 56039107a..6cf7fc713 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 1920b1584..2086a5a46 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 6c11ae400..000000000 --- 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 000000000..9e2955a71 --- /dev/null +++ b/web/src/pages/settings/security.test.tsx @@ -0,0 +1,129 @@ +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') + }) + + it('defaults to the unavailable state while the user capability is unknown', () => { + useAuthMock.mockReturnValue({ user: null }) + + const html = renderToStaticMarkup() + + expect(html).toContain('security.unavailableTitle') + 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 d9285910c..3726ad7dd 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' @@ -10,6 +11,14 @@ import { Button } from '@/shared/ui/button' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' import { Input } from '@/shared/ui/input' +interface PasswordChangeCapabilityUser { + canChangePassword?: boolean +} + +function canUsePasswordChangeForm(user?: PasswordChangeCapabilityUser | null) { + return user?.canChangePassword === true +} + /** * Security settings page for password changes. After a successful change the * user is logged out so all existing authenticated state is re-established with @@ -19,10 +28,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 canChangePassword = canUsePasswordChangeForm(user) /** * Submits the password change request and clears local auth state afterward, @@ -32,6 +43,11 @@ export function SecuritySettingsPage() { event.preventDefault() setErrorMessage('') + if (!canChangePassword) { + setErrorMessage(t('security.unavailableTitle')) + return + } + if (!currentPassword.trim()) { setErrorMessage(t('security.currentPasswordRequired')) return @@ -78,32 +94,39 @@ export function SecuritySettingsPage() { {t('security.subtitle')} -
-
- - setCurrentPassword(event.target.value)} - /> -
-
- - setNewPassword(event.target.value)} - /> + {canChangePassword ? ( + +
+ + setCurrentPassword(event.target.value)} + /> +
+
+ + setNewPassword(event.target.value)} + /> +
+ {errorMessage ?

{errorMessage}

: null} + + + ) : ( +
+

{t('security.unavailableTitle')}

+

{t('security.unavailableDescription')}

- {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 a1877cafe..000000000 --- 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 000000000..3487bd8b0 --- /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 1cb10a318..d3eb3d394 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')}