fix(auth): keep removed owners in accessible favorites

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-10-19 16:45:49 +08:00
parent 283737d175
commit bbf53c16a6
6 changed files with 28 additions and 5 deletions

View file

@ -61,7 +61,7 @@ public class JpaStarredSkillQueryRepository implements StarredSkillQueryReposito
Predicate member = roles.isEmpty() ? builder.disjunction() : skill.get("namespaceId").in(roles.keySet());
Predicate visibility = builder.or(
builder.equal(skill.get("visibility"), SkillVisibility.PUBLIC),
builder.and(builder.equal(skill.get("visibility"), SkillVisibility.NAMESPACE_ONLY), member),
builder.and(builder.equal(skill.get("visibility"), SkillVisibility.NAMESPACE_ONLY), builder.or(owner, member)),
builder.and(builder.equal(skill.get("visibility"), SkillVisibility.PRIVATE), builder.or(owner, admin)));
Predicate accessible = builder.or(
builder.and(builder.isTrue(skill.get("hidden")), builder.or(owner, admin)),

View file

@ -56,7 +56,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@ActiveProfiles("test")
@Testcontainers
@Import({PromotionRevocationService.class, PromotionRevocationHistoryJpaRepository.class,
@Import({com.iflytek.skillhub.domain.skill.SkillOwnerAccessPolicy.class, PromotionRevocationService.class, PromotionRevocationHistoryJpaRepository.class,
SkillHardDeleteService.class, AuditLogService.class, PromotionDeletionPostgresTest.Config.class})
@Transactional(propagation = Propagation.NOT_SUPPORTED)
class PromotionDeletionPostgresTest {
@ -83,6 +83,7 @@ class PromotionDeletionPostgresTest {
@Autowired private NamespaceJpaRepository namespaces;
@Autowired private UserAccountJpaRepository users;
@Autowired private SkillJpaRepository skills;
@Autowired private com.iflytek.skillhub.infra.jpa.NamespaceMemberJpaRepository members;
@Autowired private SkillVersionJpaRepository versions;
@Autowired private PromotionRequestJpaRepository promotions;
@Autowired private PromotionRevocationService revocations;
@ -145,6 +146,8 @@ class PromotionDeletionPostgresTest {
users.saveAndFlush(new UserAccount(owner, owner, null, null));
users.saveAndFlush(new UserAccount(reviewer, reviewer, null, null));
Namespace team = namespaces.saveAndFlush(new Namespace("delete-" + suffix, "Deletion team", owner));
members.saveAndFlush(new com.iflytek.skillhub.domain.namespace.NamespaceMember(
team.getId(), owner, com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER));
Namespace global = namespaces.findBySlug("global").orElseGet(() -> {
Namespace created = new Namespace("global", "Global", reviewer);
created.setType(NamespaceType.GLOBAL);

View file

@ -62,7 +62,7 @@ import static org.mockito.Mockito.when;
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@ActiveProfiles("test")
@Testcontainers
@Import({SkillAvatarRecoveryPostgresTest.ReviewConfiguration.class, com.iflytek.skillhub.domain.review.ReviewService.class, com.iflytek.skillhub.domain.review.ReviewPermissionChecker.class, SkillAvatarAppService.class, SkillAvatarMachineRecoveryTask.class,
@Import({com.iflytek.skillhub.auth.rbac.RbacService.class, com.iflytek.skillhub.domain.skill.SkillOwnerAccessPolicy.class, SkillAvatarRecoveryPostgresTest.ReviewConfiguration.class, com.iflytek.skillhub.domain.review.ReviewService.class, com.iflytek.skillhub.domain.review.ReviewPermissionChecker.class, SkillAvatarAppService.class, SkillAvatarMachineRecoveryTask.class,
SkillAvatarScopeReviewListener.class, SkillAvatarScopeReviewRecoveryTask.class,
com.iflytek.skillhub.task.SkillAvatarCleanupTask.class})
@Transactional(propagation = Propagation.NOT_SUPPORTED)
@ -82,6 +82,7 @@ class SkillAvatarRecoveryPostgresTest {
@Autowired private SkillJpaRepository skills;
@Autowired private NamespaceJpaRepository namespaces;
@Autowired private com.iflytek.skillhub.infra.jpa.NamespaceMemberJpaRepository members;
@Autowired private com.iflytek.skillhub.infra.jpa.SkillVersionJpaRepository versions;
@Autowired private SkillAvatarCandidateJpaRepository candidates;
@Autowired private SkillAvatarAppService avatars;
@ -616,6 +617,8 @@ class SkillAvatarRecoveryPostgresTest {
return new TransactionTemplate(transactions).execute(status -> {
String suffix = UUID.randomUUID().toString().substring(0, 8);
Namespace ns = namespaces.saveAndFlush(new Namespace("avatar-" + suffix, "Avatar recovery", "owner"));
members.saveAndFlush(new com.iflytek.skillhub.domain.namespace.NamespaceMember(
ns.getId(), "owner", com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER));
Skill skill = skills.saveAndFlush(new Skill(ns.getId(), "skill", "owner", expanded ? SkillVisibility.PRIVATE : SkillVisibility.PUBLIC));
if (expanded) {
SkillAvatarCandidate old = new SkillAvatarCandidate(skill.getId(), "owner", "old/" + skill.getId(), "old-sha-" + suffix,

View file

@ -57,7 +57,7 @@ import static org.mockito.Mockito.*;
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@ActiveProfiles("test")
@Testcontainers
@Import({com.iflytek.skillhub.domain.review.ReviewService.class, com.iflytek.skillhub.domain.review.ReviewPermissionChecker.class, SkillAvatarAppService.class, SkillAvatarUploadService.class, SkillPublishAppService.class,
@Import({com.iflytek.skillhub.auth.rbac.RbacService.class, com.iflytek.skillhub.domain.skill.SkillOwnerAccessPolicy.class, com.iflytek.skillhub.domain.review.ReviewService.class, com.iflytek.skillhub.domain.review.ReviewPermissionChecker.class, SkillAvatarAppService.class, SkillAvatarUploadService.class, SkillPublishAppService.class,
SkillPublishService.class, SkillStorageDeletionCompensationService.class, JpaSkillStorageDeletionCompensationRepositoryAdapter.class, SkillAvatarCleanupTask.class,
SkillAvatarUploadPostgresTest.Configuration.class})
@Transactional(propagation = Propagation.NOT_SUPPORTED)

View file

@ -42,7 +42,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@ActiveProfiles("test")
@Testcontainers
@Import({SkillVisibilityService.class, SkillReviewSubmitService.class,
@Import({com.iflytek.skillhub.domain.skill.SkillOwnerAccessPolicy.class, SkillVisibilityService.class, SkillReviewSubmitService.class,
JpaSkillRepositoryAdapter.class, PromotionService.class, ReviewPermissionChecker.class, SkillVisibilityPostgresTest.Configuration.class})
@Transactional(propagation = Propagation.NOT_SUPPORTED)
class SkillVisibilityPostgresTest {
@ -69,6 +69,7 @@ class SkillVisibilityPostgresTest {
@Autowired SkillJpaRepository skills;
@Autowired SkillVersionJpaRepository versions;
@Autowired NamespaceJpaRepository namespaces;
@Autowired NamespaceMemberJpaRepository members;
@Autowired UserAccountJpaRepository users;
@Autowired SecurityAuditJpaRepository audits;
@Autowired ReviewTaskJpaRepository reviews;
@ -238,6 +239,8 @@ class SkillVisibilityPostgresTest {
String owner = "scope-owner-" + suffix;
users.saveAndFlush(new UserAccount(owner, owner, null, null));
Namespace ns = namespaces.saveAndFlush(new Namespace("scope-" + suffix, "Scope", owner));
members.saveAndFlush(new com.iflytek.skillhub.domain.namespace.NamespaceMember(
ns.getId(), owner, com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER));
Skill skill = skills.saveAndFlush(new Skill(ns.getId(), "foo", owner, SkillVisibility.PRIVATE));
SkillVersion published = new SkillVersion(skill.getId(), "1.0.0", owner);
published.setStatus(SkillVersionStatus.PUBLISHED);

View file

@ -122,6 +122,20 @@ class StarredSkillAccessPostgresTest {
assertThat(query.findAccessible("viewer", Map.of(), Set.of(), PageRequest.of(0, 50)).getTotalElements()).isZero();
}
@Test
void removedOwnerRetainsInternalStarsWithoutExposingThemToOtherNonMembers() {
Skill internal = fixtures.stream()
.filter(skill -> skill.getVisibility() == SkillVisibility.NAMESPACE_ONLY)
.filter(skill -> skill.getLatestVersionId() != null && !skill.isHidden())
.findFirst().orElseThrow();
var owner = query.findAccessible("owner", Map.of(), Set.of(), PageRequest.of(0, 50));
var other = query.findAccessible("viewer", Map.of(), Set.of(), PageRequest.of(0, 50));
assertThat(owner.getContent()).extracting(Skill::getId).contains(internal.getId());
assertThat(owner.getTotalElements()).isEqualTo(12);
assertThat(other.getContent()).extracting(Skill::getId).doesNotContain(internal.getId());
assertThat(other.getTotalElements()).isEqualTo(1);
}
private void assertPolicy(String user, Map<Long, NamespaceRole> roles, Set<String> platformRoles) {
var checker = new VisibilityChecker();
List<Long> expected = fixtures.stream().filter(skill -> checker.canAccess(skill, user, roles, platformRoles))