From 075683963e066faebf45ecfc3bbc7f8b7ae711ec Mon Sep 17 00:00:00 2001 From: ylhu16 Date: Thu, 30 Jul 2026 15:03:26 +0800 Subject: [PATCH 1/2] fix(auth): provision global membership on user approval Closes #632 Signed-off-by: ylhu16 --- docs/02-domain-model.md | 2 +- docs/03-authentication-design.md | 2 +- .../skillhub/service/AdminUserAppService.java | 9 +- .../service/AdminUserAppServiceTest.java | 21 ++- .../AdminUserApprovalIntegrationTest.java | 141 ++++++++++++++++++ 5 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserApprovalIntegrationTest.java diff --git a/docs/02-domain-model.md b/docs/02-domain-model.md index 65e92c29c..cce7169db 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 2f4b77052..83d4a4214 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 b3e1bdc74..4a05055c0 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.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 AdminUserMutationResponse updateUserStatus(String userId, String status) 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 8296f940e..ede74cc89 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.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 @@ void updateUserStatus_updatesPersistedStatus() { 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 000000000..c58ac2b6c --- /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); + } +} From fe1c6e718b94b6b9cfcbbaff6b95778444b420a6 Mon Sep 17 00:00:00 2001 From: ylhu16 Date: Thu, 30 Jul 2026 15:20:12 +0800 Subject: [PATCH 2/2] fix(auth): harden approved account activation flow Signed-off-by: ylhu16 --- docs/02-domain-model.md | 2 +- docs/03-authentication-design.md | 2 +- scripts/smoke-test.sh | 76 ++++++++++++++++++- .../skillhub/service/AdminUserAppService.java | 3 + .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../service/AdminUserAppServiceTest.java | 13 ++++ .../AdminUserApprovalIntegrationTest.java | 57 ++++++++++++++ .../auth/oauth/OAuthLoginFlowService.java | 3 +- .../identity/IdentityBindingServiceTest.java | 26 +++++++ .../auth/oauth/OAuthLoginFlowServiceTest.java | 58 ++++++++++++++ 11 files changed, 236 insertions(+), 6 deletions(-) diff --git a/docs/02-domain-model.md b/docs/02-domain-model.md index cce7169db..4f3703440 100644 --- a/docs/02-domain-model.md +++ b/docs/02-domain-model.md @@ -252,7 +252,7 @@ - `ACTIVE`：正常使用 - `PENDING`：等待管理员审批（AccessPolicy 返回 PENDING_APPROVAL 时创建）；批准时必须在同一事务补齐 `@global` membership 后转为 `ACTIVE` - `DISABLED`：管理员封禁，登录后拒绝所有操作，返回 403 - - `MERGED`：已合并到其他账号，保留记录不物理删除，登录时自动跳转到合并目标账号 + - `MERGED`：已合并到其他账号，保留记录不物理删除，不允许通过管理员状态接口重新激活 - 授权层在每次请求时检查用户状态，非 `ACTIVE` 用户拒绝所有写操作 ### identity_binding diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 83d4a4214..7e843f9f2 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` 并补齐 `@global` 的 `MEMBER` membership；任一步失败都回滚。用户下次 OAuth 登录才会正常建立 Session。 +- `PENDING_APPROVAL`：首次登录创建 `user_account`（status=`PENDING`），但不建立业务 Session。抛出 `AccountPendingException`，由 `failureHandler` 重定向到 `/pending-approval` 页面（纯静态提示页，无需登录态）。管理员在后台审批时，系统在同一事务内把状态变为 `ACTIVE` 并补齐 `@global` 的 `MEMBER` membership；任一步失败都回滚。后续登录以已绑定账号的持久化状态为准：`ACTIVE` 正常建立 Session，`PENDING` 继续等待，`DISABLED` 拒绝登录；准入策略持续返回 `PENDING_APPROVAL` 不会覆盖已完成的管理员审批。 安全边界：PENDING / DISABLED 用户绝不会拥有有效的业务 Session，从根源上杜绝"待审批账号已认证"的风险。 diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index e1e462894..033b0dd29 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -5,13 +5,14 @@ BASE_URL="${1:-http://localhost:8080}" PASS=0 FAIL=0 COOKIE_JAR="$(mktemp)" +REGISTER_RESPONSE_FILE="$(mktemp)" USERNAME="smoketest_$(date +%s)" EMAIL="${USERNAME}@example.com" PASSWORD="Smoke@2026" NEW_PASSWORD="Smoke@2027" cleanup() { - rm -f "$COOKIE_JAR" + rm -f "$COOKIE_JAR" "$REGISTER_RESPONSE_FILE" } trap cleanup EXIT @@ -43,7 +44,7 @@ check "Auth required" "$BASE_URL/api/v1/auth/me" "401" curl -s -c "$COOKIE_JAR" "$BASE_URL/api/v1/auth/me" >/dev/null CSRF_TOKEN="$(awk '$6 == "XSRF-TOKEN" { print $7 }' "$COOKIE_JAR" | tail -n 1)" -REGISTER_STATUS="$(curl --max-time 10 -s -o /dev/null -w "%{http_code}" \ +REGISTER_STATUS="$(curl --max-time 10 -s -o "$REGISTER_RESPONSE_FILE" -w "%{http_code}" \ -X POST "$BASE_URL/api/v1/auth/local/register" \ -b "$COOKIE_JAR" \ -c "$COOKIE_JAR" \ @@ -58,6 +59,18 @@ else FAIL=$((FAIL + 1)) fi +REGISTERED_USER_ID="$(python3 - "$REGISTER_RESPONSE_FILE" <<'PY' +import json +import sys + +try: + with open(sys.argv[1], encoding="utf-8") as response: + print(json.load(response)["data"]["userId"]) +except (KeyError, TypeError, json.JSONDecodeError): + pass +PY +)" + AUTH_ME_STATUS="$(curl --max-time 10 -s -o /dev/null -w "%{http_code}" -b "$COOKIE_JAR" "$BASE_URL/api/v1/auth/me" || true)" if [[ "$AUTH_ME_STATUS" == "200" ]]; then echo "PASS: Auth me with session (HTTP $AUTH_ME_STATUS)" @@ -146,6 +159,65 @@ fi # Refresh CSRF after login ADMIN_CSRF="$(awk '$6 == "XSRF-TOKEN" { print $7 }' "$ADMIN_COOKIE_JAR" | tail -n 1)" +# Exercise the administrator activation workflow over HTTP. The transactional +# integration test covers the missing-membership precondition; this smoke path +# verifies the deployed controller, security, persistence, and read model. +if [[ -n "$REGISTERED_USER_ID" && "$ADMIN_LOGIN_STATUS" == "200" ]]; then + DISABLE_USER_STATUS="$(curl --max-time 10 -s -o /dev/null -w "%{http_code}" \ + -X POST "$BASE_URL/api/v1/admin/users/$REGISTERED_USER_ID/disable" \ + -b "$ADMIN_COOKIE_JAR" \ + -H "X-XSRF-TOKEN: $ADMIN_CSRF" || true)" + if [[ "$DISABLE_USER_STATUS" == "200" ]]; then + echo "PASS: Admin disables smoke user (HTTP $DISABLE_USER_STATUS)" + PASS=$((PASS + 1)) + else + echo "FAIL: Admin disables smoke user (got $DISABLE_USER_STATUS)" + FAIL=$((FAIL + 1)) + fi + + APPROVE_USER_STATUS="$(curl --max-time 10 -s -o /dev/null -w "%{http_code}" \ + -X POST "$BASE_URL/api/v1/admin/users/$REGISTERED_USER_ID/approve" \ + -b "$ADMIN_COOKIE_JAR" \ + -H "X-XSRF-TOKEN: $ADMIN_CSRF" || true)" + if [[ "$APPROVE_USER_STATUS" == "200" ]]; then + echo "PASS: Admin activates smoke user (HTTP $APPROVE_USER_STATUS)" + PASS=$((PASS + 1)) + else + echo "FAIL: Admin activates smoke user (got $APPROVE_USER_STATUS)" + FAIL=$((FAIL + 1)) + fi + + GLOBAL_MEMBERS_RESPONSE="$(curl --max-time 10 -s \ + -b "$ADMIN_COOKIE_JAR" \ + "$BASE_URL/api/web/namespaces/global/members?size=1000" || true)" + if JSON_INPUT="$GLOBAL_MEMBERS_RESPONSE" python3 - "$REGISTERED_USER_ID" <<'PY' +import json +import os +import sys + +user_id = sys.argv[1] +try: + items = json.loads(os.environ["JSON_INPUT"])["data"]["items"] +except (KeyError, TypeError, json.JSONDecodeError): + raise SystemExit(1) + +raise SystemExit(0 if any( + item.get("userId") == user_id and item.get("role") == "MEMBER" + for item in items +) else 1) +PY + then + echo "PASS: Activated user has @global MEMBER membership" + PASS=$((PASS + 1)) + else + echo "FAIL: Activated user is missing @global MEMBER membership" + FAIL=$((FAIL + 1)) + fi +else + echo "FAIL: Cannot exercise admin activation workflow without registered user and admin session" + FAIL=$((FAIL + 1)) +fi + # Create label definition CREATE_LABEL_STATUS="$(curl --max-time 10 -s -o /dev/null -w "%{http_code}" \ -X POST "$BASE_URL/api/v1/admin/labels" \ 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 4a05055c0..331440891 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 @@ -113,6 +113,9 @@ public AdminUserMutationResponse updateUserStatus(String userId, String status) UserAccount user = loadUser(userId); rejectSystemAccountMutation(user); UserStatus nextStatus = parseManageableStatus(status); + if (nextStatus == UserStatus.ACTIVE && user.getStatus() == UserStatus.MERGED) { + throw new DomainBadRequestException("error.admin.user.status.mergedCannotActivate"); + } user.setStatus(nextStatus); userAccountRepository.save(user); if (nextStatus == UserStatus.ACTIVE) { diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 79195af7a..4661a4eba 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -141,6 +141,7 @@ error.admin.user.role.superAdmin.assignDenied=Only SUPER_ADMIN can mutate SUPER_ error.admin.user.systemAccount.immutable=System accounts cannot be modified from user management error.admin.user.status.invalid=Invalid user status: {0} error.admin.user.status.unsupported=Only ACTIVE or DISABLED status can be managed here +error.admin.user.status.mergedCannotActivate=Merged accounts cannot be reactivated error.skill.publish.nameConflict=A published skill with name ''{0}'' already exists in this namespace error.skill.publish.nameConflict.private=A private skill with name ''{0}'' has already been published in this namespace error.skill.approve.nameConflict=Cannot approve: a published skill with name ''{0}'' already exists in this namespace diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index d7b6b11b5..412ec7bac 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -141,6 +141,7 @@ error.admin.user.role.superAdmin.assignDenied=只有 SUPER_ADMIN 可以修改 SU error.admin.user.systemAccount.immutable=系统账号不能在用户管理中修改 error.admin.user.status.invalid=无效的用户状态：{0} error.admin.user.status.unsupported=这里只允许管理 ACTIVE 或 DISABLED 状态的用户 +error.admin.user.status.mergedCannotActivate=已合并账号不能重新激活 error.skill.publish.nameConflict=该命名空间下已存在名为"{0}"的已发布技能，无法提交 error.skill.publish.nameConflict.private=该命名空间下已存在名为"{0}"的已发布私有技能，无法提交 error.skill.approve.nameConflict=无法通过审核：该命名空间下已存在名为"{0}"的已发布技能 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 ede74cc89..a832b948e 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 @@ -182,6 +182,19 @@ void updateUserStatus_activatingUserEnsuresGlobalMembership() { assertThat(response.status()).isEqualTo("ACTIVE"); } + @Test + void updateUserStatus_rejectsReactivatingMergedAccount() { + UserAccount user = user("user-1", "alice", "alice@example.com", UserStatus.MERGED); + when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(user)); + + assertThrows(DomainBadRequestException.class, + () -> service.updateUserStatus("user-1", "ACTIVE")); + + verify(userAccountRepository, never()).save(any(UserAccount.class)); + verify(globalNamespaceMembershipService, never()).ensureMember(any()); + assertThat(user.getStatus()).isEqualTo(UserStatus.MERGED); + } + @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 index c58ac2b6c..4270ef683 100644 --- 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 @@ -106,6 +106,27 @@ void approvingActiveUserAgain_keepsSingleGlobalMembership() { }); } + @Test + void enablingDisabledUser_createsGlobalMembership() { + setUserStatus(UserStatus.DISABLED); + + adminUserAppService.updateUserStatus(userId, "ACTIVE"); + + transactionTemplate.executeWithoutResult(status -> { + UserAccount enabled = userAccountRepository.findAllById(List.of(userId)) + .stream() + .findFirst() + .orElseThrow(); + Namespace global = namespaceRepository.findBySlug("global").orElseThrow(); + + assertThat(enabled.getStatus()).isEqualTo(UserStatus.ACTIVE); + assertThat(namespaceMemberRepository.findByNamespaceIdAndUserId(global.getId(), userId)) + .get() + .extracting(member -> member.getRole()) + .isEqualTo(NamespaceRole.MEMBER); + }); + } + @Test void membershipFailure_rollsBackPendingUserActivation() { doAnswer(invocation -> { @@ -130,6 +151,42 @@ void membershipFailure_rollsBackPendingUserActivation() { }); } + @Test + void membershipFailure_rollsBackDisabledUserActivation() { + setUserStatus(UserStatus.DISABLED); + 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.DISABLED); + assertThat(namespaceMemberRepository.findByNamespaceIdAndUserId(global.getId(), userId)) + .isEmpty(); + }); + } + + private void setUserStatus(UserStatus status) { + transactionTemplate.executeWithoutResult(transactionStatus -> { + UserAccount user = userAccountRepository.findAllById(List.of(userId)) + .stream() + .findFirst() + .orElseThrow(); + user.setStatus(status); + userAccountRepository.saveAndFlush(user); + }); + } + private void ensureGlobalNamespace() { if (namespaceRepository.findBySlug("global").isPresent()) { return; diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java index e5c7dc3de..a6b448a3f 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowService.java @@ -63,8 +63,7 @@ public PlatformPrincipal authenticate(OAuthClaims claims) { AccessDecision decision = accessPolicy.evaluate(claims); if (decision == AccessDecision.PENDING_APPROVAL) { - identityBindingService.createPendingUserIfAbsent(claims); - throw new AccountPendingException(); + return identityBindingService.bindOrCreate(claims, UserStatus.PENDING); } if (decision == AccessDecision.DENY) { throw new OAuth2AuthenticationException( 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 79e5df746..f5f822a5b 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 @@ -137,6 +137,32 @@ void bindOrCreate_existingDisabledUser_throwsAccountDisabled() { .isInstanceOf(AccountDisabledException.class); } + @Test + void bindOrCreate_existingApprovedUserIgnoresPendingInitialStatus() { + OAuthClaims claims = new OAuthClaims( + "github", + "gh_1", + "alice@example.com", + true, + "alice", + Map.of() + ); + IdentityBinding binding = new IdentityBinding("usr_1", "github", "gh_1", "alice"); + UserAccount user = new UserAccount("usr_1", "alice", "alice@example.com", null); + user.setStatus(UserStatus.ACTIVE); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("usr_1")).thenReturn(Optional.of(user)); + when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(roleBindingRepo.findByUserId("usr_1")).thenReturn(List.of()); + + PlatformPrincipal principal = service.bindOrCreate(claims, UserStatus.PENDING); + + assertThat(principal.userId()).isEqualTo("usr_1"); + assertThat(principal.platformRoles()).containsExactly("USER"); + verify(globalNamespaceMembershipService, never()).ensureMember(any()); + } + @Test void bindOrCreate_returnsExplicitPlatformRolesWhenBindingsExist() { OAuthClaims claims = new OAuthClaims( diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java index 029ec2944..74ec0594d 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuthLoginFlowServiceTest.java @@ -1,19 +1,66 @@ package com.iflytek.skillhub.auth.oauth; import com.iflytek.skillhub.auth.identity.IdentityBindingService; +import com.iflytek.skillhub.auth.policy.AccessDecision; import com.iflytek.skillhub.auth.policy.AccessPolicy; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.user.UserStatus; import jakarta.servlet.http.HttpSession; import java.util.List; +import java.util.Map; +import java.util.Set; import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class OAuthLoginFlowServiceTest { + @Test + void authenticate_allowsPreviouslyApprovedUserWhenPolicyRequiresApproval() { + AccessPolicy accessPolicy = mock(AccessPolicy.class); + IdentityBindingService identityBindingService = mock(IdentityBindingService.class); + OAuthLoginFlowService service = new OAuthLoginFlowService( + List.of(), + accessPolicy, + identityBindingService + ); + OAuthClaims claims = claims(); + PlatformPrincipal approvedPrincipal = new PlatformPrincipal( + "usr_1", "alice", "alice@example.com", null, "github", Set.of("USER")); + when(accessPolicy.evaluate(claims)).thenReturn(AccessDecision.PENDING_APPROVAL); + when(identityBindingService.bindOrCreate(claims, UserStatus.PENDING)).thenReturn(approvedPrincipal); + + PlatformPrincipal principal = service.authenticate(claims); + + assertThat(principal).isSameAs(approvedPrincipal); + verify(identityBindingService).bindOrCreate(claims, UserStatus.PENDING); + } + + @Test + void authenticate_rejectsDisabledUserWhenPolicyRequiresApproval() { + AccessPolicy accessPolicy = mock(AccessPolicy.class); + IdentityBindingService identityBindingService = mock(IdentityBindingService.class); + OAuthLoginFlowService service = new OAuthLoginFlowService( + List.of(), + accessPolicy, + identityBindingService + ); + OAuthClaims claims = claims(); + when(accessPolicy.evaluate(claims)).thenReturn(AccessDecision.PENDING_APPROVAL); + when(identityBindingService.bindOrCreate(claims, UserStatus.PENDING)) + .thenThrow(new AccountDisabledException()); + + assertThatThrownBy(() -> service.authenticate(claims)) + .isInstanceOf(AccountDisabledException.class); + } + @Test void rememberReturnTo_stores_sanitized_return_target() { OAuthLoginFlowService service = new OAuthLoginFlowService( @@ -64,4 +111,15 @@ void consumeReturnTo_clearsUnsafeSessionValue() { assertThat(returnTo).isNull(); assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE)).isNull(); } + + private OAuthClaims claims() { + return new OAuthClaims( + "github", + "gh_1", + "alice@example.com", + true, + "alice", + Map.of() + ); + } }