diff --git a/docs/923-auth-system-settings.md b/docs/923-auth-system-settings.md index b0d84914..cc23b6bb 100644 --- a/docs/923-auth-system-settings.md +++ b/docs/923-auth-system-settings.md @@ -6,7 +6,7 @@ ## 空库初始化 -仅在数据库没有对应记录时，启动参数提供初始值： +全新安装且尚无用户时，启动参数提供初始值。已有用户的实例升级时，若尚无 `auth.local` 记录，首次启动会把两个开关都设为开启，以保持原有登录行为： | 环境变量 | Helm 值 | 默认值 | | --- | --- | --- | @@ -26,6 +26,6 @@ 规则按身份来源代码和**已验证邮箱**匹配。只有外部身份通过现有准入策略、并且新账号首次创建为可用状态时，才给新账号授予选定平台角色；规则随后标为“已授权”，不会再次使用。拒绝或待审批的登录不会触发授权，规则不会绕过准入策略。已有账号请在“用户管理”中直接授权。相同邮箱的本地账号不会因此合并或获得角色。 -当前飞书和钉钉适配器把邮箱标记为未验证：它们的用户资料没有提供可证明用户控制该邮箱的信号。因此以 `feishu` 或 `dingtalk` 配置的邮箱规则不会触发自动授权。GitHub 的已验证主邮箱、提供 `email_verified=true` 的 OIDC 身份等可匹配。飞书或钉钉账号首次进入后，可由已有超级管理员在用户管理中授权；如果部署方只允许飞书或钉钉登录，必须先安排其他可用的管理员入口。不能为了触发规则而把未验证邮箱改成已验证。 +当前飞书和钉钉适配器把邮箱标记为未验证：它们的用户资料没有提供可证明用户控制该邮箱的信号。因此后台和初始化均拒绝为 `feishu` 或 `dingtalk` 配置邮箱规则。GitHub 的已验证主邮箱、提供 `email_verified=true` 的 OIDC 身份等可匹配。飞书或钉钉账号首次进入后，可由已有超级管理员在用户管理中授权；如果部署方只允许飞书或钉钉登录，必须先安排其他可用的管理员入口。不能为了触发规则而把未验证邮箱改成已验证。 后台可以新增规则、调整待匹配规则的角色、停用规则，并查看已使用规则的匹配身份和授权账号。只允许超级管理员操作；修改带版本号，避免两个管理员同时改动时后写覆盖前写。紧急恢复可通过受控数据库操作处理，并保留审计记录。 diff --git a/openspec/changes/configure-auth-and-initial-roles/specs/system-auth-settings/spec.md b/openspec/changes/configure-auth-and-initial-roles/specs/system-auth-settings/spec.md index d99b0300..7116dd0f 100644 --- a/openspec/changes/configure-auth-and-initial-roles/specs/system-auth-settings/spec.md +++ b/openspec/changes/configure-auth-and-initial-roles/specs/system-auth-settings/spec.md @@ -47,10 +47,16 @@ 新安装首次初始化时 SHALL 使用部署参数或代码默认值填充缺失设置，并持久化初始化状态。已经持久化的设置 SHALL 不被后续部署参数或重启覆盖。外部服务秘密不得存入设置表。 +已有用户的实例升级且首次缺失 `auth.local` 时 SHALL 初始化为两个开关均开启，保持原有认证行为；部署参数不应在该升级场景关闭本地认证。 + #### Scenario: 后台关闭登录后重启 - **WHEN** 后台已持久化 `passwordLoginEnabled=false`，部署参数仍写着开启 - **THEN** 重启后数据库设置仍为关闭 +#### Scenario: 已有用户的实例首次升级 +- **WHEN** 数据库已有用户，但首次升级时还没有 `auth.local`，且部署参数设为关闭本地密码登录 +- **THEN** 系统把两个开关初始化为开启，不因部署参数切断原有登录入口 + #### Scenario: 新版本增加设置键 - **WHEN** 升级版本新增注册过的设置键 - **THEN** 系统只为新键补入默认值，不修改既有键 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 c2b9d059..639f9718 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 @@ -67,15 +67,21 @@ public class InitialAuthSettingsInitializer implements ApplicationRunner { } return null; }); + Long existingUsers = null; if (settings.findBySettingKey(LocalAuthSettingsService.SETTING_KEY).isEmpty()) { + existingUsers = jdbcTemplate.queryForObject("SELECT COUNT(*) FROM user_account", Long.class); + boolean newInstallation = existingUsers == 0L; settings.save(new SystemSetting(LocalAuthSettingsService.SETTING_KEY, Map.of( - LocalAuthSettingsService.PASSWORD_LOGIN_KEY, properties.isPasswordLoginEnabled(), - LocalAuthSettingsService.SELF_REGISTRATION_KEY, properties.isSelfRegistrationEnabled()))); + LocalAuthSettingsService.PASSWORD_LOGIN_KEY, + newInstallation ? properties.isPasswordLoginEnabled() : true, + LocalAuthSettingsService.SELF_REGISTRATION_KEY, + newInstallation ? properties.isSelfRegistrationEnabled() : true))); } if (settings.findBySettingKey(GRANT_SEED_MARKER).isPresent()) { return; } - if (jdbcTemplate.queryForObject("SELECT COUNT(*) FROM user_account", Long.class) == 0L) { + if ((existingUsers != null ? existingUsers + : jdbcTemplate.queryForObject("SELECT COUNT(*) FROM user_account", Long.class)) == 0L) { seedInitialRules(); } settings.save(new SystemSetting(GRANT_SEED_MARKER, Map.of("initialized", true))); 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 5b2bd6b3..6e7ef191 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 @@ -48,6 +48,41 @@ class InitialAuthSettingsInitializerTest { verify(rules, never()).save(any()); } + @Test + void emptyDatabaseUsesConfiguredLoginFlags() { + 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.setPasswordLoginEnabled(false); + properties.setSelfRegistrationEnabled(false); + + new InitialAuthSettingsInitializer(properties, settings, rules, roles, new ObjectMapper(), jdbc).run(args); + + verify(settings).save(org.mockito.ArgumentMatchers.argThat(setting -> + setting.getSettingKey().equals(LocalAuthSettingsService.SETTING_KEY) + && setting.getValue().equals(Map.of("passwordLoginEnabled", false, + "selfRegistrationEnabled", false)))); + } + + @Test + void existingUsersUpgradeWithSafeDefaultsEvenWhenDeploymentFlagsAreDisabled() { + 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(2L); + InitialAuthSettingsProperties properties = new InitialAuthSettingsProperties(); + properties.setPasswordLoginEnabled(false); + properties.setSelfRegistrationEnabled(false); + + new InitialAuthSettingsInitializer(properties, settings, rules, roles, new ObjectMapper(), jdbc).run(args); + + verify(settings).save(org.mockito.ArgumentMatchers.argThat(setting -> + setting.getSettingKey().equals(LocalAuthSettingsService.SETTING_KEY) + && setting.getValue().equals(Map.of("passwordLoginEnabled", true, + "selfRegistrationEnabled", true)))); + verify(rules, never()).save(any()); + } + @Test void existingMarkerPreventsDeletedRulesFromBeingSeededAgain() { when(settings.findBySettingKey(LocalAuthSettingsService.SETTING_KEY)) 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 d2b4715d..4ef399d4 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 @@ -178,11 +178,11 @@ class IdentityBindingServiceTest { @Test void bindOrCreate_grantIsVisibleInFirstPrincipalBeforeSessionCreation() { - OAuthClaims claims = new OAuthClaims("feishu", "external-1", "admin@example.com", true, "admin", Map.of()); + OAuthClaims claims = new OAuthClaims("github", "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(bindingRepo.findByProviderCodeAndSubject("github", "external-1")).thenReturn(Optional.empty()); when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); doAnswer(invocation -> { granted.set(true); @@ -201,9 +201,9 @@ class IdentityBindingServiceTest { @Test void bindOrCreate_doesNotMergeWithLocalAccountSharingEmail() { - OAuthClaims claims = new OAuthClaims("feishu", "external-1", "shared@example.com", true, + OAuthClaims claims = new OAuthClaims("github", "external-1", "shared@example.com", true, "external-user", Map.of()); - when(bindingRepo.findByProviderCodeAndSubject("feishu", "external-1")).thenReturn(Optional.empty()); + when(bindingRepo.findByProviderCodeAndSubject("github", "external-1")).thenReturn(Optional.empty()); when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); when(roleBindingRepo.findByUserId(any())).thenReturn(List.of()); 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 099c2b89..25077e89 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 @@ -102,6 +102,17 @@ class LocalAuthServiceTest { verifyNoInteractions(userAccountRepository, credentialRepository); } + @Test + void disabledPasswordLoginStopsChangePasswordBeforeCredentialLookup() { + doThrow(new AuthFlowException(HttpStatus.FORBIDDEN, "error.auth.local.login.disabled")) + .when(authSettings).requirePasswordLogin(); + + assertThatThrownBy(() -> service.changePassword("usr_1", "old", "Newpass123!")) + .isInstanceOf(AuthFlowException.class) + .extracting("status").isEqualTo(HttpStatus.FORBIDDEN); + verifyNoInteractions(credentialRepository, passwordEncoder); + } + @Test void register_createsUserAndCredential() { given(credentialRepository.existsByUsernameIgnoreCase("alice")).willReturn(false); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java index 5c3515ae..026cedfc 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java @@ -87,6 +87,28 @@ class PasswordResetServiceTest { verifyNoInteractions(userAccountRepository, resetRequestRepository, mailSender); } + @Test + void disabledPasswordLoginStopsResetConfirmationBeforeCodeLookup() { + doThrow(new AuthFlowException(HttpStatus.FORBIDDEN, "error.auth.local.login.disabled")) + .when(authSettings).requirePasswordLogin(); + + assertThatThrownBy(() -> service.confirmPasswordReset("alice@example.com", "123456", "Abcd123!")) + .isInstanceOf(AuthFlowException.class) + .extracting("status").isEqualTo(HttpStatus.FORBIDDEN); + verifyNoInteractions(userAccountRepository, resetRequestRepository, credentialRepository); + } + + @Test + void disabledPasswordLoginStopsAdminResetBeforeAccountLookup() { + doThrow(new AuthFlowException(HttpStatus.FORBIDDEN, "error.auth.local.login.disabled")) + .when(authSettings).requirePasswordLogin(); + + assertThatThrownBy(() -> service.adminTriggerPasswordReset("usr_1", "admin_1")) + .isInstanceOf(AuthFlowException.class) + .extracting("status").isEqualTo(HttpStatus.FORBIDDEN); + verifyNoInteractions(userAccountRepository, resetRequestRepository, credentialRepository); + } + @Test void requestPasswordReset_withEligibleEmail_savesRequestAndSendsEmail() { UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); 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 cc6a246e..457a0466 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 @@ -36,12 +36,12 @@ class InitialExternalRoleGrantServiceTest { @Test void verifiedEmailConsumesMatchingRuleAndGrantsNewUser() { - ExternalRoleGrantRule rule = new ExternalRoleGrantRule("feishu", "admin@example.com", role, null); - when(rules.lockByIdentityAndStatus("feishu", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)) + ExternalRoleGrantRule rule = new ExternalRoleGrantRule("github", "admin@example.com", role, null); + when(rules.lockByIdentityAndStatus("github", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)) .thenReturn(Optional.of(rule)); when(role.getCode()).thenReturn("SUPER_ADMIN"); - service.grantForNewUser(new OAuthClaims("FEISHU", "external-42", " Admin@Example.COM ", true, + service.grantForNewUser(new OAuthClaims("GITHUB", "external-42", " Admin@Example.COM ", true, "admin", Map.of()), "usr_new"); ArgumentCaptor grant = ArgumentCaptor.forClass(UserRoleBinding.class); @@ -58,7 +58,7 @@ class InitialExternalRoleGrantServiceTest { @Test void unverifiedEmailCannotConsumeRule() { - service.grantForNewUser(new OAuthClaims("feishu", "external-42", "admin@example.com", false, + service.grantForNewUser(new OAuthClaims("github", "external-42", "admin@example.com", false, "admin", Map.of()), "usr_new"); verify(rules, never()).lockByIdentityAndStatus(any(), any(), eq(ExternalRoleGrantRule.Status.ACTIVE)); @@ -68,10 +68,10 @@ class InitialExternalRoleGrantServiceTest { @Test void absentActiveRuleDoesNotGrantOrAudit() { - when(rules.lockByIdentityAndStatus("feishu", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)) + when(rules.lockByIdentityAndStatus("github", "admin@example.com", ExternalRoleGrantRule.Status.ACTIVE)) .thenReturn(Optional.empty()); - service.grantForNewUser(new OAuthClaims("feishu", "external-42", "admin@example.com", true, + service.grantForNewUser(new OAuthClaims("github", "external-42", "admin@example.com", true, "admin", Map.of()), "usr_new"); verify(bindings, never()).save(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 253fcf6f..69ce5bb7 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 @@ -2,11 +2,13 @@ package com.iflytek.skillhub.auth.settings; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; import com.iflytek.skillhub.auth.exception.AuthFlowException; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -19,6 +21,26 @@ import org.springframework.test.util.ReflectionTestUtils; class LocalAuthSettingsServiceTest { @Mock private SystemSettingRepository repository; + @Test + void separateServiceInstancesObserveCurrentStoredValue() { + SystemSetting initial = new SystemSetting("auth.local", + Map.of("passwordLoginEnabled", true, "selfRegistrationEnabled", true)); + ReflectionTestUtils.setField(initial, "id", 1L); + AtomicReference stored = new AtomicReference<>(initial); + when(repository.findBySettingKey("auth.local")).thenAnswer(invocation -> Optional.of(stored.get())); + LocalAuthSettingsService firstInstance = new LocalAuthSettingsService(repository); + LocalAuthSettingsService secondInstance = new LocalAuthSettingsService(repository); + + assertThat(firstInstance.current().passwordLoginEnabled()).isTrue(); + SystemSetting updated = new SystemSetting("auth.local", + Map.of("passwordLoginEnabled", false, "selfRegistrationEnabled", true)); + ReflectionTestUtils.setField(updated, "id", 1L); + stored.set(updated); + + assertThat(secondInstance.current().passwordLoginEnabled()).isFalse(); + assertThat(firstInstance.current().passwordLoginEnabled()).isFalse(); + } + @Test void closedPasswordLoginBlocksPasswordAndRegistration() { SystemSetting setting = new SystemSetting("auth.local",