mirror of
https://github.com/iflytek/skillhub.git
synced 2026-08-27 11:14:59 +00:00
fix(auth): harden approved account activation flow
Signed-off-by: ylhu16 <ylhu16@iflytek.com>
This commit is contained in:
parent
075683963e
commit
fe1c6e718b
11 changed files with 236 additions and 6 deletions
|
|
@ -252,7 +252,7 @@
|
|||
- `ACTIVE`:正常使用
|
||||
- `PENDING`:等待管理员审批(AccessPolicy 返回 PENDING_APPROVAL 时创建);批准时必须在同一事务补齐 `@global` membership 后转为 `ACTIVE`
|
||||
- `DISABLED`:管理员封禁,登录后拒绝所有操作,返回 403
|
||||
- `MERGED`:已合并到其他账号,保留记录不物理删除,登录时自动跳转到合并目标账号
|
||||
- `MERGED`:已合并到其他账号,保留记录不物理删除,不允许通过管理员状态接口重新激活
|
||||
- 授权层在每次请求时检查用户状态,非 `ACTIVE` 用户拒绝所有写操作
|
||||
|
||||
### identity_binding
|
||||
|
|
|
|||
|
|
@ -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,从根源上杜绝"待审批账号已认证"的风险。
|
||||
|
||||
|
|
|
|||
|
|
@ -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" \
|
||||
|
|
|
|||
|
|
@ -113,6 +113,9 @@ public class AdminUserAppService {
|
|||
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) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}"的已发布技能
|
||||
|
|
|
|||
|
|
@ -182,6 +182,19 @@ class AdminUserAppServiceTest {
|
|||
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"))
|
||||
|
|
|
|||
|
|
@ -106,6 +106,27 @@ class AdminUserApprovalIntegrationTest {
|
|||
});
|
||||
}
|
||||
|
||||
@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 @@ class AdminUserApprovalIntegrationTest {
|
|||
});
|
||||
}
|
||||
|
||||
@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;
|
||||
|
|
|
|||
|
|
@ -63,8 +63,7 @@ public class OAuthLoginFlowService {
|
|||
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(
|
||||
|
|
|
|||
|
|
@ -137,6 +137,32 @@ class IdentityBindingServiceTest {
|
|||
.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(
|
||||
|
|
|
|||
|
|
@ -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 @@ class OAuthLoginFlowServiceTest {
|
|||
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()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue