From 5bb2eb0d981a0a2acf214857dc5a16b45dc8fc3b Mon Sep 17 00:00:00 2001 From: vsxd Date: Fri, 13 Mar 2026 11:29:01 +0800 Subject: [PATCH] fix(auth): grant global membership to new users --- .../auth/identity/IdentityBindingService.java | 11 ++- .../skillhub/auth/local/LocalAuthService.java | 5 + .../identity/IdentityBindingServiceTest.java | 92 +++++++++++++++++++ .../auth/local/LocalAuthServiceTest.java | 6 ++ .../GlobalNamespaceMembershipService.java | 30 ++++++ .../GlobalNamespaceMembershipServiceTest.java | 71 ++++++++++++++ 6 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipService.java create mode 100644 server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipServiceTest.java 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 4227b326..871f79cb 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 @@ -5,6 +5,7 @@ import com.iflytek.skillhub.auth.oauth.OAuthClaims; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; @@ -20,13 +21,16 @@ public class IdentityBindingService { private final IdentityBindingRepository bindingRepo; private final UserAccountRepository userRepo; private final UserRoleBindingRepository roleBindingRepo; + private final GlobalNamespaceMembershipService globalNamespaceMembershipService; public IdentityBindingService(IdentityBindingRepository bindingRepo, - UserAccountRepository userRepo, - UserRoleBindingRepository roleBindingRepo) { + UserAccountRepository userRepo, + UserRoleBindingRepository roleBindingRepo, + GlobalNamespaceMembershipService globalNamespaceMembershipService) { this.bindingRepo = bindingRepo; this.userRepo = userRepo; this.roleBindingRepo = roleBindingRepo; + this.globalNamespaceMembershipService = globalNamespaceMembershipService; } @Transactional @@ -54,6 +58,9 @@ public class IdentityBindingService { ); user.setStatus(initialStatus); user = userRepo.save(user); + if (initialStatus == UserStatus.ACTIVE) { + globalNamespaceMembershipService.ensureMember(user.getId()); + } binding = new IdentityBinding(user.getId(), claims.provider(), claims.subject(), claims.providerLogin()); bindingRepo.save(binding); diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java index ab779e74..1addd5f7 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/LocalAuthService.java @@ -3,6 +3,7 @@ package com.iflytek.skillhub.auth.local; import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; @@ -28,17 +29,20 @@ public class LocalAuthService { private final LocalCredentialRepository credentialRepository; private final UserAccountRepository userAccountRepository; private final UserRoleBindingRepository userRoleBindingRepository; + private final GlobalNamespaceMembershipService globalNamespaceMembershipService; private final PasswordPolicyValidator passwordPolicyValidator; private final PasswordEncoder passwordEncoder; public LocalAuthService(LocalCredentialRepository credentialRepository, UserAccountRepository userAccountRepository, UserRoleBindingRepository userRoleBindingRepository, + GlobalNamespaceMembershipService globalNamespaceMembershipService, PasswordPolicyValidator passwordPolicyValidator, PasswordEncoder passwordEncoder) { this.credentialRepository = credentialRepository; this.userAccountRepository = userAccountRepository; this.userRoleBindingRepository = userRoleBindingRepository; + this.globalNamespaceMembershipService = globalNamespaceMembershipService; this.passwordPolicyValidator = passwordPolicyValidator; this.passwordEncoder = passwordEncoder; } @@ -76,6 +80,7 @@ public class LocalAuthService { normalizedUsername, passwordEncoder.encode(password) )); + globalNamespaceMembershipService.ensureMember(user.getId()); return buildPrincipal(user); } 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 new file mode 100644 index 00000000..c9e9ca5d --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java @@ -0,0 +1,92 @@ +package com.iflytek.skillhub.auth.identity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.auth.entity.IdentityBinding; +import com.iflytek.skillhub.auth.oauth.OAuthClaims; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.repository.IdentityBindingRepository; +import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import java.util.List; +import java.util.Map; +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.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class IdentityBindingServiceTest { + + @Mock + private IdentityBindingRepository bindingRepo; + + @Mock + private UserAccountRepository userRepo; + + @Mock + private UserRoleBindingRepository roleBindingRepo; + + @Mock + private GlobalNamespaceMembershipService globalNamespaceMembershipService; + + private IdentityBindingService service; + + @BeforeEach + void setUp() { + service = new IdentityBindingService(bindingRepo, userRepo, roleBindingRepo, globalNamespaceMembershipService); + } + + @Test + void bindOrCreate_assignsGlobalMembershipForActiveNewUsers() { + OAuthClaims claims = new OAuthClaims( + "github", + "gh_1", + "alice@example.com", + true, + "alice", + Map.of("avatar_url", "https://example.test/a.png") + ); + 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()); + + PlatformPrincipal principal = service.bindOrCreate(claims, UserStatus.ACTIVE); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); + verify(userRepo).save(userCaptor.capture()); + verify(globalNamespaceMembershipService).ensureMember(userCaptor.getValue().getId()); + verify(bindingRepo).save(any(IdentityBinding.class)); + assertThat(principal.displayName()).isEqualTo("alice"); + assertThat(principal.oauthProvider()).isEqualTo("github"); + } + + @Test + void bindOrCreate_doesNotAssignGlobalMembershipForPendingUsers() { + OAuthClaims claims = new OAuthClaims( + "github", + "gh_1", + "alice@example.com", + true, + "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.PENDING); + + verify(globalNamespaceMembershipService, never()).ensureMember(any()); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java index 361083da..8fc505cc 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/LocalAuthServiceTest.java @@ -11,6 +11,7 @@ import com.iflytek.skillhub.auth.exception.AuthFlowException; import com.iflytek.skillhub.auth.entity.Role; import com.iflytek.skillhub.auth.entity.UserRoleBinding; import com.iflytek.skillhub.auth.repository.UserRoleBindingRepository; +import com.iflytek.skillhub.domain.namespace.GlobalNamespaceMembershipService; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; @@ -38,6 +39,9 @@ class LocalAuthServiceTest { @Mock private UserRoleBindingRepository userRoleBindingRepository; + @Mock + private GlobalNamespaceMembershipService globalNamespaceMembershipService; + @Mock private PasswordEncoder passwordEncoder; @@ -49,6 +53,7 @@ class LocalAuthServiceTest { credentialRepository, userAccountRepository, userRoleBindingRepository, + globalNamespaceMembershipService, new PasswordPolicyValidator(), passwordEncoder ); @@ -70,6 +75,7 @@ class LocalAuthServiceTest { assertThat(principal.displayName()).isEqualTo("alice"); assertThat(principal.email()).isEqualTo("alice@example.com"); verify(credentialRepository).save(any(LocalCredential.class)); + verify(globalNamespaceMembershipService).ensureMember(userCaptor.getValue().getId()); } @Test diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipService.java new file mode 100644 index 00000000..947d414f --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipService.java @@ -0,0 +1,30 @@ +package com.iflytek.skillhub.domain.namespace; + +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class GlobalNamespaceMembershipService { + + private static final String GLOBAL_NAMESPACE_SLUG = "global"; + + private final NamespaceRepository namespaceRepository; + private final NamespaceMemberRepository namespaceMemberRepository; + + public GlobalNamespaceMembershipService(NamespaceRepository namespaceRepository, + NamespaceMemberRepository namespaceMemberRepository) { + this.namespaceRepository = namespaceRepository; + this.namespaceMemberRepository = namespaceMemberRepository; + } + + @Transactional + public void ensureMember(String userId) { + Namespace globalNamespace = namespaceRepository.findBySlug(GLOBAL_NAMESPACE_SLUG) + .orElseThrow(() -> new IllegalStateException("Missing built-in global namespace")); + + namespaceMemberRepository.findByNamespaceIdAndUserId(globalNamespace.getId(), userId) + .orElseGet(() -> namespaceMemberRepository.save( + new NamespaceMember(globalNamespace.getId(), userId, NamespaceRole.MEMBER) + )); + } +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipServiceTest.java new file mode 100644 index 00000000..c4e211bb --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/namespace/GlobalNamespaceMembershipServiceTest.java @@ -0,0 +1,71 @@ +package com.iflytek.skillhub.domain.namespace; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Optional; +import java.lang.reflect.Field; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(MockitoExtension.class) +class GlobalNamespaceMembershipServiceTest { + + @Mock + private NamespaceRepository namespaceRepository; + + @Mock + private NamespaceMemberRepository namespaceMemberRepository; + + private GlobalNamespaceMembershipService service; + + @BeforeEach + void setUp() { + service = new GlobalNamespaceMembershipService(namespaceRepository, namespaceMemberRepository); + } + + @Test + void ensureMember_createsGlobalMembershipWhenMissing() throws Exception { + Namespace global = new Namespace("global", "Global", "system"); + setNamespaceId(global, 1L); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "usr_1")).thenReturn(Optional.empty()); + + service.ensureMember("usr_1"); + + ArgumentCaptor memberCaptor = ArgumentCaptor.forClass(NamespaceMember.class); + verify(namespaceMemberRepository).save(memberCaptor.capture()); + assertThat(memberCaptor.getValue().getNamespaceId()).isEqualTo(1L); + assertThat(memberCaptor.getValue().getUserId()).isEqualTo("usr_1"); + assertThat(memberCaptor.getValue().getRole()).isEqualTo(NamespaceRole.MEMBER); + } + + @Test + void ensureMember_keepsExistingGlobalMembership() throws Exception { + Namespace global = new Namespace("global", "Global", "system"); + setNamespaceId(global, 1L); + NamespaceMember existing = new NamespaceMember(1L, "usr_1", NamespaceRole.ADMIN); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "usr_1")).thenReturn(Optional.of(existing)); + + service.ensureMember("usr_1"); + + verify(namespaceMemberRepository, never()).save(any()); + } + + private void setNamespaceId(Namespace namespace, Long id) throws Exception { + Field field = Namespace.class.getDeclaredField("id"); + field.setAccessible(true); + field.set(namespace, id); + } +}