fix(auth): enforce trusted OAuth identity attributes

Signed-off-by: ylhu16 <ylhu16@iflytek.com>
This commit is contained in:
ylhu16 2026-07-30 14:16:40 +08:00
parent b97487b02c
commit 833270bb31
12 changed files with 346 additions and 29 deletions

View file

@ -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

View file

@ -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 + 平台凭证)

View file

@ -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 class IdentityBindingService {
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 class IdentityBindingService {
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 class IdentityBindingService {
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<String> roles = roleBindingRepo.findByUserId(user.getId()).stream()
.map(rb -> rb.getRole().getCode())
@ -97,16 +97,14 @@ public class IdentityBindingService {
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 class IdentityBindingService {
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();
}
}
}

View file

@ -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));
}
}

View file

@ -9,7 +9,6 @@ import org.springframework.web.client.RestClient;
import java.util.Comparator;
import java.util.List;
import org.springframework.stereotype.Component;
import java.util.Map;
/**
@ -19,10 +18,14 @@ import java.util.Map;
@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 class GitHubClaimsExtractor implements OAuthClaimsExtractor {
Map<String, Object> 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",

View file

@ -98,7 +98,9 @@ public class OAuthLoginFlowService {
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

View file

@ -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
));
}
}

View file

@ -15,7 +15,7 @@ public class EmailDomainAccessPolicy implements AccessPolicy {
@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;

View file

@ -11,8 +11,10 @@ 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.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 @@ class IdentityBindingServiceTest {
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<UserAccount> 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 @@ class IdentityBindingServiceTest {
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<UserAccount> userCaptor = ArgumentCaptor.forClass(UserAccount.class);
verify(userRepo).save(userCaptor.capture());
assertThat(userCaptor.getValue().getEmail()).isNull();
}
}

View file

@ -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<String, Object> 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);
}
}

View file

@ -48,6 +48,30 @@ class OAuthLoginFlowServiceTest {
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(

View file

@ -37,10 +37,10 @@ class AccessPolicyTest {
}
@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