diff --git a/docs/02-domain-model.md b/docs/02-domain-model.md index 65e92c29..cce7169d 100644 --- a/docs/02-domain-model.md +++ b/docs/02-domain-model.md @@ -250,7 +250,7 @@ - 状态语义： - `ACTIVE`：正常使用 - - `PENDING`：等待管理员审批（AccessPolicy 返回 PENDING_APPROVAL 时创建） + - `PENDING`：等待管理员审批（AccessPolicy 返回 PENDING_APPROVAL 时创建）；批准时必须在同一事务补齐 `@global` membership 后转为 `ACTIVE` - `DISABLED`：管理员封禁，登录后拒绝所有操作，返回 403 - `MERGED`：已合并到其他账号，保留记录不物理删除，登录时自动跳转到合并目标账号 - 授权层在每次请求时检查用户状态，非 `ACTIVE` 用户拒绝所有写操作 diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 2f4b7705..83d4a421 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -92,7 +92,7 @@ astron: ### 2.2 准入失败处理 - `DENY`：抛出 `OAuth2AccessDeniedException`，由 `failureHandler` 重定向到 `/access-denied` 页面。不创建用户，不建立 Session。 -- `PENDING_APPROVAL`：创建 `user_account`（status=`PENDING`），但不建立业务 Session。抛出 `AccountPendingException`，由 `failureHandler` 重定向到 `/pending-approval` 页面（纯静态提示页，无需登录态）。管理员在后台审批后状态变为 `ACTIVE`，用户下次 OAuth 登录才会正常建立 Session。 +- `PENDING_APPROVAL`：创建 `user_account`（status=`PENDING`），但不建立业务 Session。抛出 `AccountPendingException`，由 `failureHandler` 重定向到 `/pending-approval` 页面（纯静态提示页，无需登录态）。管理员在后台审批时，系统在同一事务内把状态变为 `ACTIVE` 并补齐 `@global` 的 `MEMBER` membership；任一步失败都回滚。用户下次 OAuth 登录才会正常建立 Session。 安全边界：PENDING / DISABLED 用户绝不会拥有有效的业务 Session，从根源上杜绝"待审批账号已认证"的风险。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java index b3e1bdc7..4a05055c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; import com.iflytek.skillhub.auth.repository.RoleRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; @@ -44,16 +45,19 @@ public class AdminUserAppService { private final UserAccountRepository userAccountRepository; private final UserRoleBindingRepository userRoleBindingRepository; private final RoleRepository roleRepository; + private final GlobalNamespaceMembershipService globalNamespaceMembershipService; public AdminUserAppService( AdminUserSearchRepository adminUserSearchRepository, UserAccountRepository userAccountRepository, UserRoleBindingRepository userRoleBindingRepository, - RoleRepository roleRepository) { + RoleRepository roleRepository, + GlobalNamespaceMembershipService globalNamespaceMembershipService) { this.adminUserSearchRepository = adminUserSearchRepository; this.userAccountRepository = userAccountRepository; this.userRoleBindingRepository = userRoleBindingRepository; this.roleRepository = roleRepository; + this.globalNamespaceMembershipService = globalNamespaceMembershipService; } @Transactional(readOnly = true) @@ -111,6 +115,9 @@ public class AdminUserAppService { UserStatus nextStatus = parseManageableStatus(status); user.setStatus(nextStatus); userAccountRepository.save(user); + if (nextStatus == UserStatus.ACTIVE) { + globalNamespaceMembershipService.ensureMember(user.getId()); + } return new AdminUserMutationResponse(user.getId(), null, nextStatus.name()); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java index 8296f940..ede74cc8 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; import com.iflytek.skillhub.auth.repository.RoleRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; @@ -35,11 +36,14 @@ class AdminUserAppServiceTest { private final UserRoleBindingRepository userRoleBindingRepository = mock(UserRoleBindingRepository.class); private final RoleRepository roleRepository = mock(RoleRepository.class); private final UserAccountRepository userAccountRepository = mock(UserAccountRepository.class); + private final GlobalNamespaceMembershipService globalNamespaceMembershipService = + mock(GlobalNamespaceMembershipService.class); private final AdminUserAppService service = new AdminUserAppService( adminUserSearchRepository, userAccountRepository, userRoleBindingRepository, - roleRepository + roleRepository, + globalNamespaceMembershipService ); @Test @@ -159,10 +163,25 @@ class AdminUserAppServiceTest { var response = service.updateUserStatus("user-1", "DISABLED"); verify(userAccountRepository).save(user); + verify(globalNamespaceMembershipService, never()).ensureMember(any()); assertThat(user.getStatus()).isEqualTo(UserStatus.DISABLED); assertThat(response.status()).isEqualTo("DISABLED"); } + @Test + void updateUserStatus_activatingUserEnsuresGlobalMembership() { + UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.PENDING); + when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user)); + when(userAccountRepository.save(user)).thenReturn(user); + + var response = service.updateUserStatus("user-1", "ACTIVE"); + + verify(userAccountRepository).save(user); + verify(globalNamespaceMembershipService).ensureMember("user-1"); + assertThat(user.getStatus()).isEqualTo(UserStatus.ACTIVE); + assertThat(response.status()).isEqualTo("ACTIVE"); + } + @Test void updateUserStatus_rejectsSystemAccount() { when(userAccountRepository.findById("builtin-skill-publisher")) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserApprovalIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserApprovalIntegrationTest.java new file mode 100644 index 00000000..c58ac2b6 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserApprovalIntegrationTest.java @@ -0,0 +1,141 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.doAnswer; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserStatus; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.NamespaceMemberJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import java.util.List; +import java.util.UUID; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.transaction.support.TransactionTemplate; + +@SpringBootTest(classes = SkillhubApplication.class) +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class AdminUserApprovalIntegrationTest { + + @Autowired + private AdminUserAppService adminUserAppService; + + @Autowired + private UserAccountJpaRepository userAccountRepository; + + @Autowired + private NamespaceJpaRepository namespaceRepository; + + @Autowired + private NamespaceMemberJpaRepository namespaceMemberRepository; + + @Autowired + private TransactionTemplate transactionTemplate; + + @SpyBean + private GlobalNamespaceMembershipService globalNamespaceMembershipService; + + private String userId; + + @BeforeEach + void setUp() { + userId = "pending-" + UUID.randomUUID(); + transactionTemplate.executeWithoutResult(status -> { + ensureGlobalNamespace(); + UserAccount user = new UserAccount(userId, "Pending User", null, null); + user.setStatus(UserStatus.PENDING); + userAccountRepository.saveAndFlush(user); + }); + } + + @AfterEach + void tearDown() { + transactionTemplate.executeWithoutResult(status -> { + namespaceMemberRepository.findByUserId(userId) + .forEach(namespaceMemberRepository::delete); + userAccountRepository.deleteById(userId); + }); + } + + @Test + void activatingPendingUser_createsGlobalMembershipInSameWorkflow() { + adminUserAppService.updateUserStatus(userId, "ACTIVE"); + + transactionTemplate.executeWithoutResult(status -> { + UserAccount approved = userAccountRepository.findAllById(List.of(userId)) + .stream() + .findFirst() + .orElseThrow(); + Namespace global = namespaceRepository.findBySlug("global").orElseThrow(); + + assertThat(approved.getStatus()).isEqualTo(UserStatus.ACTIVE); + assertThat(namespaceMemberRepository.findByNamespaceIdAndUserId(global.getId(), userId)) + .get() + .extracting(member -> member.getRole()) + .isEqualTo(NamespaceRole.MEMBER); + }); + } + + @Test + void approvingActiveUserAgain_keepsSingleGlobalMembership() { + adminUserAppService.updateUserStatus(userId, "ACTIVE"); + adminUserAppService.updateUserStatus(userId, "ACTIVE"); + + transactionTemplate.executeWithoutResult(status -> { + Namespace global = namespaceRepository.findBySlug("global").orElseThrow(); + assertThat(namespaceMemberRepository.findByUserId(userId)) + .filteredOn(member -> member.getNamespaceId().equals(global.getId())) + .singleElement() + .extracting(member -> member.getRole()) + .isEqualTo(NamespaceRole.MEMBER); + }); + } + + @Test + void membershipFailure_rollsBackPendingUserActivation() { + doAnswer(invocation -> { + userAccountRepository.flush(); + throw new IllegalStateException("membership write failed"); + }).when(globalNamespaceMembershipService).ensureMember(userId); + + assertThatThrownBy(() -> adminUserAppService.updateUserStatus(userId, "ACTIVE")) + .isInstanceOf(IllegalStateException.class) + .hasMessage("membership write failed"); + + transactionTemplate.executeWithoutResult(status -> { + UserAccount user = userAccountRepository.findAllById(List.of(userId)) + .stream() + .findFirst() + .orElseThrow(); + Namespace global = namespaceRepository.findBySlug("global").orElseThrow(); + + assertThat(user.getStatus()).isEqualTo(UserStatus.PENDING); + assertThat(namespaceMemberRepository.findByNamespaceIdAndUserId(global.getId(), userId)) + .isEmpty(); + }); + } + + private void ensureGlobalNamespace() { + if (namespaceRepository.findBySlug("global").isPresent()) { + return; + } + Namespace global = new Namespace("global", "Global", null); + global.setType(NamespaceType.GLOBAL); + namespaceRepository.saveAndFlush(global); + } +}