From f297fdfd099fe52de401d8881824c4fa4f15b6a5 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:38:27 +0800 Subject: [PATCH] fix(auth): verify initial grants and auth UI behavior Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/923-auth-system-settings.md | 4 +- .../design.md | 2 + .../initial-external-role-grants/spec.md | 4 + .../configure-auth-and-initial-roles/tasks.md | 2 +- .../InitialAuthSettingsInitializer.java | 3 + .../service/SystemAuthSettingsAppService.java | 3 + .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_ru.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../InitialAuthSettingsInitializerTest.java | 16 ++++ .../controller/AuthControllerTest.java | 10 ++ .../AuthRateLimitControllerTest.java | 10 ++ .../controller/DirectAuthControllerTest.java | 10 ++ .../SessionBootstrapControllerTest.java | 10 ++ .../SystemAuthSettingsPostgresTest.java | 94 +++++++++++++++++++ .../SystemAuthSettingsAppServiceTest.java | 38 +++++++- .../identity/IdentityBindingServiceTest.java | 41 ++++++++ .../InitialExternalRoleGrantServiceTest.java | 15 +++ .../LocalAuthSettingsServiceTest.java | 15 ++- .../auth/use-local-auth-capabilities.ts | 2 +- web/src/i18n/locales/en.json | 2 +- web/src/i18n/locales/ru.json | 2 +- web/src/i18n/locales/zh.json | 2 +- web/src/pages/admin/system-config.test.tsx | 51 ++++++++++ web/src/pages/admin/system-config.tsx | 12 ++- web/src/pages/reset-password.test.tsx | 12 ++- 26 files changed, 352 insertions(+), 11 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SystemAuthSettingsPostgresTest.java create mode 100644 web/src/pages/admin/system-config.test.tsx diff --git a/docs/923-auth-system-settings.md b/docs/923-auth-system-settings.md index 1e989d5e..b0d84914 100644 --- a/docs/923-auth-system-settings.md +++ b/docs/923-auth-system-settings.md @@ -17,7 +17,7 @@ 首次授权规则的 JSON 示例： ```json -[{"provider":"feishu","email":"admin@example.com","role":"SUPER_ADMIN"}] +[{"provider":"github","email":"admin@example.com","role":"SUPER_ADMIN"}] ``` 规则仅在第一次初始化且 `user_account` 为空时写入；初始化标记防止以后删除规则再重启时重复创建。正式环境请通过 Secret 提供 JSON，不要将实际邮箱写进公开的 values 文件。初始化以后，数据库和“系统配置”页面是权威来源。 @@ -26,4 +26,6 @@ 规则按身份来源代码和**已验证邮箱**匹配。只有外部身份通过现有准入策略、并且新账号首次创建为可用状态时，才给新账号授予选定平台角色；规则随后标为“已授权”，不会再次使用。拒绝或待审批的登录不会触发授权，规则不会绕过准入策略。已有账号请在“用户管理”中直接授权。相同邮箱的本地账号不会因此合并或获得角色。 +当前飞书和钉钉适配器把邮箱标记为未验证：它们的用户资料没有提供可证明用户控制该邮箱的信号。因此以 `feishu` 或 `dingtalk` 配置的邮箱规则不会触发自动授权。GitHub 的已验证主邮箱、提供 `email_verified=true` 的 OIDC 身份等可匹配。飞书或钉钉账号首次进入后，可由已有超级管理员在用户管理中授权；如果部署方只允许飞书或钉钉登录，必须先安排其他可用的管理员入口。不能为了触发规则而把未验证邮箱改成已验证。 + 后台可以新增规则、调整待匹配规则的角色、停用规则，并查看已使用规则的匹配身份和授权账号。只允许超级管理员操作；修改带版本号，避免两个管理员同时改动时后写覆盖前写。紧急恢复可通过受控数据库操作处理，并保留审计记录。 diff --git a/openspec/changes/configure-auth-and-initial-roles/design.md b/openspec/changes/configure-auth-and-initial-roles/design.md index 48769ca2..ceb9a2d7 100644 --- a/openspec/changes/configure-auth-and-initial-roles/design.md +++ b/openspec/changes/configure-auth-and-initial-roles/design.md @@ -33,6 +33,8 @@ 首版规则以 `provider_code` 和规范化的**已验证邮箱**匹配。未返回邮箱、邮箱未验证、来源不匹配或账号已存在都不授予角色。匹配成功后记录实际 `(provider_code, subject)` 和用户 ID；规则不能因邮箱回收而再次授予另一身份。后台为每条规则选择一个现有平台角色，不创建自定义角色或命名空间角色，也不把角色写死为 `SUPER_ADMIN`。规则消费后修改或停用不改动已写入 `user_role_binding` 的角色。 +当前飞书和钉钉适配器没有可证明邮箱已验证的资料，明确返回 `emailVerified=false`，因此这两个来源的邮箱规则首版不会被消费。不能为实现首次授权而将它们标记为已验证。部署方须使用已验证邮箱来源，或先保留其他超级管理员入口并在首次登录后人工授权；飞书、钉钉的可信身份依据需要单独设计。 + 现有公开 OAuth 账号不按邮箱自动合并：即使本地账号已经使用相同邮箱，首次进入的外部身份仍会创建独立账号；若命中规则，角色授予这个新外部账号，不授予本地账号。规则配置页须明确提示“同邮箱不代表同一 SkillHub 账号”，并在消费结果中展示实际获授角色的账号与外部来源，防止管理员误认授权对象。角色授予接入当前公开 OAuth 的账号创建事务，不改变统一身份核心已有的关联或准入决策。 建议独立表保存 `id`、`provider_code`、`normalized_email`、`role_id`、`status`（`ACTIVE`/`DISABLED`/`CONSUMED`）、`matched_subject`、`granted_user_id`、`granted_at`、`created_by`、`updated_by`、时间戳和并发版本。最多允许一条同一来源与规范化邮箱的 ACTIVE 规则；消费记录保留用于审计。首次绑定、角色写入和规则消费必须原子完成，并通过身份唯一约束及规则条件更新处理并发首次登录。若规则在登录时被停用，以事务中读取并确认的当前状态为准。 diff --git a/openspec/changes/configure-auth-and-initial-roles/specs/initial-external-role-grants/spec.md b/openspec/changes/configure-auth-and-initial-roles/specs/initial-external-role-grants/spec.md index 1c8e7d9c..709e3d87 100644 --- a/openspec/changes/configure-auth-and-initial-roles/specs/initial-external-role-grants/spec.md +++ b/openspec/changes/configure-auth-and-initial-roles/specs/initial-external-role-grants/spec.md @@ -21,6 +21,10 @@ - **WHEN** 外部来源未提供邮箱，或邮箱未验证 - **THEN** 系统不通过邮箱规则自动授予角色 +#### Scenario: 来源不提供邮箱验证信号 +- **WHEN** 管理员尝试为当前始终返回未验证邮箱的飞书或钉钉来源配置首次角色规则 +- **THEN** 系统拒绝创建无法生效的规则，并说明该来源不支持已验证邮箱匹配 + #### Scenario: 已有普通账号后来出现规则 - **WHEN** 已有身份绑定的普通账号再次登录，后台才新增对应规则 - **THEN** 登录不消费该规则，也不自动改变已有角色 diff --git a/openspec/changes/configure-auth-and-initial-roles/tasks.md b/openspec/changes/configure-auth-and-initial-roles/tasks.md index c0a91f5d..3e9a1330 100644 --- a/openspec/changes/configure-auth-and-initial-roles/tasks.md +++ b/openspec/changes/configure-auth-and-initial-roles/tasks.md @@ -8,7 +8,7 @@ - [x] 在本地登录、direct 本地认证、注册和密码管理服务端入口执行对应开关；关闭密码登录时一并阻止会直接建会话的本地注册。 - [x] 在 OAuth 普通准入允许后的首次 ACTIVE 账号创建事务中匹配并消费角色规则，首次主体包含新角色。 -- [ ] 在统一身份核心的 `LEGACY`、`SHADOW`、`ACTIVE` 模式下核对公开 OAuth 接入点；保持旧身份绑定写入权威与同邮箱不自动合并的现有行为。 +- [x] 在统一身份核心的 `LEGACY`、`SHADOW`、`ACTIVE` 模式下核对公开 OAuth 接入点；保持旧身份绑定写入权威与同邮箱不自动合并的现有行为（`OAuthLoginFlowServiceTest`、`IdentityBindingServiceTest`）。 - [ ] 保持已有账号、准入拒绝、未验证邮箱、停用规则、并发首次登录和人工角色修改的既定行为。 ## 3. API 与 Web diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializer.java index 771449cd..c2b9d059 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializer.java @@ -99,6 +99,9 @@ public class InitialAuthSettingsInitializer implements ApplicationRunner { if (!PROVIDER.matcher(provider).matches() || !EMAIL.matcher(email).matches()) { throw new IllegalStateException("Invalid initial role grant identity"); } + if ("feishu".equals(provider) || "dingtalk".equals(provider)) { + throw new IllegalStateException("Provider does not attest verified email for initial role grants: " + provider); + } if (!identities.add(provider + ":" + email)) { throw new IllegalStateException("Duplicate initial role grant identity"); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SystemAuthSettingsAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SystemAuthSettingsAppService.java index ee7cc779..51e9f5a9 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SystemAuthSettingsAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SystemAuthSettingsAppService.java @@ -160,6 +160,9 @@ public class SystemAuthSettingsAppService { if (!PROVIDER.matcher(normalized).matches()) { throw new DomainBadRequestException("error.system.roleGrant.invalidProvider"); } + if ("feishu".equals(normalized) || "dingtalk".equals(normalized)) { + throw new DomainBadRequestException("error.system.roleGrant.unverifiedProvider"); + } return normalized; } diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 734fd943..24bf9324 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -283,3 +283,4 @@ promotion.revocation.not_pending=Revocation request {0} is no longer pending promotion.revocation.required=Use the reviewed revocation workflow to remove a promoted skill promotion.revocation.page_invalid=Page must be nonnegative and size must be between 1 and 100 error.auth.settings.unavailable=Authentication settings are unavailable +error.system.roleGrant.unverifiedProvider=This identity provider does not attest verified email and cannot use initial role rules diff --git a/server/skillhub-app/src/main/resources/messages_ru.properties b/server/skillhub-app/src/main/resources/messages_ru.properties index 45a953c3..566d443e 100644 --- a/server/skillhub-app/src/main/resources/messages_ru.properties +++ b/server/skillhub-app/src/main/resources/messages_ru.properties @@ -224,3 +224,4 @@ error.suite.bundle.operation.retry.notAllowed=Повторить можно то error.suite.bundle.member.stateChanged=Состояние связанного Skill или версии изменилось; создайте новый предварительный просмотр пакета Skill Suite error.suite.bundle.actor.inactive=Пользователь операции Skill Suite Bundle больше не активен error.auth.settings.unavailable=Настройки аутентификации временно недоступны +error.system.roleGrant.unverifiedProvider=Этот поставщик удостоверений не подтверждает адрес почты и не поддерживает правила первоначального назначения роли diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 838e9943..90bad54b 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -283,3 +283,4 @@ promotion.revocation.not_pending=撤销申请 {0} 已不在待审核状态 promotion.revocation.required=已提升的技能须通过审核撤销流程删除 promotion.revocation.page_invalid=页码不能为负数，每页数量须在 1 到 100 之间 error.auth.settings.unavailable=认证配置暂时不可用 +error.system.roleGrant.unverifiedProvider=该身份来源不提供已验证邮箱，不能用于首次角色授权规则 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializerTest.java index 14bf7f0f..5b2bd6b3 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/InitialAuthSettingsInitializerTest.java @@ -5,6 +5,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.auth.repository.RoleRepository; @@ -64,4 +65,19 @@ class InitialAuthSettingsInitializerTest { verify(rules, never()).save(any()); verify(jdbc, never()).queryForObject(eq("SELECT COUNT(*) FROM user_account"), eq(Long.class)); } + + @Test + void initialGrantForUnverifiedProviderFailsStartupInsteadOfCreatingDeadRule() { + when(settings.findBySettingKey(LocalAuthSettingsService.SETTING_KEY)).thenReturn(Optional.empty()); + when(settings.findBySettingKey("auth.initial-role-grants.initialized")).thenReturn(Optional.empty()); + when(jdbc.queryForObject("SELECT COUNT(*) FROM user_account", Long.class)).thenReturn(0L); + InitialAuthSettingsProperties properties = new InitialAuthSettingsProperties(); + properties.setRoleGrantsJson("[{\"provider\":\"feishu\",\"email\":\"admin@example.com\",\"role\":\"SUPER_ADMIN\"}]"); + + assertThatThrownBy(() -> new InitialAuthSettingsInitializer( + properties, settings, rules, roles, new ObjectMapper(), jdbc).run(args)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("verified email"); + verify(rules, never()).save(any()); + } } 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 8e4118c6..8b1c47ba 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 @@ -2,6 +2,8 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.local.LocalCredentialRepository; +import com.iflytek.skillhub.auth.settings.LocalAuthSettings; +import com.iflytek.skillhub.auth.settings.LocalAuthSettingsService; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.user.UserAccount; @@ -68,6 +70,14 @@ class AuthControllerTest { @MockBean private LocalCredentialRepository localCredentialRepository; + @MockBean + private LocalAuthSettingsService localAuthSettingsService; + + @org.junit.jupiter.api.BeforeEach + void localAuthEnabled() { + given(localAuthSettingsService.current()).willReturn(new LocalAuthSettings(1L, true, true, 0L, null)); + } + @Test void meShouldReturnUnauthorizedForAnonymousRequest() throws Exception { mockMvc.perform(get("/api/v1/auth/me")) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthRateLimitControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthRateLimitControllerTest.java index 6e2d936e..91ee7b63 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthRateLimitControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AuthRateLimitControllerTest.java @@ -3,6 +3,8 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.local.LocalAuthService; import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.settings.LocalAuthSettings; +import com.iflytek.skillhub.auth.settings.LocalAuthSettingsService; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.metrics.SkillHubMetrics; import com.iflytek.skillhub.ratelimit.RateLimiter; @@ -49,6 +51,14 @@ class AuthRateLimitControllerTest { @MockBean private AuthFailureThrottleService authFailureThrottleService; + @MockBean + private LocalAuthSettingsService localAuthSettingsService; + + @org.junit.jupiter.api.BeforeEach + void localAuthEnabled() { + given(localAuthSettingsService.current()).willReturn(new LocalAuthSettings(1L, true, true, 0L, null)); + } + @Test void localLoginShouldReturnTooManyRequestsWhenRateLimitIsExceeded() throws Exception { given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(false); 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 97db51d4..5ea3f739 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 @@ -9,6 +9,8 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import com.iflytek.skillhub.auth.local.LocalCredentialRepository; import com.iflytek.skillhub.auth.local.LocalAuthService; +import com.iflytek.skillhub.auth.settings.LocalAuthSettings; +import com.iflytek.skillhub.auth.settings.LocalAuthSettingsService; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -56,6 +58,14 @@ class DirectAuthControllerTest { @MockBean private LocalCredentialRepository localCredentialRepository; + @MockBean + private LocalAuthSettingsService localAuthSettingsService; + + @org.junit.jupiter.api.BeforeEach + void localAuthEnabled() { + given(localAuthSettingsService.current()).willReturn(new LocalAuthSettings(1L, true, true, 0L, null)); + } + @Test void directLoginShouldAuthenticateViaConfiguredProvider() throws Exception { PlatformPrincipal principal = new PlatformPrincipal( 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 36fd80bf..a430b30f 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 @@ -2,6 +2,8 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.bootstrap.PassiveSessionAuthenticator; import com.iflytek.skillhub.auth.local.LocalCredentialRepository; +import com.iflytek.skillhub.auth.settings.LocalAuthSettings; +import com.iflytek.skillhub.auth.settings.LocalAuthSettingsService; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -52,6 +54,14 @@ class SessionBootstrapControllerTest { @MockBean private LocalCredentialRepository localCredentialRepository; + @MockBean + private LocalAuthSettingsService localAuthSettingsService; + + @org.junit.jupiter.api.BeforeEach + void localAuthEnabled() { + given(localAuthSettingsService.current()).willReturn(new LocalAuthSettings(1L, true, true, 0L, null)); + } + @Test void sessionBootstrapShouldEstablishSessionWhenAuthenticatorSucceeds() throws Exception { given(namespaceMemberRepository.findByUserId("sso-user-1")).willReturn(List.of()); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SystemAuthSettingsPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SystemAuthSettingsPostgresTest.java new file mode 100644 index 00000000..28b67ce6 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SystemAuthSettingsPostgresTest.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.repository; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.iflytek.skillhub.auth.repository.RoleRepository; +import com.iflytek.skillhub.auth.settings.ExternalRoleGrantRule; +import com.iflytek.skillhub.auth.settings.ExternalRoleGrantRuleRepository; +import com.iflytek.skillhub.auth.settings.SystemSetting; +import com.iflytek.skillhub.auth.settings.SystemSettingRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import jakarta.persistence.EntityManager; +import java.util.Map; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@ActiveProfiles("test") +@Testcontainers +class SystemAuthSettingsPostgresTest { + @Container + private static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer<>("postgres:16-alpine"); + + @DynamicPropertySource + static void configurePostgres(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); + registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); + registry.add("spring.flyway.enabled", () -> true); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "validate"); + } + + @Autowired private SystemSettingRepository settings; + @Autowired private ExternalRoleGrantRuleRepository rules; + @Autowired private RoleRepository roles; + @Autowired private UserAccountRepository users; + @Autowired private EntityManager entityManager; + @Autowired private JdbcTemplate jdbc; + + @Test + void flywaySchemaStoresSettingsAsJsonObjectWithOptimisticVersion() { + String key = "test.auth." + UUID.randomUUID(); + SystemSetting setting = settings.saveAndFlush(new SystemSetting(key, + Map.of("passwordLoginEnabled", true, "selfRegistrationEnabled", true))); + Long id = setting.getId(); + entityManager.clear(); + + SystemSetting reloaded = settings.findBySettingKey(key).orElseThrow(); + assertThat(reloaded.getValue().get("passwordLoginEnabled")).isEqualTo(true); + assertThat(jdbc.queryForObject("SELECT jsonb_typeof(value_json) FROM system_setting WHERE id = ?", + String.class, id)).isEqualTo("object"); + + reloaded.update(Map.of("passwordLoginEnabled", false, "selfRegistrationEnabled", true), "admin"); + settings.saveAndFlush(reloaded); + entityManager.clear(); + assertThat(settings.findBySettingKey(key).orElseThrow().getVersion()).isEqualTo(1L); + assertThat(settings.findBySettingKey(key).orElseThrow().getValue().get("passwordLoginEnabled")) + .isEqualTo(false); + } + + @Test + void consumedRuleKeepsActualExternalSubjectAndUser() { + String suffix = UUID.randomUUID().toString(); + String userId = "usr_" + suffix; + users.save(new UserAccount(userId, "new-user", null, null)); + entityManager.flush(); + var role = roles.findByCode("SUPER_ADMIN").orElseThrow(); + ExternalRoleGrantRule rule = rules.saveAndFlush(new ExternalRoleGrantRule( + "github", suffix + "@example.com", role, "admin")); + + rule.consume("external-" + suffix, userId); + rules.saveAndFlush(rule); + entityManager.clear(); + + ExternalRoleGrantRule reloaded = rules.findById(rule.getId()).orElseThrow(); + assertThat(reloaded.getStatus()).isEqualTo(ExternalRoleGrantRule.Status.CONSUMED); + assertThat(reloaded.getMatchedSubject()).isEqualTo("external-" + suffix); + assertThat(reloaded.getGrantedUserId()).isEqualTo(userId); + assertThat(reloaded.getGrantedAt()).isNotNull(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SystemAuthSettingsAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SystemAuthSettingsAppServiceTest.java index e1f39f52..81d5fb12 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SystemAuthSettingsAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SystemAuthSettingsAppServiceTest.java @@ -9,6 +9,7 @@ import static org.mockito.ArgumentMatchers.any; import com.iflytek.skillhub.auth.repository.RoleRepository; import com.iflytek.skillhub.auth.settings.ExternalRoleGrantRule; import com.iflytek.skillhub.auth.settings.ExternalRoleGrantRuleRepository; +import com.iflytek.skillhub.auth.settings.LocalAuthSettings; import com.iflytek.skillhub.auth.settings.LocalAuthSettingsService; import com.iflytek.skillhub.auth.settings.SystemSetting; import com.iflytek.skillhub.auth.settings.SystemSettingRepository; @@ -24,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.test.util.ReflectionTestUtils; @ExtendWith(MockitoExtension.class) class SystemAuthSettingsAppServiceTest { @@ -53,15 +55,47 @@ class SystemAuthSettingsAppServiceTest { verify(settings, never()).saveAndFlush(any()); } + @Test + void successfulSettingsChangeWritesAuditAndReturnsStoredValue() { + SystemSetting current = new SystemSetting("auth.local", + Map.of("passwordLoginEnabled", true, "selfRegistrationEnabled", true)); + ReflectionTestUtils.setField(current, "id", 1L); + when(settings.findBySettingKey("auth.local")).thenReturn(Optional.of(current)); + when(localSettings.current()).thenReturn(new LocalAuthSettings(1L, false, true, 1L, null)); + + var response = service.updateLocalSettings( + new SystemAuthSettingsUpdateRequest(false, true, 0L), "admin", + new AuditRequestContext("127.0.0.1", "test")); + + org.assertj.core.api.Assertions.assertThat(response.passwordLoginEnabled()).isFalse(); + org.assertj.core.api.Assertions.assertThat(current.getValue().get("passwordLoginEnabled")).isEqualTo(false); + verify(settings).saveAndFlush(current); + verify(audit).record(org.mockito.ArgumentMatchers.eq("admin"), + org.mockito.ArgumentMatchers.eq("SYSTEM_AUTH_SETTINGS_UPDATE"), + org.mockito.ArgumentMatchers.eq("SYSTEM_SETTING"), + org.mockito.ArgumentMatchers.eq(1L), org.mockito.ArgumentMatchers.eq(null), + org.mockito.ArgumentMatchers.eq("127.0.0.1"), + org.mockito.ArgumentMatchers.eq("test"), any()); + } + @Test void duplicateActiveGrantIsRejectedBeforeWrite() { when(rules.existsByProviderCodeAndNormalizedEmailAndStatus( - "feishu", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)).thenReturn(true); + "github", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)).thenReturn(true); assertThatThrownBy(() -> service.createRule( - new ExternalRoleGrantCreateRequest(" FEISHU ", "Admin@Example.Com", "SUPER_ADMIN"), + new ExternalRoleGrantCreateRequest(" GITHUB ", "Admin@Example.Com", "SUPER_ADMIN"), "admin", new AuditRequestContext(null, null))) .isInstanceOf(DomainConflictException.class); verify(rules, never()).saveAndFlush(any()); } + + @Test + void providerWithoutVerifiedEmailCannotCreateDeadGrantRule() { + assertThatThrownBy(() -> service.createRule( + new ExternalRoleGrantCreateRequest("feishu", "admin@example.com", "SUPER_ADMIN"), + "admin", new AuditRequestContext(null, null))) + .isInstanceOf(com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException.class); + verify(rules, never()).saveAndFlush(any()); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java index 1450e5d1..d2b4715d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java @@ -8,6 +8,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.inOrder; import com.iflytek.skillhub.auth.entity.IdentityBinding; import com.iflytek.skillhub.auth.entity.Role; @@ -28,6 +30,7 @@ import com.iflytek.skillhub.domain.user.UserStatus; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -173,6 +176,44 @@ class IdentityBindingServiceTest { assertThat(principal.platformRoles()).containsExactly("USER"); } + @Test + void bindOrCreate_grantIsVisibleInFirstPrincipalBeforeSessionCreation() { + OAuthClaims claims = new OAuthClaims("feishu", "external-1", "admin@example.com", true, "admin", Map.of()); + Role role = new Role(); + ReflectionTestUtils.setField(role, "code", "SUPER_ADMIN"); + AtomicBoolean granted = new AtomicBoolean(); + when(bindingRepo.findByProviderCodeAndSubject("feishu", "external-1")).thenReturn(Optional.empty()); + when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + doAnswer(invocation -> { + granted.set(true); + return null; + }).when(initialRoleGrants).grantForNewUser(any(), any()); + when(roleBindingRepo.findByUserId(any())).thenAnswer(invocation -> granted.get() + ? List.of(new UserRoleBinding(invocation.getArgument(0), role)) : List.of()); + + PlatformPrincipal principal = service.bindOrCreate(claims, UserStatus.ACTIVE); + + assertThat(principal.platformRoles()).contains("SUPER_ADMIN"); + var ordered = inOrder(initialRoleGrants, roleBindingRepo); + ordered.verify(initialRoleGrants).grantForNewUser(claims, principal.userId()); + ordered.verify(roleBindingRepo).findByUserId(principal.userId()); + } + + @Test + void bindOrCreate_doesNotMergeWithLocalAccountSharingEmail() { + OAuthClaims claims = new OAuthClaims("feishu", "external-1", "shared@example.com", true, + "external-user", Map.of()); + when(bindingRepo.findByProviderCodeAndSubject("feishu", "external-1")).thenReturn(Optional.empty()); + when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(roleBindingRepo.findByUserId(any())).thenReturn(List.of()); + + PlatformPrincipal principal = service.bindOrCreate(claims, UserStatus.ACTIVE); + + assertThat(principal.userId()).startsWith("usr_"); + verify(userRepo, never()).findByEmailIgnoreCase(any()); + verify(initialRoleGrants).grantForNewUser(claims, principal.userId()); + } + @Test void bindOrCreate_existingDisabledUser_throwsAccountDisabled() { OAuthClaims claims = new OAuthClaims( diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/InitialExternalRoleGrantServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/InitialExternalRoleGrantServiceTest.java index c2232671..cc6a246e 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/InitialExternalRoleGrantServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/InitialExternalRoleGrantServiceTest.java @@ -52,6 +52,8 @@ class InitialExternalRoleGrantServiceTest { assertThat(rule.getMatchedSubject()).isEqualTo("external-42"); assertThat(rule.getGrantedUserId()).isEqualTo("usr_new"); verify(rules).saveAndFlush(rule); + verify(audit).record(eq(null), eq("INITIAL_ROLE_GRANTED"), eq("EXTERNAL_ROLE_GRANT_RULE"), + eq(rule.getId()), eq(null), eq(null), eq(null), any()); } @Test @@ -61,5 +63,18 @@ class InitialExternalRoleGrantServiceTest { verify(rules, never()).lockByIdentityAndStatus(any(), any(), eq(ExternalRoleGrantRule.Status.ACTIVE)); verify(bindings, never()).save(any()); + verify(audit, never()).record(any(), any(), any(), any(), any(), any(), any(), any()); + } + + @Test + void absentActiveRuleDoesNotGrantOrAudit() { + when(rules.lockByIdentityAndStatus("feishu", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)) + .thenReturn(Optional.empty()); + + service.grantForNewUser(new OAuthClaims("feishu", "external-42", "admin@example.com", true, + "admin", Map.of()), "usr_new"); + + verify(bindings, never()).save(any()); + verify(audit, never()).record(any(), any(), any(), any(), any(), any(), any(), any()); } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/LocalAuthSettingsServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/LocalAuthSettingsServiceTest.java index c8b6f55c..253fcf6f 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/LocalAuthSettingsServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/settings/LocalAuthSettingsServiceTest.java @@ -11,6 +11,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.DataAccessResourceFailureException; +import org.springframework.http.HttpStatus; import org.springframework.test.util.ReflectionTestUtils; @ExtendWith(MockitoExtension.class) @@ -69,6 +71,17 @@ class LocalAuthSettingsServiceTest { void missingSettingFailsClosed() { when(repository.findBySettingKey("auth.local")).thenReturn(Optional.empty()); assertThatThrownBy(() -> new LocalAuthSettingsService(repository).requirePasswordLogin()) - .isInstanceOf(AuthFlowException.class); + .isInstanceOf(AuthFlowException.class) + .extracting("status").isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + } + + @Test + void databaseFailureFailsClosedWithServiceUnavailable() { + when(repository.findBySettingKey("auth.local")) + .thenThrow(new DataAccessResourceFailureException("database unavailable")); + + assertThatThrownBy(() -> new LocalAuthSettingsService(repository).requirePasswordLogin()) + .isInstanceOf(AuthFlowException.class) + .extracting("status").isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); } } diff --git a/web/src/features/auth/use-local-auth-capabilities.ts b/web/src/features/auth/use-local-auth-capabilities.ts index 25cdd396..53b9db2d 100644 --- a/web/src/features/auth/use-local-auth-capabilities.ts +++ b/web/src/features/auth/use-local-auth-capabilities.ts @@ -5,6 +5,6 @@ export function useLocalAuthCapabilities() { return useQuery({ queryKey: ['auth', 'local-capabilities'], queryFn: authApi.getLocalCapabilities, - staleTime: 30_000, + staleTime: 0, }) } diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 8d269a6f..ad920e45 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -1684,7 +1684,7 @@ "registrationRequiresLogin": "Password login is off, so registration is currently unavailable. The registration setting is retained.", "lockoutWarning": "Admins who rely only on passwords will not be able to sign in again. Continue?", "initialRoleRules": "Initial external account role rules", - "ruleHelp": "Applies only when a new external account is created after passing the existing access policy. A local account with the same email is not merged. Grant existing users a role in User Management.", + "ruleHelp": "Applies only when a new external account is created after passing the existing access policy. A local account with the same email is not merged. Grant existing users a role in User Management. Feishu and DingTalk do not currently provide verified email, so these rules are unavailable for them.", "provider": "Identity provider code", "email": "Verified email", "role": "Platform role", diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index fdf7affc..e432857e 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -2003,7 +2003,7 @@ "registrationRequiresLogin": "Вход по паролю отключён, поэтому регистрация сейчас недоступна. Настройка регистрации сохранена.", "lockoutWarning": "Администраторы, использующие только пароль, не смогут войти снова. Продолжить?", "initialRoleRules": "Правила первоначального назначения ролей", - "ruleHelp": "Правило действует только при создании новой внешней учётной записи после проверки доступа. Локальная запись с тем же адресом не объединяется. Существующим пользователям назначайте роль в управлении пользователями.", + "ruleHelp": "Правило действует только при создании новой внешней учётной записи после проверки доступа. Локальная запись с тем же адресом не объединяется. Существующим пользователям назначайте роль в управлении пользователями. Feishu и DingTalk сейчас не подтверждают адрес почты, поэтому такие правила для них недоступны.", "provider": "Код поставщика удостоверений", "email": "Подтверждённая почта", "role": "Роль платформы", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index e931cc8e..24afa0f0 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -1683,7 +1683,7 @@ "registrationRequiresLogin": "密码登录已关闭，当前无法自行注册；注册设置会保留。", "lockoutWarning": "关闭密码登录后，原本仅靠密码登录的管理员将无法重新登录。确定继续吗？", "initialRoleRules": "外部账号首次授权规则", - "ruleHelp": "仅当新外部账号首次创建且通过现有准入策略时生效。相同邮箱的本地账号不会合并；已有账号请在用户管理中授权。", + "ruleHelp": "仅当新外部账号首次创建且通过现有准入策略时生效。相同邮箱的本地账号不会合并；已有账号请在用户管理中授权。 当前飞书和钉钉不提供已验证邮箱，暂不能用此规则。", "provider": "身份来源代码", "email": "已验证邮箱", "role": "平台角色", diff --git a/web/src/pages/admin/system-config.test.tsx b/web/src/pages/admin/system-config.test.tsx new file mode 100644 index 00000000..d5c0522c --- /dev/null +++ b/web/src/pages/admin/system-config.test.tsx @@ -0,0 +1,51 @@ +/** @vitest-environment jsdom */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const api = vi.hoisted(() => ({ + getLocalAuth: vi.fn(), + updateLocalAuth: vi.fn(), + listRoles: vi.fn(), + listRoleGrants: vi.fn(), + createRoleGrant: vi.fn(), + updateRoleGrant: vi.fn(), + disableRoleGrant: vi.fn(), +})) + +vi.mock('@/api/client', () => ({ systemConfigApi: api })) +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) + +import { SystemConfigPage } from './system-config' + +describe('SystemConfigPage', () => { + afterEach(() => { + vi.restoreAllMocks() + Object.values(api).forEach((mock) => mock.mockReset()) + }) + + it('warns before disabling login and saves with the observed version', async () => { + api.getLocalAuth.mockResolvedValue({ passwordLoginEnabled: true, selfRegistrationEnabled: true, version: 3 }) + api.listRoles.mockResolvedValue([{ code: 'SUPER_ADMIN', name: 'Super Admin' }]) + api.listRoleGrants.mockResolvedValue([]) + api.updateLocalAuth.mockResolvedValue({ passwordLoginEnabled: false, selfRegistrationEnabled: true, version: 4 }) + vi.spyOn(window, 'confirm').mockReturnValue(true) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + + render() + const checkbox = await screen.findByRole('checkbox', { name: 'systemConfig.passwordLogin' }) + fireEvent.click(checkbox) + + expect(window.confirm).toHaveBeenCalledWith('systemConfig.lockoutWarning') + await waitFor(() => expect(api.updateLocalAuth.mock.calls[0]?.[0]).toEqual({ + passwordLoginEnabled: false, + selfRegistrationEnabled: true, + version: 3, + })) + await waitFor(() => expect(client.getQueryData(['auth', 'local-capabilities'])).toEqual({ + passwordLoginEnabled: false, + selfRegistrationEnabled: true, + registrationAvailable: false, + })) + }) +}) diff --git a/web/src/pages/admin/system-config.tsx b/web/src/pages/admin/system-config.tsx index 8fafe4ad..7759eb28 100644 --- a/web/src/pages/admin/system-config.tsx +++ b/web/src/pages/admin/system-config.tsx @@ -22,7 +22,17 @@ export function SystemConfigPage() { const refresh = async () => { await queryClient.invalidateQueries({ queryKey: ['system-config'] }) } - const updateSettings = useMutation({ mutationFn: systemConfigApi.updateLocalAuth, onSuccess: refresh }) + const updateSettings = useMutation({ + mutationFn: systemConfigApi.updateLocalAuth, + onSuccess: async (value) => { + queryClient.setQueryData(['auth', 'local-capabilities'], { + passwordLoginEnabled: value.passwordLoginEnabled, + selfRegistrationEnabled: value.selfRegistrationEnabled, + registrationAvailable: value.passwordLoginEnabled && value.selfRegistrationEnabled, + }) + await refresh() + }, + }) const createRule = useMutation({ mutationFn: systemConfigApi.createRoleGrant, onSuccess: refresh }) const updateRule = useMutation({ mutationFn: ({ id, version, code }: { id: number; version: number; code: string }) => diff --git a/web/src/pages/reset-password.test.tsx b/web/src/pages/reset-password.test.tsx index c28d1fbe..fba576b5 100644 --- a/web/src/pages/reset-password.test.tsx +++ b/web/src/pages/reset-password.test.tsx @@ -1,5 +1,7 @@ import { describe, expect, it, vi } from 'vitest' +const capabilities = vi.hoisted(() => ({ passwordLoginEnabled: true })) + vi.mock('@tanstack/react-router', () => ({ Link: ({ children }: { children: unknown }) => children, })) @@ -22,7 +24,7 @@ vi.mock('@/api/client', () => ({ })) vi.mock('@/features/auth/use-local-auth-capabilities', () => ({ - useLocalAuthCapabilities: () => ({ data: { passwordLoginEnabled: true }, isError: false }), + useLocalAuthCapabilities: () => ({ data: { passwordLoginEnabled: capabilities.passwordLoginEnabled }, isError: false }), })) vi.mock('@/shared/ui/button', () => ({ @@ -55,4 +57,12 @@ describe('ResetPasswordPage', () => { expect(html).toContain('resetPassword.sendCode') expect(html).toContain('resetPassword.submit') }) + + it('hides reset actions when password login is disabled', () => { + capabilities.passwordLoginEnabled = false + const html = renderToStaticMarkup() + expect(html).not.toContain('resetPassword.sendCode') + expect(html).toContain('systemConfig.passwordDisabled') + capabilities.passwordLoginEnabled = true + }) })