From bbf53c16a6044828003e57f183d04095a57d2a19 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:45:49 +0800 Subject: [PATCH] fix(auth): keep removed owners in accessible favorites Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../repository/JpaStarredSkillQueryRepository.java | 2 +- .../repository/PromotionDeletionPostgresTest.java | 5 ++++- .../SkillAvatarRecoveryPostgresTest.java | 5 ++++- .../repository/SkillAvatarUploadPostgresTest.java | 2 +- .../repository/SkillVisibilityPostgresTest.java | 5 ++++- .../repository/StarredSkillAccessPostgresTest.java | 14 ++++++++++++++ 6 files changed, 28 insertions(+), 5 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaStarredSkillQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaStarredSkillQueryRepository.java index 42e73c6bd..304036131 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaStarredSkillQueryRepository.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaStarredSkillQueryRepository.java @@ -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)), diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java index c7858ad89..f7fb75094 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java @@ -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); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java index 472b0f88f..c71f48a16 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java @@ -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, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java index 319f90f96..8c1b3642a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java @@ -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) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillVisibilityPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillVisibilityPostgresTest.java index fac17644b..a360fc7e8 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillVisibilityPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillVisibilityPostgresTest.java @@ -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); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/StarredSkillAccessPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/StarredSkillAccessPostgresTest.java index 6d6c724fa..502e22a50 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/StarredSkillAccessPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/StarredSkillAccessPostgresTest.java @@ -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 roles, Set platformRoles) { var checker = new VisibilityChecker(); List expected = fixtures.stream().filter(skill -> checker.canAccess(skill, user, roles, platformRoles))