From e7d43891ce1a0b6e2770752554081cbada4c7ed9 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 29 Jul 2026 04:17:50 +0800 Subject: [PATCH 1/6] test(deploy): run PostgreSQL storage regression in CI Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .github/workflows/pr-e2e.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/pr-e2e.yml b/.github/workflows/pr-e2e.yml index f94ffdc29..29fb694a0 100644 --- a/.github/workflows/pr-e2e.yml +++ b/.github/workflows/pr-e2e.yml @@ -37,6 +37,9 @@ jobs: with: persist-credentials: false + - name: Verify Kubernetes PostgreSQL data-directory compatibility + run: bash scripts/tests/k8s-postgres-storage-test.sh + - name: Set up pnpm uses: pnpm/action-setup@v4 with: From b5607dfa053e96bd3024a130581044059ca8be5f Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 29 Jul 2026 04:21:27 +0800 Subject: [PATCH 2/6] test(deploy): wait for final PostgreSQL process Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- scripts/tests/k8s-postgres-storage-test.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/tests/k8s-postgres-storage-test.sh b/scripts/tests/k8s-postgres-storage-test.sh index f22242bb7..3a3d3fab9 100755 --- a/scripts/tests/k8s-postgres-storage-test.sh +++ b/scripts/tests/k8s-postgres-storage-test.sh @@ -40,7 +40,13 @@ fi wait_for_postgres() { local container="$1" for _ in $(seq 1 30); do - if docker exec "$container" pg_isready -U skillhub -d skillhub >/dev/null 2>&1; then + # docker-entrypoint.sh briefly starts a temporary PostgreSQL process while + # initializing a fresh cluster. Wait until PID 1 is the final server so a + # successful readiness probe cannot race with that temporary shutdown. + if docker exec "$container" sh -ec \ + 'test "$(cat /proc/1/comm)" = postgres' >/dev/null 2>&1 \ + && docker exec "$container" psql -U skillhub -d skillhub -Atqc \ + 'SELECT 1' >/dev/null 2>&1; then return 0 fi sleep 1 From 3f1eebd1e6130f39e16b90272e80110e558f9d46 Mon Sep 17 00:00:00 2001 From: ylhu16 Date: Thu, 30 Jul 2026 14:16:40 +0800 Subject: [PATCH 3/6] fix(auth): enforce trusted OAuth identity attributes Signed-off-by: ylhu16 --- docs/02-domain-model.md | 5 +- docs/03-authentication-design.md | 6 +- .../auth/identity/IdentityBindingService.java | 43 ++++-- .../auth/oauth/AccountMergedException.java | 14 ++ .../auth/oauth/GitHubClaimsExtractor.java | 17 ++- .../auth/oauth/OAuthLoginFlowService.java | 4 +- .../oauth/SystemAccountLoginException.java | 18 +++ .../auth/policy/EmailDomainAccessPolicy.java | 2 +- .../identity/IdentityBindingServiceTest.java | 140 +++++++++++++++++- .../auth/oauth/GitHubClaimsExtractorTest.java | 98 ++++++++++++ .../auth/oauth/OAuthLoginFlowServiceTest.java | 24 +++ .../auth/policy/AccessPolicyTest.java | 4 +- 12 files changed, 346 insertions(+), 29 deletions(-) create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountMergedException.java create mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SystemAccountLoginException.java create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractorTest.java diff --git a/docs/02-domain-model.md b/docs/02-domain-model.md index 65e92c29c..5aaaa4a64 100644 --- a/docs/02-domain-model.md +++ b/docs/02-domain-model.md @@ -245,6 +245,7 @@ | avatar_url | varchar(512) | | | status | enum | `ACTIVE` / `PENDING` / `DISABLED` / `MERGED` | | merged_to_user_id | varchar(128) | 合并目标用户 ID，仅 MERGED 状态有值 | +| system_account | boolean | 系统服务账号，禁止交互式 Web/OAuth 登录 | | created_at | datetime | | | updated_at | datetime | | @@ -252,8 +253,10 @@ - `ACTIVE`：正常使用 - `PENDING`：等待管理员审批（AccessPolicy 返回 PENDING_APPROVAL 时创建） - `DISABLED`：管理员封禁，登录后拒绝所有操作，返回 403 - - `MERGED`：已合并到其他账号，保留记录不物理删除，登录时自动跳转到合并目标账号 + - `MERGED`：已合并到其他账号，保留记录不物理删除；登录直接拒绝，不向调用方泄露合并目标 - 授权层在每次请求时检查用户状态，非 `ACTIVE` 用户拒绝所有写操作 +- system account 可按独立 Token Policy 使用非交互凭证，但不能通过本地密码或外部 OAuth + 建立普通用户 Session ### identity_binding diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 2f4b77052..269a85bc2 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -94,7 +94,8 @@ astron: - `DENY`：抛出 `OAuth2AccessDeniedException`，由 `failureHandler` 重定向到 `/access-denied` 页面。不创建用户，不建立 Session。 - `PENDING_APPROVAL`：创建 `user_account`（status=`PENDING`），但不建立业务 Session。抛出 `AccountPendingException`，由 `failureHandler` 重定向到 `/pending-approval` 页面（纯静态提示页，无需登录态）。管理员在后台审批后状态变为 `ACTIVE`，用户下次 OAuth 登录才会正常建立 Session。 -安全边界：PENDING / DISABLED 用户绝不会拥有有效的业务 Session，从根源上杜绝"待审批账号已认证"的风险。 +安全边界：PENDING / DISABLED / MERGED 用户和 system account 绝不会通过交互式登录获得 +业务 Session。外部身份命中这些账号时，在更新用户资料或加载角色前直接拒绝。 ### 2.3 扩展性 @@ -361,7 +362,8 @@ public class OAuthClaimsExtractor { 合并操作规则： - 合并操作写入审计日志 - 合并后原 user_account 标记为 `MERGED`，保留记录不物理删除 -- 预留扩展位：未来可配置 `astron.identity.auto-merge-on-verified-email=true` 开启基于已验证邮箱的自动合并 +- 不提供按 email 自动合并；即使 Provider 声明 email 已验证，也不能替代对两个账号控制权 + 的分别证明。未来绑定/合并必须使用显式、可审计的重新认证流程。 ## 5. CLI 认证（OAuth Device Flow + 平台凭证） diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java index 2a4fae8b3..49fa3dc7f 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/identity/IdentityBindingService.java @@ -1,7 +1,11 @@ package com.iflytek.skillhub.auth.identity; import com.iflytek.skillhub.auth.entity.IdentityBinding; +import com.iflytek.skillhub.auth.oauth.AccountDisabledException; +import com.iflytek.skillhub.auth.oauth.AccountMergedException; +import com.iflytek.skillhub.auth.oauth.AccountPendingException; import com.iflytek.skillhub.auth.oauth.OAuthClaims; +import com.iflytek.skillhub.auth.oauth.SystemAccountLoginException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.rbac.PlatformRoleDefaults; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; @@ -48,8 +52,9 @@ public PlatformPrincipal bindOrCreate(OAuthClaims claims, UserStatus initialStat if (binding != null) { user = userRepo.findById(binding.getUserId()) .orElseThrow(() -> new IllegalStateException("User not found for binding")); + ensureExternalLoginAllowed(user); user.setDisplayName(claims.providerLogin()); - if (claims.email() != null) user.setEmail(claims.email()); + if (trustedEmail(claims) != null) user.setEmail(claims.email()); if (claims.extra().get("avatar_url") != null) { user.setAvatarUrl((String) claims.extra().get("avatar_url")); } @@ -58,7 +63,7 @@ public PlatformPrincipal bindOrCreate(OAuthClaims claims, UserStatus initialStat user = new UserAccount( "usr_" + UUID.randomUUID(), claims.providerLogin(), - claims.email(), + trustedEmail(claims), (String) claims.extra().get("avatar_url") ); user.setStatus(initialStatus); @@ -71,12 +76,7 @@ public PlatformPrincipal bindOrCreate(OAuthClaims claims, UserStatus initialStat bindingRepo.save(binding); } - if (user.getStatus() == UserStatus.PENDING) { - throw new com.iflytek.skillhub.auth.oauth.AccountPendingException(); - } - if (user.getStatus() == UserStatus.DISABLED) { - throw new com.iflytek.skillhub.auth.oauth.AccountDisabledException(); - } + ensureExternalLoginAllowed(user); Set roles = roleBindingRepo.findByUserId(user.getId()).stream() .map(rb -> rb.getRole().getCode()) @@ -97,16 +97,14 @@ public void createPendingUserIfAbsent(OAuthClaims claims) { if (existingBinding != null) { UserAccount existingUser = userRepo.findById(existingBinding.getUserId()) .orElseThrow(() -> new IllegalStateException("User not found for binding")); - if (existingUser.getStatus() == UserStatus.DISABLED) { - throw new com.iflytek.skillhub.auth.oauth.AccountDisabledException(); - } - throw new com.iflytek.skillhub.auth.oauth.AccountPendingException(); + ensureExternalLoginAllowed(existingUser); + throw new AccountPendingException(); } UserAccount user = new UserAccount( "usr_" + UUID.randomUUID(), claims.providerLogin(), - claims.email(), + trustedEmail(claims), (String) claims.extra().get("avatar_url") ); user.setStatus(UserStatus.PENDING); @@ -115,4 +113,23 @@ public void createPendingUserIfAbsent(OAuthClaims claims) { IdentityBinding binding = new IdentityBinding(user.getId(), claims.provider(), claims.subject(), claims.providerLogin()); bindingRepo.save(binding); } + + private String trustedEmail(OAuthClaims claims) { + return claims.emailVerified() ? claims.email() : null; + } + + private void ensureExternalLoginAllowed(UserAccount user) { + if (user.isSystemAccount()) { + throw new SystemAccountLoginException(); + } + if (user.getStatus() == UserStatus.PENDING) { + throw new AccountPendingException(); + } + if (user.getStatus() == UserStatus.DISABLED) { + throw new AccountDisabledException(); + } + if (user.getStatus() == UserStatus.MERGED) { + throw new AccountMergedException(); + } + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountMergedException.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountMergedException.java new file mode 100644 index 000000000..0956be852 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/AccountMergedException.java @@ -0,0 +1,14 @@ +package com.iflytek.skillhub.auth.oauth; + +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; + +/** + * OAuth authentication exception raised when the mapped platform account was merged. + */ +public class AccountMergedException extends OAuth2AuthenticationException { + + public AccountMergedException() { + super(new OAuth2Error("account_merged", "Account was merged", null)); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractor.java index 51b802dac..b165e1faf 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractor.java @@ -9,7 +9,6 @@ import java.util.Comparator; import java.util.List; -import org.springframework.stereotype.Component; import java.util.Map; /** @@ -19,10 +18,14 @@ @Component public class GitHubClaimsExtractor implements OAuthClaimsExtractor { - private final RestClient restClient = RestClient.builder() - .baseUrl("https://api.github.com") - .defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) - .build(); + private final RestClient restClient; + + public GitHubClaimsExtractor(RestClient.Builder restClientBuilder) { + this.restClient = restClientBuilder + .baseUrl("https://api.github.com") + .defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) + .build(); + } @Override public String getProvider() { return "github"; } @@ -32,9 +35,7 @@ public OAuthClaims extract(OAuth2UserRequest request, OAuth2User oAuth2User) { Map attrs = oAuth2User.getAttributes(); GitHubEmail primaryEmail = loadPrimaryEmail(request); String email = primaryEmail != null ? primaryEmail.email() : (String) attrs.get("email"); - boolean emailVerified = primaryEmail != null - ? primaryEmail.verified() - : attrs.get("email") != null; + boolean emailVerified = primaryEmail != null && primaryEmail.verified(); return new OAuthClaims( "github", 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..ac3d413cb 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 @@ -98,7 +98,9 @@ public String resolveFailureRedirect(AuthenticationException exception, String r if (exception instanceof AccountPendingException) { return "/pending-approval"; } - if (exception instanceof AccountDisabledException) { + if (exception instanceof AccountDisabledException + || exception instanceof AccountMergedException + || exception instanceof SystemAccountLoginException) { return "/access-denied"; } if (exception instanceof OAuth2AuthenticationException oauth2Exception diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SystemAccountLoginException.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SystemAccountLoginException.java new file mode 100644 index 000000000..bab0c4cd3 --- /dev/null +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SystemAccountLoginException.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.auth.oauth; + +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2Error; + +/** + * OAuth authentication exception raised when an external identity resolves to a system account. + */ +public class SystemAccountLoginException extends OAuth2AuthenticationException { + + public SystemAccountLoginException() { + super(new OAuth2Error( + "system_account_forbidden", + "System accounts cannot use interactive OAuth login", + null + )); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/EmailDomainAccessPolicy.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/EmailDomainAccessPolicy.java index d688f2a2e..b65a738d5 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/EmailDomainAccessPolicy.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/EmailDomainAccessPolicy.java @@ -15,7 +15,7 @@ public EmailDomainAccessPolicy(Set allowedDomains) { @Override public AccessDecision evaluate(OAuthClaims claims) { - if (claims.email() == null) return AccessDecision.DENY; + if (claims.email() == null || !claims.emailVerified()) return AccessDecision.DENY; String domain = claims.email().substring(claims.email().indexOf('@') + 1); return allowedDomains.contains(domain.toLowerCase()) ? AccessDecision.ALLOW : AccessDecision.DENY; 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..75dda2de0 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 @@ -11,8 +11,10 @@ import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; import com.iflytek.skillhub.auth.oauth.AccountDisabledException; +import com.iflytek.skillhub.auth.oauth.AccountMergedException; import com.iflytek.skillhub.auth.oauth.OAuthClaims; import com.iflytek.skillhub.auth.oauth.AccountPendingException; +import com.iflytek.skillhub.auth.oauth.SystemAccountLoginException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; @@ -131,12 +133,84 @@ void bindOrCreate_existingDisabledUser_throwsAccountDisabled() { 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)); assertThatThrownBy(() -> service.bindOrCreate(claims, UserStatus.ACTIVE)) .isInstanceOf(AccountDisabledException.class); } + @Test + void bindOrCreate_existingMergedUser_throwsBeforeProfileUpdate() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "attacker@example.com", true, "attacker", 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.MERGED); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("usr_1")).thenReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.bindOrCreate(claims, UserStatus.ACTIVE)) + .isInstanceOf(AccountMergedException.class); + + assertThat(user.getDisplayName()).isEqualTo("alice"); + assertThat(user.getEmail()).isEqualTo("alice@example.com"); + verify(userRepo, never()).save(any(UserAccount.class)); + } + + @Test + void bindOrCreate_existingSystemAccount_throwsBeforeProfileUpdate() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "attacker@example.com", true, "attacker", Map.of() + ); + IdentityBinding binding = new IdentityBinding("system_1", "github", "gh_1", "system"); + UserAccount user = UserAccount.systemAccount("system_1", "system", null, null); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("system_1")).thenReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.bindOrCreate(claims, UserStatus.ACTIVE)) + .isInstanceOf(SystemAccountLoginException.class); + + assertThat(user.getDisplayName()).isEqualTo("system"); + assertThat(user.getEmail()).isNull(); + verify(userRepo, never()).save(any(UserAccount.class)); + } + + @Test + void bindOrCreate_unverifiedEmailDoesNotPopulateNewAccount() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "unverified@example.com", false, "alice", Map.of() + ); + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.empty()); + when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(roleBindingRepo.findByUserId(any())).thenReturn(List.of()); + + service.bindOrCreate(claims, UserStatus.ACTIVE); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); + verify(userRepo).save(userCaptor.capture()); + assertThat(userCaptor.getValue().getEmail()).isNull(); + } + + @Test + void bindOrCreate_unverifiedEmailDoesNotOverwriteExistingEmail() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "unverified@example.com", false, "alice", Map.of() + ); + IdentityBinding binding = new IdentityBinding("usr_1", "github", "gh_1", "alice"); + UserAccount user = new UserAccount("usr_1", "alice", "verified@example.com", null); + + 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()); + + service.bindOrCreate(claims, UserStatus.ACTIVE); + + assertThat(user.getEmail()).isEqualTo("verified@example.com"); + } + @Test void bindOrCreate_returnsExplicitPlatformRolesWhenBindingsExist() { OAuthClaims claims = new OAuthClaims( @@ -179,4 +253,68 @@ void createPendingUserIfAbsent_existingDisabledBinding_throwsAccountDisabled() { assertThatThrownBy(() -> service.createPendingUserIfAbsent(claims)) .isInstanceOf(AccountDisabledException.class); } + + @Test + void createPendingUserIfAbsent_existingPendingBinding_throwsAccountPending() { + 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.PENDING); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("usr_1")).thenReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.createPendingUserIfAbsent(claims)) + .isInstanceOf(AccountPendingException.class); + verify(userRepo, never()).save(any(UserAccount.class)); + verify(bindingRepo, never()).save(any(IdentityBinding.class)); + } + + @Test + void createPendingUserIfAbsent_existingMergedBinding_throwsAccountMerged() { + 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.MERGED); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("usr_1")).thenReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.createPendingUserIfAbsent(claims)) + .isInstanceOf(AccountMergedException.class); + } + + @Test + void createPendingUserIfAbsent_existingSystemBinding_throwsSystemAccountLogin() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "alice@example.com", true, "alice", Map.of() + ); + IdentityBinding binding = new IdentityBinding("system_1", "github", "gh_1", "system"); + UserAccount user = UserAccount.systemAccount("system_1", "system", null, null); + + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.of(binding)); + when(userRepo.findById("system_1")).thenReturn(Optional.of(user)); + + assertThatThrownBy(() -> service.createPendingUserIfAbsent(claims)) + .isInstanceOf(SystemAccountLoginException.class); + } + + @Test + void createPendingUserIfAbsent_unverifiedEmailDoesNotPopulateAccount() { + OAuthClaims claims = new OAuthClaims( + "github", "gh_1", "unverified@example.com", false, "alice", Map.of() + ); + when(bindingRepo.findByProviderCodeAndSubject("github", "gh_1")).thenReturn(Optional.empty()); + when(userRepo.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + service.createPendingUserIfAbsent(claims); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); + verify(userRepo).save(userCaptor.capture()); + assertThat(userCaptor.getValue().getEmail()).isNull(); + } } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractorTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractorTest.java new file mode 100644 index 000000000..35de77b8c --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/GitHubClaimsExtractorTest.java @@ -0,0 +1,98 @@ +package com.iflytek.skillhub.auth.oauth; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; + +import java.time.Instant; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.user.DefaultOAuth2User; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.web.client.RestClient; + +class GitHubClaimsExtractorTest { + + @Test + void extract_doesNotTrustProfileEmailWhenEmailsApiHasNoVerifiedEmail() { + RestClient.Builder restClientBuilder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(restClientBuilder).build(); + server.expect(requestTo("https://api.github.com/user/emails")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer token-123")) + .andRespond(withSuccess( + """ + [{"email":"alice@example.com","primary":true,"verified":false}] + """, + MediaType.APPLICATION_JSON + )); + GitHubClaimsExtractor extractor = new GitHubClaimsExtractor(restClientBuilder); + + OAuthClaims claims = extractor.extract(userRequest(), githubUser("alice@example.com")); + + assertThat(claims.email()).isEqualTo("alice@example.com"); + assertThat(claims.emailVerified()).isFalse(); + server.verify(); + } + + @Test + void extract_usesVerifiedEmailFromEmailsApi() { + RestClient.Builder restClientBuilder = RestClient.builder(); + MockRestServiceServer server = MockRestServiceServer.bindTo(restClientBuilder).build(); + server.expect(requestTo("https://api.github.com/user/emails")) + .andExpect(header(HttpHeaders.AUTHORIZATION, "Bearer token-123")) + .andRespond(withSuccess( + """ + [ + {"email":"secondary@example.com","primary":false,"verified":true}, + {"email":"alice@example.com","primary":true,"verified":true} + ] + """, + MediaType.APPLICATION_JSON + )); + GitHubClaimsExtractor extractor = new GitHubClaimsExtractor(restClientBuilder); + + OAuthClaims claims = extractor.extract(userRequest(), githubUser(null)); + + assertThat(claims.email()).isEqualTo("alice@example.com"); + assertThat(claims.emailVerified()).isTrue(); + server.verify(); + } + + private DefaultOAuth2User githubUser(String email) { + Map attributes = new java.util.HashMap<>(); + attributes.put("id", 42); + attributes.put("login", "alice"); + attributes.put("email", email); + return new DefaultOAuth2User(List.of(), attributes, "login"); + } + + private OAuth2UserRequest userRequest() { + ClientRegistration registration = ClientRegistration.withRegistrationId("github") + .clientId("client-id") + .clientSecret("client-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .scope("read:user", "user:email") + .authorizationUri("https://github.com/login/oauth/authorize") + .tokenUri("https://github.com/login/oauth/access_token") + .userInfoUri("https://api.github.com/user") + .userNameAttributeName("login") + .clientName("GitHub") + .build(); + OAuth2AccessToken accessToken = new OAuth2AccessToken( + OAuth2AccessToken.TokenType.BEARER, + "token-123", + Instant.now(), + Instant.now().plusSeconds(3600) + ); + return new OAuth2UserRequest(registration, accessToken); + } +} 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..d51311da5 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 @@ -48,6 +48,30 @@ void resolveFailureRedirect_maps_access_denied_to_user_facing_page() { assertThat(redirect).isEqualTo("/access-denied"); } + @Test + void resolveFailureRedirect_mapsMergedAccountToAccessDenied() { + OAuthLoginFlowService service = new OAuthLoginFlowService( + List.of(), + mock(AccessPolicy.class), + mock(IdentityBindingService.class) + ); + + assertThat(service.resolveFailureRedirect(new AccountMergedException(), null)) + .isEqualTo("/access-denied"); + } + + @Test + void resolveFailureRedirect_mapsSystemAccountToAccessDenied() { + OAuthLoginFlowService service = new OAuthLoginFlowService( + List.of(), + mock(AccessPolicy.class), + mock(IdentityBindingService.class) + ); + + assertThat(service.resolveFailureRedirect(new SystemAccountLoginException(), null)) + .isEqualTo("/access-denied"); + } + @Test void consumeReturnTo_clearsUnsafeSessionValue() { OAuthLoginFlowService service = new OAuthLoginFlowService( diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/AccessPolicyTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/AccessPolicyTest.java index 62a40abf5..3bfb7b90a 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/AccessPolicyTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/AccessPolicyTest.java @@ -37,10 +37,10 @@ void emailDomainPolicy_deniesNullEmail() { } @Test - void emailDomainPolicy_allowsUnverifiedEmailFromMatchingDomain() { + void emailDomainPolicy_deniesUnverifiedEmailFromMatchingDomain() { var policy = new EmailDomainAccessPolicy(Set.of("company.com")); var claims = new OAuthClaims("github", "123", "user@company.com", false, "user", Map.of()); - assertThat(policy.evaluate(claims)).isEqualTo(AccessDecision.ALLOW); + assertThat(policy.evaluate(claims)).isEqualTo(AccessDecision.DENY); } @Test From 075683963e066faebf45ecfc3bbc7f8b7ae711ec Mon Sep 17 00:00:00 2001 From: ylhu16 Date: Thu, 30 Jul 2026 15:03:26 +0800 Subject: [PATCH 4/6] 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 5/6] 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() + ); + } } From a13429a95e404844cdadce202e40fbcaf7419e11 Mon Sep 17 00:00:00 2001 From: ylhu16 Date: Thu, 30 Jul 2026 15:57:21 +0800 Subject: [PATCH 6/6] fix(auth): isolate unsafe account merge flow Keep the legacy routes fail-closed, remove the unsafe orchestration service, replace the UI controls with a security notice, and define the acceptance contract for the future safe merge flow. Closes #634 Parent: #628 Signed-off-by: ylhu16 --- docs/03-authentication-design.md | 18 +- docs/10-delivery-roadmap.md | 6 +- ...-secure-account-merge-acceptance-design.md | 461 ++++++++++++++++++ docs/skillhub/en/faq.md | 15 + docs/skillhub/faq.md | 13 + .../controller/AccountMergeController.java | 42 +- .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../AccountMergeControllerTest.java | 101 ++-- .../auth/merge/AccountMergeService.java | 288 ----------- .../auth/merge/AccountMergeServiceTest.java | 185 ------- web/src/i18n/locales/en.json | 23 +- web/src/i18n/locales/zh.json | 23 +- web/src/pages/settings/accounts.test.ts | 41 +- web/src/pages/settings/accounts.tsx | 131 +---- 15 files changed, 617 insertions(+), 732 deletions(-) create mode 100644 docs/22-secure-account-merge-acceptance-design.md delete mode 100644 server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/merge/AccountMergeService.java delete mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/merge/AccountMergeServiceTest.java diff --git a/docs/03-authentication-design.md b/docs/03-authentication-design.md index 41cd4576b..b8dce5552 100644 --- a/docs/03-authentication-design.md +++ b/docs/03-authentication-design.md @@ -353,17 +353,29 @@ public class OAuthClaimsExtractor { 同一个员工通过不同 OAuth Provider 登录时，可能产生多个 `user_account`。 -一期策略：默认关闭自动合并，仅支持管理员手动合并。 +当前策略：不自动合并，旧的手动合并流程也已临时隔离。旧流程把次账号 verification +token 直接返回给主账号会话，不能分别证明两个账号的控制权，因此不能继续作为管理员或 +用户合并入口。 - 一期 GitHub-only：不需要自动合并，每个 Provider 登录独立创建用户 -- 多 Provider 上线时，再引入显式绑定/合并流程（用户主动发起 + 邮箱验证确认） -- 管理员可在后台手动合并两个 user_account（合并 identity_binding、迁移 skill ownership、合并角色取并集） +- 多 Provider 上线时，再引入显式 Identity Link 和安全 Account Merge +- email、username、display name 或主账号会话拿到的 token 均不能证明次账号所有权 +- 安全 Account Merge 必须要求主、次账号分别完成 fresh reauthentication +- 在安全流程上线前，`/api/v1/account/merge/initiate`、`verify`、`confirm` 对已认证请求 + 统一返回 `503 Service Unavailable` 合并操作规则： - 合并操作写入审计日志 - 合并后原 user_account 标记为 `MERGED`，保留记录不物理删除 - 不提供按 email 自动合并；即使 Provider 声明 email 已验证，也不能替代对两个账号控制权 的分别证明。未来绑定/合并必须使用显式、可审计的重新认证流程。 +- 旧 `account_merge_request` 记录保留用于审计和未来迁移，但不得通过 SQL 手工改为 + `VERIFIED`/`COMPLETED`，也不得手工迁移身份、角色、membership、凭据或 Token +- 回滚到仍包含旧合并实现的镜像会重新暴露该安全问题；如必须回滚，应先在网关阻断 + `/api/v1/account/merge/*` + +未来安全流程的完整验收条件见 +[`22-secure-account-merge-acceptance-design.md`](./22-secure-account-merge-acceptance-design.md)。 ## 5. CLI 认证（OAuth Device Flow + 平台凭证） diff --git a/docs/10-delivery-roadmap.md b/docs/10-delivery-roadmap.md index f1d5b8c6b..d1beb85bd 100644 --- a/docs/10-delivery-roadmap.md +++ b/docs/10-delivery-roadmap.md @@ -99,7 +99,7 @@ ### 后端 - 本地认证体系（用户名密码注册/登录 + BCrypt + 密码策略 + 账号锁定） -- 多账号合并流程（发起 → 验证 → 确认 → 数据迁移） +- 多账号合并流程（旧实现已因控制权证明不足临时隔离，等待安全重构） - 技能治理（隐藏/恢复 + 已发布版本撤回 YANKED） - 审计日志查询 API（多条件筛选 + 分页） - Prometheus 指标暴露（Actuator + Micrometer 自定义业务指标） @@ -109,7 +109,7 @@ ### 前端 - 注册页、登录页扩展（用户名密码 + OAuth 双模式） -- 密码修改页、账号合并页 +- 密码修改页、账号合并安全隔离提示页 - 审计日志查询页 - 技能隐藏/恢复/已发布版本撤回操作（管理员可见） - 前端代码分割（TanStack Router lazy routes） @@ -125,7 +125,7 @@ ### 验收 -本地认证可用，多账号合并可用，技能隐藏/恢复/已发布版本撤回可用，审计日志可查询，Prometheus 指标可拉取，`docker compose up` 一键启动，K8s 清单可部署，开源基础设施齐全 +本地认证可用；多账号合并在主、次账号独立控制权证明完成前保持关闭；技能隐藏/恢复/已发布版本撤回可用，审计日志可查询，Prometheus 指标可拉取，`docker compose up` 一键启动，K8s 清单可部署，开源基础设施齐全 ## Phase 5：治理闭环 + 社交 diff --git a/docs/22-secure-account-merge-acceptance-design.md b/docs/22-secure-account-merge-acceptance-design.md new file mode 100644 index 000000000..8d8815523 --- /dev/null +++ b/docs/22-secure-account-merge-acceptance-design.md @@ -0,0 +1,461 @@ +# SkillHub 安全账号合并验收设计 + +> 状态：Proposed +> +> 日期：2026-07-30 +> +> 上位设计：[#628 统一身份联邦架构](https://github.com/iflytek/skillhub/issues/628) +> +> P0 隔离：[#634](https://github.com/iflytek/skillhub/issues/634) +> +> 目标实现：统一身份计划 PR 6，必须在统一身份核心与 Provider Adapter 契约稳定后实施 + +## 1. 目的 + +本文定义未来安全 Account Merge 的实现边界和可执行验收条件。它不是 P0 隔离 PR 的实现 +清单，也不授权重新开启旧流程。 + +账号合并的含义是：两个已存在、都有独立登录方式和业务数据的 Platform Account，在分别 +证明控制权后，把允许迁移的数据收敛到主账号，并把次账号永久标记为 `MERGED`。 + +它不同于： + +- **首次登录建号**：外部身份还没有 Platform Account。 +- **Identity Link**：给同一个 Platform Account 增加一种登录方式。 +- **Profile Sync**：同步 display name、email、avatar 等资料字段。 +- **管理员改状态**：启用或禁用账号，不证明另一个账号的控制权。 +- **按 email 去重**：email 不是账号控制权证明，不能触发 Link 或 Merge。 + +## 2. P0 隔离契约 + +安全 Account Merge 上线前必须维持以下行为： + +1. 旧 `/api/v1/account/merge/initiate`、`verify`、`confirm` 路径保留，避免部署客户端因 + 404 误判，但对有效的已认证请求统一返回稳定的 `503 Service Unavailable`。 +2. 旧路径不解析次账号标识、不创建或更新 merge request、不返回 verification token， + 不迁移任何账号数据。 +3. 账号设置页面只显示不可用说明，不展示 username、`provider:subject`、request id、 + verification token 或确认控件。 +4. 现有 `account_merge_request` 表和记录保留；P0 不删除、不完成、不转换旧请求。 +5. 运维人员不得通过 SQL 把旧请求改为 `VERIFIED`/`COMPLETED`，也不得手工修改 + `identity_binding`、`api_token`、平台角色、namespace membership 或本地凭据来模拟合并。 +6. 回滚到旧镜像会恢复不安全实现。若发生必须回滚的故障，应先在 Ingress、网关或反向代理 + 阻断 `/api/v1/account/merge/*`。 + +## 3. 威胁模型 + +### 3.1 需要防御的攻击者 + +- 只控制主账号 Web Session，但知道次账号 username。 +- 只控制主账号 Web Session，但从页面、日志或其他系统知道 + `provider_code:subject`。 +- 窃取了一个旧 merge request id、state、浏览器历史或回调 URL。 +- 可以重放已完成或已过期的 proof。 +- 同时发起多个请求，尝试并发确认同一个合并。 +- 控制一个 OAuth/OIDC Provider 账号，但不控制碰撞 email 对应的平台账号。 +- 可以诱导已登录用户打开跨站页面，但不能读取同源响应。 +- 拥有普通管理员权限，试图把合并当作后台数据修复操作。 + +### 3.2 安全不变量 + +实现和测试必须证明： + +1. 主账号现有 Session 只表示“当前操作人已登录”，不能代替 fresh reauthentication。 +2. 主账号证明和次账号证明来自两个独立认证动作。 +3. 次账号只能由成功认证结果解析，主账号请求不能通过 email、username、 + `provider:subject` 或 userId 指定合并目标。 +4. proof 只保存在服务端，浏览器、API 响应、URL、日志和审计 detail 不出现可直接消费的 + raw proof。 +5. proof 同时绑定 merge request、对应账号、主账号 Session nonce、认证方法、签发时间和 + 短 TTL。 +6. request、state、proof 和 confirmation 都只能消费一次。 +7. 两个账号必须不同、均为 `ACTIVE`、均非 system account、均未 `MERGED`。 +8. 任一冲突、状态变化、过期、重放、Session 不匹配或并发竞争都必须 fail closed。 +9. 合并不会通过角色并集静默提升平台权限。 +10. 次账号 API Token 默认撤销，不迁移成主账号 Token。 +11. 数据库提交后，次账号旧 Web Session 即使尚未从 Redis 删除，也不能继续访问业务接口。 +12. 事务失败时不能留下部分 Binding、角色、membership、凭据、Token 或业务归属迁移。 + +## 4. 核心对象 + +### 4.1 Merge Intent + +服务端创建的一次性合并意图。建议使用不可预测 UUID，至少包含： + +```text +id +primary_user_id +secondary_user_id # 次账号认证成功前为 NULL +status +primary_session_nonce_hash +primary_proof_at +secondary_proof_at +expires_at +preview_version +preview_digest +confirmed_at +completed_at +row_version +created_at +updated_at +``` + +约束： + +- 不保存 raw Session ID、密码、OAuth token、authorization code、CAS Ticket、SAML + assertion 或 raw proof。 +- `primary_session_nonce_hash` 对 Session 中的高熵随机 nonce 做 SHA-256；不直接散列低熵 + 用户输入。 +- `secondary_user_id` 只能由认证成功后的统一身份核心写入。 +- `row_version` 或等价 CAS 条件用于防止并发消费。 +- `preview_digest` 只固定迁移计划，不包含可用于认证的秘密。 + +### 4.2 Fresh Reauthentication + +fresh reauthentication 不是“Session 仍有效”。它必须重新执行当前账号的一种可用登录方式： + +- 本地账号：重新校验密码、锁定和限流策略。 +- OAuth/OIDC：强制新的上游认证交互；不能直接复用当前 Platform Session。 +- LDAP、CAS、DingTalk 等：由对应 Adapter 重新验证协议结果，再进入统一身份核心。 + +主账号 proof 与次账号 proof 默认都在 10 分钟内有效；最终实现可以选择更短 TTL，但不得 +超过普通 Web Session 生命周期，也不能靠续期 Session 自动延长。 + +### 4.3 Session Binding + +创建 intent 时，在当前主账号 Session 生成一个合并专用高熵 nonce： + +```text +HttpSession: raw merge_session_nonce +Database: SHA-256(raw merge_session_nonce) +``` + +所有查询、次账号认证启动、预览、确认和取消都必须同时校验： + +- 当前 `principal.userId == primary_user_id` +- 当前 Session nonce hash 匹配 +- intent 未过期且状态允许该操作 + +换浏览器、换 Session、Session rotation 后未安全迁移 nonce，均不能继续旧 intent。 + +## 5. 必须采用的流程 + +```text +主账号已登录 + → 主账号 fresh reauthentication + → 创建 Merge Intent，并绑定主账号 Session nonce + → 启动独立的次账号认证 + → 统一身份核心由认证事实解析 secondary_user_id + → 服务端把次账号 proof 写入 Intent，不向浏览器返回 raw proof + → 生成冲突预览和 preview digest + → 主账号确认同一 preview version + → 单事务重新校验并迁移 PostgreSQL 数据 + → secondary account = MERGED，撤销 secondary API Token + → 事务提交 + → 每请求账号状态守卫立即拒绝次账号 + → 可靠任务删除次账号 Redis Session + → 审计完成 +``` + +### 5.1 发起 + +- 只接受主账号 Session 和完成的主账号 fresh reauthentication challenge。 +- 请求体不接受 `secondaryUserId`、email、username 或 `provider:subject`。 +- 返回 intent id、可用的次账号认证方式、到期时间；不返回 proof 或推测出的次账号信息。 +- 同一主账号可以只有一个有效 intent，或使用明确的数量限制；重复发起不能绕过限流。 + +### 5.2 次账号认证 + +- 使用独立认证事务，不把次账号 Principal 写入主账号 HttpSession。 +- Browser Provider 的 state 必须绑定 intent 和主账号 Session nonce。 +- 本地密码输入只进入专用 reauthentication 端点，并沿用密码锁定、限流和统一错误。 +- OAuth/OIDC 等回调完成后，只在服务端记录“该 intent 已证明控制 secondary_user_id”。 +- 次账号认证结果与主账号相同、账号不合格或已绑定到另一 intent 时返回通用错误，避免账号 + 枚举。 + +### 5.3 预览 + +预览必须从数据库实时读取，至少列出： + +- 将迁移的 Identity Binding 数量和 Provider 名称。 +- 本地凭据保留/迁移/丢弃策略。 +- 平台角色变化和所有阻塞的高权限角色。 +- Namespace membership 变化及角色。 +- 将撤销的次账号 API Token 名称、prefix 和数量，不返回 hash 或 raw token。 +- Skill ownership 和其他业务资源归属变化。 +- 阻塞冲突以及用户或管理员应先执行的解决动作。 + +预览生成 `preview_version` 和服务端 `preview_digest`。确认时必须重新计算；任何相关数据或 +账号状态变化都使旧预览失效并返回 `409 Conflict`。 + +### 5.4 确认 + +- 只接受创建 intent 的主账号 Session。 +- 主、次账号 proof 均未过期。 +- intent 状态为可确认，preview version 匹配，且没有阻塞冲突。 +- 使用行锁或等价并发控制按稳定顺序锁定两个账号和 intent。 +- 同一 intent 并发确认时只能有一个提交成功；其他请求返回已消费的稳定错误。 + +## 6. 状态机 + +建议状态： + +```text +PENDING_SECONDARY_PROOF + → READY_FOR_PREVIEW + → READY_TO_CONFIRM + → COMPLETED + +任一未完成状态 + → CANCELLED + → EXPIRED + → FAILED_CONFLICT +``` + +规则： + +- 不允许从 `COMPLETED`、`CANCELLED`、`EXPIRED` 返回可执行状态。 +- `FAILED_CONFLICT` 不能直接确认；冲突解决后创建新 preview，必要时创建新 intent。 +- 过期由请求时检查和后台清理共同执行，不能只依赖定时任务。 +- 所有状态迁移使用条件更新或乐观锁，不能“先查后改”而没有数据库竞争保护。 + +## 7. 数据迁移与冲突规则 + +### 7.1 Identity Binding + +- 只迁移 `ACTIVE` Binding。 +- 主账号已经存在同一 Provider Instance 的 ACTIVE Binding 时阻塞，不自动覆盖或选择。 +- typed Subject/Alias 的全局唯一约束必须继续成立。 +- 已撤销 Binding 保留历史归属，不自动复活。 + +### 7.2 本地凭据 + +- 主账号无本地凭据、次账号有本地凭据：可以迁移到主账号。 +- 两个账号都有本地凭据：保留主账号凭据并使次账号凭据失效；该结果必须在预览中明确展示。 +- username 唯一约束、锁定状态和密码策略不能因迁移被绕过。 +- 不复制 password hash，不让同一凭据同时属于两个账号。 + +### 7.3 平台角色 + +- 默认角色无需复制。 +- 次账号拥有任何非默认平台角色时，第一版安全实现应阻塞合并，由平台管理员先显式调整。 +- 禁止使用“角色取并集”作为默认行为，避免普通主账号通过合并获得管理权限。 +- 管理员调整和最终合并分别写审计。 + +### 7.4 Namespace Membership + +- 只有次账号存在 membership：迁移同一角色。 +- 两个账号都有 membership：保留权限更高者，并在预览中展示。 +- 如果迁移会让主账号新获得 `OWNER`，第一版应阻塞；先通过 Namespace 治理流程显式转移 + ownership，再重新发起合并。 +- 必须继续满足 `(namespace_id, user_id)` 唯一约束和 Namespace 最后所有者规则。 + +### 7.5 API Token + +- 次账号所有未撤销 API Token 在数据库事务内统一设置 `revoked_at`。 +- 不把次账号 Token 的 `user_id` 或 `subject_id` 改成主账号。 +- 主账号 Token 不变。 +- 预览只显示非秘密元数据；完成响应不返回任何新 Token。 + +### 7.6 业务数据 + +PR 6 开始前必须建立所有 userId 引用的迁移清单，并分类： + +1. **当前归属**：如 Skill owner，需要迁移。 +2. **当前授权**：如 membership、role、Token，按本设计处理。 +3. **历史事实**：如 audit actor、过去的 reviewer/creator，应保留原 userId，不重写历史。 +4. **通知和临时数据**：明确迁移、失效或删除策略。 + +任何未分类的 userId 外键、字符串引用或 JSON 引用都阻塞发布。不能只迁移旧 +`AccountMergeService` 已知的几张表就宣称完成。 + +## 8. 事务、Session 与跨存储一致性 + +### 8.1 PostgreSQL 事务 + +以下操作必须在同一事务： + +- 锁定和重新校验 intent、主账号、次账号。 +- 重算 preview digest。 +- 迁移允许迁移的 Binding、credential、membership 和业务归属。 +- 撤销次账号 API Token。 +- 把次账号设为 `MERGED` 并写 `merged_to_user_id`。 +- 把 intent 标记为 `COMPLETED`。 +- 写数据库审计或可靠 outbox 事件。 + +任一 repository 失败必须整体回滚。 + +### 8.2 Redis Session + +PostgreSQL 与 Redis 不能依赖普通本地事务实现原子提交，因此需要两层保证： + +1. 所有基于 Web Session 的请求在授权前检查账号仍可登录；数据库已是 `MERGED` 时立即 + 拒绝，即使 Redis 中还存在旧 Principal 快照。 +2. PostgreSQL 提交后，通过可重试的 session revocation 任务按次账号删除所有 indexed + Spring Session。任务失败必须有指标、告警和重试，不能吞掉异常。 + +只有“删 Redis Session”而没有每请求状态守卫，会产生提交到删除之间的继续访问窗口； +只有状态守卫而不删除 Session，会留下长期无效 Session。两者都必须实现。 + +### 8.3 API Token + +Token 在同一个 PostgreSQL 事务撤销。Token Authentication 还必须检查账号状态，因此即使 +个别旧 Token 未被清理，`MERGED` 账号也不能认证成功。 + +## 9. API 与错误语义 + +未来 API 应使用新资源式路径，不复活旧 token 驱动接口。建议： + +```text +POST /api/v1/account/merge/intents +POST /api/v1/account/merge/intents/{id}/secondary-auth/start +GET /api/v1/account/merge/intents/{id} +POST /api/v1/account/merge/intents/{id}/preview +POST /api/v1/account/merge/intents/{id}/confirm +DELETE /api/v1/account/merge/intents/{id} +``` + +协议回调由各 Adapter 的既有 callback transport 处理，最终只调用统一的 server-side proof +完成接口，不把 proof 暴露为公共请求参数。 + +至少定义以下稳定 reason code： + +| reason code | HTTP | 含义 | +|---|---:|---| +| `ACCOUNT_MERGE_UNAVAILABLE` | 503 | 功能尚未安全启用 | +| `MERGE_INTENT_NOT_FOUND` | 404 | 不存在或当前 Session 不可见 | +| `MERGE_REAUTH_REQUIRED` | 401 | 需要重新认证 | +| `MERGE_SESSION_MISMATCH` | 403 | Intent 不属于当前 Session | +| `MERGE_PROOF_EXPIRED` | 410 | 任一 proof 或 intent 已过期 | +| `MERGE_CONFLICT` | 409 | 数据或权限存在阻塞冲突 | +| `MERGE_PREVIEW_STALE` | 409 | 确认的预览已失效 | +| `MERGE_ALREADY_CONSUMED` | 409 | 已完成、取消或并发消费 | +| `MERGE_ACCOUNT_NOT_ELIGIBLE` | 409 | 账号状态不允许合并 | + +面向未证明身份的响应不得泄露目标 userId、完整 email、角色、membership、Provider subject +或账号是否存在。 + +## 10. 审计、日志与指标 + +审计事件至少包括： + +```text +ACCOUNT_MERGE_INTENT_CREATED +ACCOUNT_MERGE_PRIMARY_REAUTHENTICATED +ACCOUNT_MERGE_SECONDARY_REAUTHENTICATED +ACCOUNT_MERGE_PREVIEWED +ACCOUNT_MERGE_CONFIRMED +ACCOUNT_MERGE_COMPLETED +ACCOUNT_MERGE_CANCELLED +ACCOUNT_MERGE_EXPIRED +ACCOUNT_MERGE_REJECTED +ACCOUNT_MERGE_SESSION_REVOCATION_RETRIED +``` + +审计 detail 可以记录 request id、主/次账号内部 ID、Provider code、迁移数量、冲突 reason +code 和结果，但不能记录密码、raw proof、Session ID/nonce、OAuth token、authorization +code、Ticket、SAML assertion、API Token hash 或完整上游响应。 + +指标至少覆盖 intent 创建、proof 成功/失败、冲突、过期、完成、事务回滚和 Session 撤销 +重试；Provider code 可以作为受控低基数标签，userId、request id 和 intent id 不能作为 +指标标签。 + +## 11. 升级、启用与回滚 + +### 11.1 旧请求 + +- 旧 `PENDING`/`VERIFIED` 请求不携带可信的次账号 proof，不能转换为可确认的新 intent。 +- 新版本可以把它们标记为 `LEGACY_BLOCKED`/过期，或保留只读；无论采用哪种方式，都必须 + 保留审计证据，不能自动完成。 +- 用户必须从头执行主、次账号 fresh reauthentication。 + +### 11.2 滚动升级 + +1. 数据库只做 additive migration，新旧 Pod 共存时旧路径仍由网关阻断。 +2. 部署所有包含新实现的 Pod，但新 Account Merge 功能保持关闭。 +3. 验证 schema、Session index、状态守卫、Provider reauthentication 和回滚脚本。 +4. 确认没有旧 Pod 后再打开新资源式 API 和 UI。 +5. 旧 initiate/verify/confirm 路径继续返回 503，不重定向到新确认接口。 + +### 11.3 回滚 + +- 关闭新 UI 和新 intent 创建。 +- 已 `COMPLETED` 的合并不能靠镜像回滚自动拆分；拆分账号需要独立、人工审核的数据恢复 + 流程。 +- 未完成的新 intent 可统一失效。 +- 数据库 additive 字段和表保留，不在应用回滚时删除。 +- 不得回滚到会重新开放旧 token 流程的版本；如果镜像回滚不可避免，网关阻断规则必须先 + 生效。 + +## 12. 自动化验收矩阵 + +### 12.1 控制权证明 + +- 只有主账号 Session，无法产生次账号 proof。 +- email、username、display name、userId、`provider:subject` 均不能替代次账号认证。 +- 次账号密码错误、锁定、禁用、待审批、已合并或 system account 时失败。 +- OAuth/OIDC state 不能用于另一个 intent 或另一个主账号 Session。 +- proof、Session nonce、callback 和 confirmation 不出现在日志或审计 detail。 + +### 12.2 过期与重放 + +- 主 proof 过期、次 proof 过期、intent 过期分别 fail closed。 +- 已完成、取消、过期的 intent 无法恢复。 +- 同一 callback 重放、同一 confirmation 重放都失败。 +- 两个线程同时确认，只有一个成功，另一个得到 `MERGE_ALREADY_CONSUMED`。 + +### 12.3 冲突与权限 + +- 两账号相同、状态变化、system account、`MERGED` 均失败。 +- 同 Provider Binding 冲突不覆盖。 +- 非默认平台角色阻塞，不执行角色并集。 +- Namespace OWNER 新增冲突阻塞。 +- preview 后新增 Token、Binding、membership 或业务归属会让 preview stale。 + +### 12.4 原子性 + +在 Binding、credential、membership、业务归属、Token 撤销、账号状态、intent 状态和审计 +每一步注入失败，验证所有 PostgreSQL 数据回到事务前状态。 + +### 12.5 Session 与 Token + +- 合并提交后，次账号现有 Web Session 下一次请求立即被拒绝。 +- Redis 删除失败时，请求仍被状态守卫拒绝，并产生重试任务和指标。 +- 重试成功后次账号所有 indexed Session 消失。 +- 次账号所有 API Token 被撤销，旧 Token 返回 401；主账号 Token 不受影响。 + +### 12.6 升级与回滚 + +- 从包含旧 `account_merge_request` 数据的版本升级，旧请求不能确认。 +- 新旧 Pod 混跑期间旧路径始终被阻断。 +- 关闭功能后不能新建 intent，已有未完成 intent 按策略失效。 +- 应用版本回滚不删除新表，也不让旧 token 流程恢复可用。 + +## 13. 测试环境人工验收 + +在 `big-main` 测试镜像完成自动化检查后，人工至少验证： + +1. 旧三个接口：未登录为 401；缺少有效 CSRF 时由现有安全链以 4xx 拒绝（当前实现为 + 401）；已登录、CSRF 有效且请求合法时统一返回 503。 +2. 账号设置页只显示中英文不可用说明，没有 identifier、request id、token 和确认按钮。 +3. 普通本地登录、OAuth 登录、`/api/v1/auth/me`、Namespace 列表和 Skill 浏览不受影响。 +4. 数据库已有 `account_merge_request` 数量和内容未被 P0 隔离部署修改。 +5. 日志只包含稳定错误和 request id，不包含请求中的 secondary identifier 或 token。 +6. 部署和回滚文档明确要求旧路径网关阻断；不得通过数据库手工演示“成功合并”。 + +PR 6 上线时，再执行本文件第 12 节的完整双账号、Redis Session、API Token、冲突、并发和 +事务回滚验收。 + +## 14. 合并门禁 + +PR 6 只有同时满足以下条件才可以从 `big-main` 进入 `main`： + +- 统一身份核心、Binding V2、Provider Registry/Adapter 契约已经稳定。 +- 独立 Identity Link 已经证明 fresh reauthentication、一次性 request 和重放保护可用。 +- 本文件所有自动化验收项有对应测试和可追溯结果。 +- PostgreSQL + Redis 真实集成测试通过。 +- 测试环境完成双账号人工验收，记录镜像 tag、commit、请求结果和数据前后快照。 +- 安全 Review 没有 Blocker/Critical 发现。 +- 运维、升级和回滚文档同步完成。 + +不满足任一门禁时，继续保持 Account Merge 不可用；不得以管理员手工操作作为替代方案。 diff --git a/docs/skillhub/en/faq.md b/docs/skillhub/en/faq.md index 7cc4500f1..5b6c8dd70 100644 --- a/docs/skillhub/en/faq.md +++ b/docs/skillhub/en/faq.md @@ -323,6 +323,21 @@ xargs -a skills.txt -I {} skillhub install "{}" --dir "$target_dir" Since **SkillHub Server v0.2.12**, public skills support anonymous search and install. Note that an invalid bearer token now fails the command instead of falling back to anonymous access — update or remove the stale credential in that case. +## Q: Why does the account merge page say that merging is temporarily unavailable? + +A: The legacy account merge flow could not independently prove control of the primary and +secondary accounts, so it has been isolated as a security measure. Until the replacement +double-reauthentication flow is available: + +- Continue using the two accounts separately. +- Do not ask an administrator to edit the database or manually move identity bindings, roles, + namespace memberships, local credentials, or API tokens. +- Existing `account_merge_request` rows are retained but cannot be completed. +- Authenticated API clients calling the legacy routes receive `503 Service Unavailable`. + +Normal login, existing identity binding lookup, namespace operations, and skill operations are not +affected. + ## Q: What should I do if I encounter issues? A: You can get help through the following channels: diff --git a/docs/skillhub/faq.md b/docs/skillhub/faq.md index b16ecd81a..a9de4e5e9 100644 --- a/docs/skillhub/faq.md +++ b/docs/skillhub/faq.md @@ -323,6 +323,19 @@ xargs -a skills.txt -I {} skillhub install "{}" --dir "$target_dir" 自 **SkillHub Server v0.2.12** 起，公开技能支持匿名搜索与安装；如果配置了无效的 Bearer Token，命令会直接失败而不再回退匿名访问，遇到这种情况请更新凭据或先移除无效 Token。 +## Q: 为什么账号合并页面显示暂时不可用？ + +A: 旧的账号合并流程不能分别证明主账号和次账号的控制权，因此已被安全隔离。在新的 +双重重新认证流程上线前： + +- 请继续分别使用两个账号。 +- 不要让管理员直接修改数据库、移动 identity binding、角色、namespace membership、 + 本地凭据或 API Token。 +- 已存在的 `account_merge_request` 记录会被保留，但不会继续执行。 +- 如果 API 客户端仍调用旧接口，已认证请求会收到 `503 Service Unavailable`。 + +这不会影响普通登录、身份绑定读取、Namespace 或 Skill 操作。 + ## Q: 遇到问题怎么办？ A: 可以通过以下方式获取帮助： diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AccountMergeController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AccountMergeController.java index b7e1386e1..c196af72b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AccountMergeController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/AccountMergeController.java @@ -1,6 +1,6 @@ package com.iflytek.skillhub.controller; -import com.iflytek.skillhub.auth.merge.AccountMergeService; +import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; @@ -10,6 +10,7 @@ import com.iflytek.skillhub.dto.MessageResponse; import com.iflytek.skillhub.exception.UnauthorizedException; import jakarta.validation.Valid; +import org.springframework.http.HttpStatus; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -17,19 +18,19 @@ import org.springframework.web.bind.annotation.RestController; /** - * Endpoints for initiating, verifying, and confirming account merge flows - * across multiple identities owned by the same user. + * Compatibility endpoints for the temporarily isolated legacy account merge flow. + * + *

The previous implementation returned the secondary-account verification token to the + * primary-account session and therefore did not prove independent control of both accounts. Keep + * the routes stable for deployed clients, but fail closed until the safe account merge flow is + * implemented. */ @RestController @RequestMapping("/api/v1/account/merge") public class AccountMergeController extends BaseApiController { - private final AccountMergeService accountMergeService; - - public AccountMergeController(ApiResponseFactory responseFactory, - AccountMergeService accountMergeService) { + public AccountMergeController(ApiResponseFactory responseFactory) { super(responseFactory); - this.accountMergeService = accountMergeService; } @PostMapping("/initiate") @@ -38,13 +39,7 @@ public ApiResponse initiate(@AuthenticationPrincipal Plat if (principal == null) { throw new UnauthorizedException("error.auth.required"); } - var result = accountMergeService.initiate(principal.userId(), request.secondaryIdentifier()); - return ok("response.success.created", new MergeInitiateResponse( - result.mergeRequestId(), - result.secondaryUserId(), - result.verificationToken(), - result.expiresAt().toString() - )); + throw mergeTemporarilyUnavailable(); } @PostMapping("/verify") @@ -53,12 +48,7 @@ public ApiResponse verify(@AuthenticationPrincipal PlatformPrin if (principal == null) { throw new UnauthorizedException("error.auth.required"); } - accountMergeService.verify( - principal.userId(), - request.mergeRequestId(), - request.verificationToken() - ); - return ok("response.success.updated", new MessageResponse("Account merge verified")); + throw mergeTemporarilyUnavailable(); } @PostMapping("/confirm") @@ -67,9 +57,15 @@ public ApiResponse confirm(@AuthenticationPrincipal PlatformPri if (principal == null) { throw new UnauthorizedException("error.auth.required"); } - accountMergeService.confirm(principal.userId(), request.mergeRequestId()); - return ok("response.success.updated", new MessageResponse("Account merge completed")); + throw mergeTemporarilyUnavailable(); } public record ConfirmMergeRequest(@jakarta.validation.constraints.NotNull Long mergeRequestId) {} + + private AuthFlowException mergeTemporarilyUnavailable() { + return new AuthFlowException( + HttpStatus.SERVICE_UNAVAILABLE, + "error.auth.merge.temporarilyUnavailable" + ); + } } diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 4661a4eba..c7d873e47 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -46,6 +46,7 @@ error.auth.direct.providerUnsupported=Unsupported direct authentication provider error.auth.sessionBootstrap.disabled=Session bootstrap is disabled error.auth.sessionBootstrap.providerUnsupported=Unsupported session bootstrap provider: {0} error.auth.sessionBootstrap.notAuthenticated=No authenticated external session found +error.auth.merge.temporarilyUnavailable=Account merging is temporarily unavailable while the ownership verification flow is being secured error.badRequest=Invalid request error.forbidden=Forbidden error.apiToken.scope.missing=API token is missing required scope: {0} diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 412ec7bac..82ec60013 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -46,6 +46,7 @@ error.auth.direct.providerUnsupported=不支持的直连认证提供方：{0} error.auth.sessionBootstrap.disabled=会话引导能力未启用 error.auth.sessionBootstrap.providerUnsupported=不支持的会话引导提供方：{0} error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会话 +error.auth.merge.temporarilyUnavailable=账号合并功能正在进行安全升级，暂时不可用 error.badRequest=请求参数不合法 error.forbidden=没有权限执行该操作 error.apiToken.scope.missing=API 令牌缺少所需权限范围：{0} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AccountMergeControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AccountMergeControllerTest.java index e0d5677bc..c97a803fb 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AccountMergeControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/AccountMergeControllerTest.java @@ -1,18 +1,15 @@ package com.iflytek.skillhub.controller; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.verify; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import com.iflytek.skillhub.auth.merge.AccountMergeService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import java.time.Instant; import java.util.List; +import java.util.Locale; import java.util.Set; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -21,7 +18,6 @@ import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; @@ -33,69 +29,94 @@ class AccountMergeControllerTest { @Autowired private MockMvc mockMvc; - @MockBean - private AccountMergeService accountMergeService; - @MockBean private NamespaceMemberRepository namespaceMemberRepository; @Test - void initiate_returnsVerificationToken() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal("usr_primary", "primary", "p@example.com", "", "local", Set.of()); - var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of()); - given(accountMergeService.initiate("usr_primary", "secondary")) - .willReturn(new AccountMergeService.InitiationResult(1L, "usr_secondary", "merge-token", Instant.parse("2026-03-12T22:30:00Z"))); - + void initiate_failsClosedWithoutReturningSecondaryAccountProof() throws Exception { mockMvc.perform(post("/api/v1/account/merge/initiate") - .with(authentication(auth)) + .with(authentication(primaryAuthentication())) .with(csrf()) + .locale(Locale.ENGLISH) .contentType(MediaType.APPLICATION_JSON) .content(""" {"secondaryIdentifier":"secondary"} """)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.mergeRequestId").value(1)) - .andExpect(jsonPath("$.data.secondaryUserId").value("usr_secondary")) - .andExpect(jsonPath("$.data.verificationToken").value("merge-token")) - .andExpect(jsonPath("$.data.expiresAt").value("2026-03-12T22:30:00Z")); + .andExpect(status().isServiceUnavailable()) + .andExpect(jsonPath("$.code").value(503)) + .andExpect(jsonPath("$.msg").value( + "Account merging is temporarily unavailable while the ownership verification flow is being secured" + )) + .andExpect(jsonPath("$.data.verificationToken").doesNotExist()) + .andExpect(jsonPath("$.data.secondaryUserId").doesNotExist()) + .andExpect(jsonPath("$.data.mergeRequestId").doesNotExist()); } @Test - void verify_returnsSuccessMessage() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal("usr_primary", "primary", "p@example.com", "", "local", Set.of()); - var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"))); - + void verify_failsClosedWithTheSameStableError() throws Exception { mockMvc.perform(post("/api/v1/account/merge/verify") - .with(authentication(auth)) + .with(authentication(primaryAuthentication())) .with(csrf()) + .locale(Locale.ENGLISH) .contentType(MediaType.APPLICATION_JSON) .content(""" {"mergeRequestId":1,"verificationToken":"merge-token"} """)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.message").value("Account merge verified")); - - verify(accountMergeService).verify("usr_primary", 1L, "merge-token"); + .andExpect(status().isServiceUnavailable()) + .andExpect(jsonPath("$.code").value(503)) + .andExpect(jsonPath("$.msg").value( + "Account merging is temporarily unavailable while the ownership verification flow is being secured" + )); } @Test - void confirm_returnsSuccessMessage() throws Exception { - PlatformPrincipal principal = new PlatformPrincipal("usr_primary", "primary", "p@example.com", "", "local", Set.of()); - var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"))); - + void confirm_failsClosedWithTheSameStableError() throws Exception { mockMvc.perform(post("/api/v1/account/merge/confirm") - .with(authentication(auth)) + .with(authentication(primaryAuthentication())) .with(csrf()) + .locale(Locale.ENGLISH) .contentType(MediaType.APPLICATION_JSON) .content(""" {"mergeRequestId":1} """)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.code").value(0)) - .andExpect(jsonPath("$.data.message").value("Account merge completed")); + .andExpect(status().isServiceUnavailable()) + .andExpect(jsonPath("$.code").value(503)) + .andExpect(jsonPath("$.msg").value( + "Account merging is temporarily unavailable while the ownership verification flow is being secured" + )); + } + + @Test + void initiate_withoutCsrf_isRejectedByTheExistingSecurityChain() throws Exception { + mockMvc.perform(post("/api/v1/account/merge/initiate") + .with(authentication(primaryAuthentication())) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"secondaryIdentifier":"secondary"} + """)) + .andExpect(status().isUnauthorized()); + } + + @Test + void initiate_withoutAuthentication_remainsUnauthorized() throws Exception { + mockMvc.perform(post("/api/v1/account/merge/initiate") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"secondaryIdentifier":"secondary"} + """)) + .andExpect(status().isUnauthorized()); + } - verify(accountMergeService).confirm("usr_primary", 1L); + private UsernamePasswordAuthenticationToken primaryAuthentication() { + PlatformPrincipal principal = new PlatformPrincipal( + "usr_primary", + "primary", + "p@example.com", + "", + "local", + Set.of() + ); + return new UsernamePasswordAuthenticationToken(principal, null, List.of()); } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/merge/AccountMergeService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/merge/AccountMergeService.java deleted file mode 100644 index 20fc40802..000000000 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/merge/AccountMergeService.java +++ /dev/null @@ -1,288 +0,0 @@ -package com.iflytek.skillhub.auth.merge; - -import com.iflytek.skillhub.auth.entity.ApiToken; -import com.iflytek.skillhub.auth.entity.IdentityBinding; -import com.iflytek.skillhub.auth.entity.Role; -import com.iflytek.skillhub.auth.entity.UserRoleBinding; -import com.iflytek.skillhub.auth.exception.AuthFlowException; -import com.iflytek.skillhub.auth.local.LocalCredential; -import com.iflytek.skillhub.auth.local.LocalCredentialRepository; -import com.iflytek.skillhub.auth.repository.ApiTokenRepository; -import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; -import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceMember; -import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.user.UserAccount; -import com.iflytek.skillhub.domain.user.UserAccountRepository; -import com.iflytek.skillhub.domain.user.UserStatus; -import java.security.SecureRandom; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.util.Base64; -import java.util.Comparator; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Optional; -import java.util.Set; -import org.springframework.http.HttpStatus; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; - -/** - * Coordinates account merge requests and consolidates credentials, bindings, - * roles, memberships, and tokens into a single primary user. - */ -@Service -public class AccountMergeService { - - private static final Comparator NAMESPACE_ROLE_ORDER = Comparator.comparingInt(role -> switch (role) { - case MEMBER -> 0; - case ADMIN -> 1; - case OWNER -> 2; - }); - - private final AccountMergeRequestRepository mergeRequestRepository; - private final UserAccountRepository userAccountRepository; - private final LocalCredentialRepository localCredentialRepository; - private final IdentityBindingRepository identityBindingRepository; - private final UserRoleBindingRepository userRoleBindingRepository; - private final ApiTokenRepository apiTokenRepository; - private final NamespaceMemberRepository namespaceMemberRepository; - private final PasswordEncoder passwordEncoder; - private final Clock clock; - private final SecureRandom secureRandom = new SecureRandom(); - - public AccountMergeService(AccountMergeRequestRepository mergeRequestRepository, - UserAccountRepository userAccountRepository, - LocalCredentialRepository localCredentialRepository, - IdentityBindingRepository identityBindingRepository, - UserRoleBindingRepository userRoleBindingRepository, - ApiTokenRepository apiTokenRepository, - NamespaceMemberRepository namespaceMemberRepository, - PasswordEncoder passwordEncoder, - Clock clock) { - this.mergeRequestRepository = mergeRequestRepository; - this.userAccountRepository = userAccountRepository; - this.localCredentialRepository = localCredentialRepository; - this.identityBindingRepository = identityBindingRepository; - this.userRoleBindingRepository = userRoleBindingRepository; - this.apiTokenRepository = apiTokenRepository; - this.namespaceMemberRepository = namespaceMemberRepository; - this.passwordEncoder = passwordEncoder; - this.clock = clock; - } - - public record InitiationResult(Long mergeRequestId, String secondaryUserId, String verificationToken, Instant expiresAt) {} - - @Transactional - public InitiationResult initiate(String primaryUserId, String secondaryIdentifier) { - UserAccount primaryUser = loadActiveUser(primaryUserId); - UserAccount secondaryUser = resolveSecondaryUser(secondaryIdentifier); - validateMergePair(primaryUser, secondaryUser); - - if (mergeRequestRepository.existsBySecondaryUserIdAndStatus( - secondaryUser.getId(), - AccountMergeRequest.STATUS_PENDING - )) { - throw new AuthFlowException(HttpStatus.CONFLICT, "error.auth.merge.pendingExists"); - } - - Optional primaryCredential = localCredentialRepository.findByUserId(primaryUserId); - Optional secondaryCredential = localCredentialRepository.findByUserId(secondaryUser.getId()); - if (primaryCredential.isPresent() && secondaryCredential.isPresent()) { - throw new AuthFlowException(HttpStatus.CONFLICT, "error.auth.merge.localCredentialConflict"); - } - - String rawToken = generateVerificationToken(); - AccountMergeRequest request = new AccountMergeRequest( - primaryUserId, - secondaryUser.getId(), - passwordEncoder.encode(rawToken), - currentTime().plus(Duration.ofMinutes(30)) - ); - request = mergeRequestRepository.save(request); - return new InitiationResult(request.getId(), secondaryUser.getId(), rawToken, request.getTokenExpiresAt()); - } - - @Transactional - public void verify(String primaryUserId, Long mergeRequestId, String verificationToken) { - AccountMergeRequest request = mergeRequestRepository.findByIdAndPrimaryUserId(mergeRequestId, primaryUserId) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.requestNotFound")); - if (!AccountMergeRequest.STATUS_PENDING.equals(request.getStatus())) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.requestNotPending"); - } - if (request.getTokenExpiresAt() == null || request.getTokenExpiresAt().isBefore(currentTime())) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.tokenExpired"); - } - if (!passwordEncoder.matches(verificationToken, request.getVerificationToken())) { - throw new AuthFlowException(HttpStatus.UNAUTHORIZED, "error.auth.merge.invalidToken"); - } - - loadActiveUser(primaryUserId); - UserAccount secondaryUser = userAccountRepository.findById(request.getSecondaryUserId()) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - validateMergePair(loadActiveUser(primaryUserId), secondaryUser); - - request.setStatus(AccountMergeRequest.STATUS_VERIFIED); - mergeRequestRepository.save(request); - } - - @Transactional - public void confirm(String primaryUserId, Long mergeRequestId) { - AccountMergeRequest request = mergeRequestRepository.findByIdAndPrimaryUserId(mergeRequestId, primaryUserId) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.requestNotFound")); - if (!AccountMergeRequest.STATUS_VERIFIED.equals(request.getStatus())) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.requestNotVerified"); - } - - UserAccount primaryUser = loadActiveUser(primaryUserId); - UserAccount secondaryUser = userAccountRepository.findById(request.getSecondaryUserId()) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - validateMergePair(primaryUser, secondaryUser); - - migrateIdentityBindings(primaryUser.getId(), secondaryUser.getId()); - migrateApiTokens(primaryUser.getId(), secondaryUser.getId()); - migrateUserRoles(primaryUser.getId(), secondaryUser.getId()); - migrateNamespaceMemberships(primaryUser.getId(), secondaryUser.getId()); - migrateLocalCredential(primaryUser.getId(), secondaryUser.getId()); - - if ((primaryUser.getEmail() == null || primaryUser.getEmail().isBlank()) - && secondaryUser.getEmail() != null && !secondaryUser.getEmail().isBlank()) { - primaryUser.setEmail(secondaryUser.getEmail()); - } - userAccountRepository.save(primaryUser); - - secondaryUser.setStatus(UserStatus.MERGED); - secondaryUser.setMergedToUserId(primaryUser.getId()); - userAccountRepository.save(secondaryUser); - - request.setStatus(AccountMergeRequest.STATUS_COMPLETED); - request.setCompletedAt(currentTime()); - request.setVerificationToken(null); - mergeRequestRepository.save(request); - } - - private UserAccount resolveSecondaryUser(String identifier) { - String normalized = identifier == null ? "" : identifier.trim(); - if (normalized.isBlank()) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.identifierRequired"); - } - if (normalized.contains(":")) { - String[] parts = normalized.split(":", 2); - if (parts.length != 2 || parts[0].isBlank() || parts[1].isBlank()) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.identifierInvalid"); - } - IdentityBinding binding = identityBindingRepository.findByProviderCodeAndSubject(parts[0], parts[1]) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - return userAccountRepository.findById(binding.getUserId()) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - } - - LocalCredential credential = localCredentialRepository.findByUsernameIgnoreCase(normalized.toLowerCase(Locale.ROOT)) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - return userAccountRepository.findById(credential.getUserId()) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.secondaryNotFound")); - } - - private UserAccount loadActiveUser(String userId) { - UserAccount user = userAccountRepository.findById(userId) - .orElseThrow(() -> new AuthFlowException(HttpStatus.NOT_FOUND, "error.auth.merge.primaryNotFound")); - if (user.getStatus() != UserStatus.ACTIVE) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.primaryNotActive"); - } - return user; - } - - private void validateMergePair(UserAccount primaryUser, UserAccount secondaryUser) { - if (primaryUser.getId().equals(secondaryUser.getId())) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.sameAccount"); - } - if (secondaryUser.getStatus() != UserStatus.ACTIVE) { - throw new AuthFlowException(HttpStatus.BAD_REQUEST, "error.auth.merge.secondaryNotActive"); - } - } - - private void migrateIdentityBindings(String primaryUserId, String secondaryUserId) { - List bindings = identityBindingRepository.findByUserId(secondaryUserId); - for (IdentityBinding binding : bindings) { - binding.setUserId(primaryUserId); - } - identityBindingRepository.saveAll(bindings); - } - - private void migrateApiTokens(String primaryUserId, String secondaryUserId) { - List tokens = apiTokenRepository.findByUserId(secondaryUserId); - for (ApiToken token : tokens) { - token.setUserId(primaryUserId); - if ("USER".equals(token.getSubjectType())) { - token.setSubjectId(primaryUserId); - } - } - apiTokenRepository.saveAll(tokens); - } - - private void migrateUserRoles(String primaryUserId, String secondaryUserId) { - Set primaryRoleCodes = new HashSet<>(); - for (UserRoleBinding binding : userRoleBindingRepository.findByUserId(primaryUserId)) { - primaryRoleCodes.add(binding.getRole().getCode()); - } - - List secondaryBindings = userRoleBindingRepository.findByUserId(secondaryUserId); - for (UserRoleBinding binding : secondaryBindings) { - Role role = binding.getRole(); - if (!primaryRoleCodes.contains(role.getCode())) { - userRoleBindingRepository.save(new UserRoleBinding(primaryUserId, role)); - primaryRoleCodes.add(role.getCode()); - } - } - userRoleBindingRepository.deleteAll(secondaryBindings); - } - - private void migrateNamespaceMemberships(String primaryUserId, String secondaryUserId) { - List secondaryMemberships = namespaceMemberRepository.findByUserId(secondaryUserId); - for (NamespaceMember secondaryMembership : secondaryMemberships) { - Optional existingPrimaryMembership = namespaceMemberRepository - .findByNamespaceIdAndUserId(secondaryMembership.getNamespaceId(), primaryUserId); - if (existingPrimaryMembership.isPresent()) { - NamespaceMember primaryMembership = existingPrimaryMembership.get(); - if (NAMESPACE_ROLE_ORDER.compare(secondaryMembership.getRole(), primaryMembership.getRole()) > 0) { - primaryMembership.setRole(secondaryMembership.getRole()); - namespaceMemberRepository.save(primaryMembership); - } - namespaceMemberRepository.deleteByNamespaceIdAndUserId( - secondaryMembership.getNamespaceId(), - secondaryUserId - ); - } else { - secondaryMembership.setUserId(primaryUserId); - namespaceMemberRepository.save(secondaryMembership); - } - } - } - - private void migrateLocalCredential(String primaryUserId, String secondaryUserId) { - Optional primaryCredential = localCredentialRepository.findByUserId(primaryUserId); - Optional secondaryCredential = localCredentialRepository.findByUserId(secondaryUserId); - if (primaryCredential.isPresent() && secondaryCredential.isPresent()) { - throw new AuthFlowException(HttpStatus.CONFLICT, "error.auth.merge.localCredentialConflict"); - } - secondaryCredential.ifPresent(credential -> { - credential.setUserId(primaryUserId); - localCredentialRepository.save(credential); - }); - } - - private String generateVerificationToken() { - byte[] tokenBytes = new byte[24]; - secureRandom.nextBytes(tokenBytes); - return Base64.getUrlEncoder().withoutPadding().encodeToString(tokenBytes); - } - - private Instant currentTime() { - return Instant.now(clock); - } -} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/merge/AccountMergeServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/merge/AccountMergeServiceTest.java deleted file mode 100644 index d882386d5..000000000 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/merge/AccountMergeServiceTest.java +++ /dev/null @@ -1,185 +0,0 @@ -package com.iflytek.skillhub.auth.merge; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; - -import com.iflytek.skillhub.auth.entity.ApiToken; -import com.iflytek.skillhub.auth.entity.IdentityBinding; -import com.iflytek.skillhub.auth.entity.Role; -import com.iflytek.skillhub.auth.entity.UserRoleBinding; -import com.iflytek.skillhub.auth.exception.AuthFlowException; -import com.iflytek.skillhub.auth.local.LocalCredential; -import com.iflytek.skillhub.auth.local.LocalCredentialRepository; -import com.iflytek.skillhub.auth.repository.ApiTokenRepository; -import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; -import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceMember; -import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; -import com.iflytek.skillhub.domain.namespace.NamespaceRole; -import com.iflytek.skillhub.domain.user.UserAccount; -import com.iflytek.skillhub.domain.user.UserAccountRepository; -import java.lang.reflect.Field; -import java.time.Clock; -import java.time.Instant; -import java.time.ZoneOffset; -import java.util.List; -import java.util.Optional; -import org.junit.jupiter.api.BeforeEach; -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.security.crypto.password.PasswordEncoder; - -@ExtendWith(MockitoExtension.class) -class AccountMergeServiceTest { - - @Mock - private AccountMergeRequestRepository mergeRequestRepository; - @Mock - private UserAccountRepository userAccountRepository; - @Mock - private LocalCredentialRepository localCredentialRepository; - @Mock - private IdentityBindingRepository identityBindingRepository; - @Mock - private UserRoleBindingRepository userRoleBindingRepository; - @Mock - private ApiTokenRepository apiTokenRepository; - @Mock - private NamespaceMemberRepository namespaceMemberRepository; - @Mock - private PasswordEncoder passwordEncoder; - - private AccountMergeService service; - private Clock clock; - - @BeforeEach - void setUp() { - clock = Clock.fixed(Instant.parse("2026-03-18T00:00:00Z"), ZoneOffset.UTC); - service = new AccountMergeService( - mergeRequestRepository, - userAccountRepository, - localCredentialRepository, - identityBindingRepository, - userRoleBindingRepository, - apiTokenRepository, - namespaceMemberRepository, - passwordEncoder, - clock - ); - } - - @Test - void initiate_withLocalUsername_createsPendingRequest() { - UserAccount primary = new UserAccount("usr_primary", "primary", "primary@example.com", null); - UserAccount secondary = new UserAccount("usr_secondary", "secondary", "secondary@example.com", null); - LocalCredential secondaryCredential = new LocalCredential("usr_secondary", "secondary", "hash"); - given(userAccountRepository.findById("usr_primary")).willReturn(Optional.of(primary)); - given(localCredentialRepository.findByUsernameIgnoreCase("secondary")).willReturn(Optional.of(secondaryCredential)); - given(userAccountRepository.findById("usr_secondary")).willReturn(Optional.of(secondary)); - given(mergeRequestRepository.existsBySecondaryUserIdAndStatus("usr_secondary", AccountMergeRequest.STATUS_PENDING)) - .willReturn(false); - given(localCredentialRepository.findByUserId("usr_primary")).willReturn(Optional.empty()); - given(localCredentialRepository.findByUserId("usr_secondary")).willReturn(Optional.of(secondaryCredential)); - given(passwordEncoder.encode(any())).willReturn("encoded-token"); - given(mergeRequestRepository.save(any(AccountMergeRequest.class))).willAnswer(invocation -> invocation.getArgument(0)); - - var result = service.initiate("usr_primary", "secondary"); - - assertThat(result.secondaryUserId()).isEqualTo("usr_secondary"); - assertThat(result.verificationToken()).isNotBlank(); - assertThat(result.expiresAt()).isEqualTo(Instant.parse("2026-03-18T00:30:00Z")); - verify(mergeRequestRepository).save(any(AccountMergeRequest.class)); - } - - @Test - void verify_marksRequestVerifiedWhenTokenMatches() throws Exception { - UserAccount primary = new UserAccount("usr_primary", "primary", "primary@example.com", null); - UserAccount secondary = new UserAccount("usr_secondary", "secondary", "", null); - AccountMergeRequest request = request("usr_primary", "usr_secondary", "encoded"); - - given(mergeRequestRepository.findByIdAndPrimaryUserId(7L, "usr_primary")).willReturn(Optional.of(request)); - given(userAccountRepository.findById("usr_primary")).willReturn(Optional.of(primary)); - given(userAccountRepository.findById("usr_secondary")).willReturn(Optional.of(secondary)); - given(passwordEncoder.matches("raw-token", "encoded")).willReturn(true); - given(mergeRequestRepository.save(any(AccountMergeRequest.class))).willAnswer(invocation -> invocation.getArgument(0)); - - service.verify("usr_primary", 7L, "raw-token"); - - assertThat(request.getStatus()).isEqualTo(AccountMergeRequest.STATUS_VERIFIED); - verify(mergeRequestRepository).save(request); - } - - @Test - void confirm_migratesBindingsRolesTokensAndMemberships() throws Exception { - UserAccount primary = new UserAccount("usr_primary", "primary", "primary@example.com", null); - UserAccount secondary = new UserAccount("usr_secondary", "secondary", "", null); - AccountMergeRequest request = request("usr_primary", "usr_secondary", "encoded"); - request.setStatus(AccountMergeRequest.STATUS_VERIFIED); - Role role = mock(Role.class); - given(role.getCode()).willReturn("AUDITOR"); - UserRoleBinding secondaryRole = new UserRoleBinding("usr_secondary", role); - IdentityBinding binding = new IdentityBinding("usr_secondary", "github", "gh_123", "secondary"); - ApiToken token = new ApiToken("usr_secondary", "cli", "sk_123", "hash", "[]"); - NamespaceMember secondaryMembership = new NamespaceMember(1L, "usr_secondary", NamespaceRole.ADMIN); - - given(mergeRequestRepository.findByIdAndPrimaryUserId(7L, "usr_primary")).willReturn(Optional.of(request)); - given(userAccountRepository.findById("usr_primary")).willReturn(Optional.of(primary)); - given(userAccountRepository.findById("usr_secondary")).willReturn(Optional.of(secondary)); - given(mergeRequestRepository.save(any(AccountMergeRequest.class))).willAnswer(invocation -> invocation.getArgument(0)); - given(identityBindingRepository.findByUserId("usr_secondary")).willReturn(List.of(binding)); - given(apiTokenRepository.findByUserId("usr_secondary")).willReturn(List.of(token)); - given(userRoleBindingRepository.findByUserId("usr_primary")).willReturn(List.of()); - given(userRoleBindingRepository.findByUserId("usr_secondary")).willReturn(List.of(secondaryRole)); - given(namespaceMemberRepository.findByUserId("usr_secondary")).willReturn(List.of(secondaryMembership)); - given(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "usr_primary")).willReturn(Optional.empty()); - given(localCredentialRepository.findByUserId("usr_primary")).willReturn(Optional.empty()); - given(localCredentialRepository.findByUserId("usr_secondary")).willReturn(Optional.empty()); - - service.confirm("usr_primary", 7L); - - assertThat(binding.getUserId()).isEqualTo("usr_primary"); - assertThat(token.getUserId()).isEqualTo("usr_primary"); - assertThat(token.getSubjectId()).isEqualTo("usr_primary"); - assertThat(secondaryMembership.getUserId()).isEqualTo("usr_primary"); - assertThat(secondary.getStatus()).isEqualTo(com.iflytek.skillhub.domain.user.UserStatus.MERGED); - assertThat(secondary.getMergedToUserId()).isEqualTo("usr_primary"); - assertThat(request.getStatus()).isEqualTo(AccountMergeRequest.STATUS_COMPLETED); - assertThat(request.getCompletedAt()).isEqualTo(Instant.parse("2026-03-18T00:00:00Z")); - assertThat(request.getVerificationToken()).isNull(); - verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); - verify(userRoleBindingRepository).deleteAll(List.of(secondaryRole)); - } - - @Test - void verify_rejectsInvalidToken() throws Exception { - AccountMergeRequest request = request("usr_primary", "usr_secondary", "encoded"); - given(mergeRequestRepository.findByIdAndPrimaryUserId(7L, "usr_primary")).willReturn(Optional.of(request)); - given(passwordEncoder.matches("bad-token", "encoded")).willReturn(false); - - assertThatThrownBy(() -> service.verify("usr_primary", 7L, "bad-token")) - .isInstanceOf(AuthFlowException.class) - .hasMessageContaining("error.auth.merge.invalidToken"); - - verify(identityBindingRepository, never()).saveAll(any()); - } - - private AccountMergeRequest request(String primaryUserId, String secondaryUserId, String token) throws Exception { - AccountMergeRequest request = new AccountMergeRequest( - primaryUserId, - secondaryUserId, - token, - Instant.now(clock).plusSeconds(600) - ); - Field idField = AccountMergeRequest.class.getDeclaredField("id"); - idField.setAccessible(true); - idField.set(request, 7L); - return request; - } -} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 2e7d4f285..0a35eec5f 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -765,26 +765,9 @@ "submit": "Update Password" }, "accounts": { - "initiateTitle": "Initiate Account Merge", - "initiateDesc": "Enter the secondary account identifier. Supports local username or `provider:subject` format for external identities.", - "secondaryLabel": "Secondary Identifier", - "secondaryPlaceholder": "e.g.: other_user or github:123456", - "initiating": "Initiating...", - "initiate": "Initiate Merge", - "initiateSuccess": "Merge request created, secondary={{secondaryUserId}}", - "initiateError": "Failed to initiate merge", - "verifyTitle": "Verify & Complete Merge", - "verifyDesc": "Complete token verification first, then confirm to execute data migration.", - "mergeRequestId": "Merge Request ID", - "verificationToken": "Verification Token", - "verifying": "Verifying...", - "verify": "Complete Merge", - "verifySuccess": "Verification successful, confirm to execute the merge", - "verifyError": "Merge verification failed", - "confirming": "Confirming...", - "confirm": "Confirm & Complete Merge", - "confirmSuccess": "Account merge completed", - "confirmError": "Merge confirmation failed" + "unavailableTitle": "Account merging is temporarily unavailable", + "unavailableDescription": "The previous flow could not independently verify control of both accounts, so it has been disabled during a security redesign.", + "unavailableOperatorAction": "Do not ask an administrator to complete a merge in the database. Keep using the accounts separately until the secure flow is available." }, "namespace": { "notFound": "Namespace not found", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 2281121ae..49b0c3c0e 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -765,26 +765,9 @@ "submit": "更新密码" }, "accounts": { - "initiateTitle": "发起账号合并", - "initiateDesc": "输入 secondary 账号标识。支持本地用户名，或 `provider:subject` 格式的外部身份。", - "secondaryLabel": "Secondary 标识", - "secondaryPlaceholder": "例如：other_user 或 github:123456", - "initiating": "发起中...", - "initiate": "发起合并", - "initiateSuccess": "已创建合并请求，secondary={{secondaryUserId}}", - "initiateError": "发起合并失败", - "verifyTitle": "验证并完成合并", - "verifyDesc": "先完成 token 验证，再单独确认执行数据迁移。", - "mergeRequestId": "Merge Request ID", - "verificationToken": "Verification Token", - "verifying": "验证中...", - "verify": "完成合并", - "verifySuccess": "验证成功，确认后将执行正式合并", - "verifyError": "验证合并失败", - "confirming": "确认中...", - "confirm": "确认并完成合并", - "confirmSuccess": "账号合并已完成", - "confirmError": "确认合并失败" + "unavailableTitle": "账号合并暂时不可用", + "unavailableDescription": "旧流程无法分别证明两个账号的控制权，因此在安全重构完成前已被停用。", + "unavailableOperatorAction": "请勿让管理员通过数据库手工完成合并。在安全流程上线前，请继续分别使用两个账号。" }, "namespace": { "notFound": "命名空间不存在", diff --git a/web/src/pages/settings/accounts.test.ts b/web/src/pages/settings/accounts.test.ts index 3b62220a2..d09e2456b 100644 --- a/web/src/pages/settings/accounts.test.ts +++ b/web/src/pages/settings/accounts.test.ts @@ -1,3 +1,7 @@ +/** @vitest-environment jsdom */ + +import { render, screen } from '@testing-library/react' +import { createElement } from 'react' import { describe, expect, it, vi } from 'vitest' vi.mock('react-i18next', async () => { @@ -10,36 +14,17 @@ vi.mock('react-i18next', async () => { } }) -vi.mock('@/features/auth/use-account-merge', () => ({ - useInitiateAccountMerge: () => ({ mutateAsync: vi.fn(), isPending: false }), - useVerifyAccountMerge: () => ({ mutateAsync: vi.fn(), isPending: false }), - useConfirmAccountMerge: () => ({ mutateAsync: vi.fn(), isPending: false }), -})) - -vi.mock('@/shared/lib/error-display', () => ({ - truncateErrorMessage: (v: string) => v, -})) - -vi.mock('@/shared/ui/button', () => ({ - Button: ({ children }: { children: unknown }) => children, -})) - -vi.mock('@/shared/ui/card', () => ({ - Card: ({ children }: { children: unknown }) => children, - CardContent: ({ children }: { children: unknown }) => children, - CardDescription: ({ children }: { children: unknown }) => children, - CardHeader: ({ children }: { children: unknown }) => children, - CardTitle: ({ children }: { children: unknown }) => children, -})) - -vi.mock('@/shared/ui/input', () => ({ - Input: () => null, -})) - import { AccountSettingsPage } from './accounts' describe('AccountSettingsPage', () => { - it('exports a named component function', () => { - expect(typeof AccountSettingsPage).toBe('function') + it('shows the temporary isolation notice without legacy merge controls', () => { + const { container } = render(createElement(AccountSettingsPage)) + + expect(screen.getByText('accounts.unavailableTitle')).toBeTruthy() + expect(screen.getByText('accounts.unavailableDescription')).toBeTruthy() + expect(screen.getByText('accounts.unavailableOperatorAction')).toBeTruthy() + expect(container.querySelector('form')).toBeNull() + expect(container.querySelector('input')).toBeNull() + expect(container.querySelector('button')).toBeNull() }) }) diff --git a/web/src/pages/settings/accounts.tsx b/web/src/pages/settings/accounts.tsx index fb13a9336..e97b04832 100644 --- a/web/src/pages/settings/accounts.tsx +++ b/web/src/pages/settings/accounts.tsx @@ -1,138 +1,25 @@ -import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { useConfirmAccountMerge, useInitiateAccountMerge, useVerifyAccountMerge } from '@/features/auth/use-account-merge' -import { truncateErrorMessage } from '@/shared/lib/error-display' -import { Button } from '@/shared/ui/button' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' -import { Input } from '@/shared/ui/input' /** - * Account linking settings page for the multi-step account merge workflow. - * The route intentionally keeps all three steps visible because operators often - * need to paste ids or tokens across systems while completing the merge. + * Account merge is intentionally unavailable until the platform can prove independent control of + * both accounts. Keep the settings route so existing links remain valid, but do not render any + * legacy identifier, token, verification, or confirmation controls. */ export function AccountSettingsPage() { const { t } = useTranslation() - const [secondaryIdentifier, setSecondaryIdentifier] = useState('') - const [mergeRequestId, setMergeRequestId] = useState('') - const [verificationToken, setVerificationToken] = useState('') - const [statusMessage, setStatusMessage] = useState('') - - const initiateMutation = useInitiateAccountMerge() - const verifyMutation = useVerifyAccountMerge() - const confirmMutation = useConfirmAccountMerge() - - /** - * Starts the merge flow and surfaces the request id plus verification token - * returned by the backend for the following steps. - */ - async function handleInitiate(event: React.FormEvent) { - event.preventDefault() - setStatusMessage('') - try { - const result = await initiateMutation.mutateAsync({ secondaryIdentifier }) - setMergeRequestId(String(result.mergeRequestId)) - setVerificationToken(result.verificationToken) - setStatusMessage(t('accounts.initiateSuccess', { secondaryUserId: result.secondaryUserId })) - } catch (error) { - setStatusMessage( - truncateErrorMessage(error instanceof Error ? error.message : t('accounts.initiateError')) ?? t('accounts.initiateError'), - ) - } - } - - /** - * Verifies ownership of the secondary account before the final merge step. - */ - async function handleVerify(event: React.FormEvent) { - event.preventDefault() - setStatusMessage('') - try { - await verifyMutation.mutateAsync({ - mergeRequestId: Number(mergeRequestId), - verificationToken, - }) - setStatusMessage(t('accounts.verifySuccess')) - } catch (error) { - setStatusMessage( - truncateErrorMessage(error instanceof Error ? error.message : t('accounts.verifyError')) ?? t('accounts.verifyError'), - ) - } - } - - /** - * Finalizes the merge after verification has succeeded. - */ - async function handleConfirm() { - setStatusMessage('') - try { - await confirmMutation.mutateAsync({ mergeRequestId: Number(mergeRequestId) }) - setStatusMessage(t('accounts.confirmSuccess')) - } catch (error) { - setStatusMessage( - truncateErrorMessage(error instanceof Error ? error.message : t('accounts.confirmError')) ?? t('accounts.confirmError'), - ) - } - } return ( -

- - - {t('accounts.initiateTitle')} - {t('accounts.initiateDesc')} - - -
-
- - setSecondaryIdentifier(event.target.value)} - placeholder={t('accounts.secondaryPlaceholder')} - /> -
- -
- - - +
- {t('accounts.verifyTitle')} - {t('accounts.verifyDesc')} + {t('accounts.unavailableTitle')} + {t('accounts.unavailableDescription')} -
-
- - setMergeRequestId(event.target.value)} - /> -
-
- - setVerificationToken(event.target.value)} - /> -
- -
-
- -
- {statusMessage ?

{statusMessage}

: null} +

+ {t('accounts.unavailableOperatorAction')} +