From 35e9ca588ac536a9977409a15db5b222c1aaca51 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 15 Jul 2026 16:32:36 +0800 Subject: [PATCH] fix(namespace): allow super admin namespace detail reads Signed-off-by: dongmucat <1127093059@qq.com> --- .../portal/NamespaceController.java | 5 ++-- .../NamespacePortalQueryAppService.java | 21 +++++++++++---- .../NamespacePortalControllerTest.java | 14 ++++++++++ .../NamespacePortalQueryAppServiceTest.java | 27 ++++++++++++++++++- .../my-namespaces-super-admin-actions.spec.ts | 25 ++++++++++++++++- web/src/shared/components/user-menu.test.tsx | 23 ++++++++++++++-- web/src/shared/components/user-menu.tsx | 6 ++--- 7 files changed, 106 insertions(+), 15 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java index bed9d962..ffbe75c2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NamespaceController.java @@ -79,9 +79,10 @@ public class NamespaceController extends BaseApiController { @GetMapping("/namespaces/{slug}") public ApiResponse getNamespace(@PathVariable String slug, @RequestAttribute("userId") String userId, - @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + @RequestAttribute(value = "platformRoles", required = false) Set platformRoles) { return ok("response.success.read", - namespacePortalQueryAppService.getNamespace(slug, userId, userNsRoles)); + namespacePortalQueryAppService.getNamespace(slug, userId, userNsRoles, normalizePlatformRoles(platformRoles))); } @PostMapping("/namespaces") diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java index e11742d1..66c91d72 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalQueryAppService.java @@ -135,12 +135,23 @@ public class NamespacePortalQueryAppService { @Transactional(readOnly = true) public NamespaceResponse getNamespace(String slug, String userId, Map userNamespaceRoles) { + return getNamespace(slug, userId, userNamespaceRoles, Set.of()); + } + + @Transactional(readOnly = true) + public NamespaceResponse getNamespace(String slug, + String userId, + Map userNamespaceRoles, + Set platformRoles) { Map namespaceRoles = userNamespaceRoles != null ? userNamespaceRoles : Map.of(); - Namespace namespace = namespaceService.getNamespaceBySlugForRead( - slug, - userId, - namespaceRoles); - if (!namespaceRoles.containsKey(namespace.getId())) { + boolean superAdmin = isSuperAdmin(platformRoles); + Namespace namespace = superAdmin + ? namespaceService.getNamespaceBySlug(slug) + : namespaceService.getNamespaceBySlugForRead( + slug, + userId, + namespaceRoles); + if (!superAdmin && !namespaceRoles.containsKey(namespace.getId())) { throw new DomainForbiddenException("error.namespace.membership.required"); } return NamespaceResponse.from(namespace); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java index d94a1746..f75746fd 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespacePortalControllerTest.java @@ -147,6 +147,20 @@ class NamespacePortalControllerTest { .andExpect(status().isUnauthorized()); } + @Test + void getNamespace_superAdminReadsNamespaceWithoutMembership() throws Exception { + Namespace namespace = namespace(1L, "team-a", NamespaceStatus.ACTIVE, NamespaceType.TEAM); + given(namespaceMemberRepository.findByUserId("super-1")).willReturn(List.of()); + given(namespaceService.getNamespaceBySlug("team-a")).willReturn(namespace); + + mockMvc.perform(get("/api/v1/namespaces/team-a") + .with(auth("super-1", Set.of("SUPER_ADMIN"))) + .requestAttr("userId", "super-1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.slug").value("team-a")); + } + @Test void archiveNamespace_returnsUpdatedNamespace() throws Exception { Namespace archived = namespace(1L, "team-a", NamespaceStatus.ARCHIVED, NamespaceType.TEAM); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java index 39453e8f..743079db 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/NamespacePortalQueryAppServiceTest.java @@ -5,6 +5,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -151,10 +152,22 @@ class NamespacePortalQueryAppServiceTest { when(namespaceService.getNamespaceBySlugForRead("team-a", "user-1", Map.of())) .thenReturn(namespace); - assertThatThrownBy(() -> service.getNamespace("team-a", "user-1", Map.of())) + assertThatThrownBy(() -> service.getNamespace("team-a", "user-1", Map.of(), Set.of())) .isInstanceOf(DomainForbiddenException.class); } + @Test + void getNamespace_superAdminReadsArchivedNamespaceWithoutMembership() { + Namespace archived = namespace(1L, "archived-team"); + archived.setStatus(NamespaceStatus.ARCHIVED); + when(namespaceService.getNamespaceBySlug("archived-team")).thenReturn(archived); + + var response = service.getNamespace("archived-team", "super-1", Map.of(), Set.of("SUPER_ADMIN")); + + assertThat(response.slug()).isEqualTo("archived-team"); + assertThat(response.status()).isEqualTo(NamespaceStatus.ARCHIVED); + } + private Namespace namespace(Long id, String slug) { Namespace namespace = new Namespace(slug, slug, "owner-1"); ReflectionTestUtils.setField(namespace, "id", id); @@ -219,4 +232,16 @@ class NamespacePortalQueryAppServiceTest { .isInstanceOf(DomainForbiddenException.class) .hasMessageContaining("error.namespace.global.members.platformAdmin.required"); } + + @Test + void listMembers_teamNamespaceRejectsSuperAdminWithoutMembership() { + Namespace ns = namespace(1L, "team-a"); + when(namespaceService.getNamespaceBySlug("team-a")).thenReturn(ns); + doThrow(new DomainForbiddenException("error.namespace.membership.required")) + .when(namespaceService).assertMember(1L, "super-1"); + + assertThatThrownBy(() -> service.listMembers("team-a", PageRequest.of(0, 20), "super-1", Set.of("SUPER_ADMIN"))) + .isInstanceOf(DomainForbiddenException.class) + .hasMessageContaining("error.namespace.membership.required"); + } } diff --git a/web/e2e/my-namespaces-super-admin-actions.spec.ts b/web/e2e/my-namespaces-super-admin-actions.spec.ts index 02b3ce56..2c3949c5 100644 --- a/web/e2e/my-namespaces-super-admin-actions.spec.ts +++ b/web/e2e/my-namespaces-super-admin-actions.spec.ts @@ -70,9 +70,26 @@ test.describe('My Namespaces super admin actions', () => { canDelete: true, }, ])) + + await page.route('**/api/web/namespaces/visible-no-role', (route) => fulfillJson(route, { + id: 101, + slug: 'visible-no-role', + displayName: 'Visible Without Membership', + description: 'Returned by SUPER_ADMIN namespace visibility', + type: 'TEAM', + status: 'ACTIVE', + createdAt: '2026-07-15T00:00:00Z', + })) + + await page.route('**/api/web/skills?**', (route) => fulfillJson(route, { + items: [], + page: 0, + size: 20, + total: 0, + })) }) - test('hides namespace-scoped actions for visible namespaces without membership', async ({ page }) => { + test('keeps namespace-scoped actions hidden and opens detail for visible namespaces without membership', async ({ page }) => { await page.goto('/dashboard/namespaces') const visibleCard = page.getByTestId('namespace-card-visible-no-role') @@ -84,5 +101,11 @@ test.describe('My Namespaces super admin actions', () => { const ownedCard = page.getByTestId('namespace-card-owned-team') await expect(ownedCard.getByRole('button', { name: 'Manage Members' })).toBeVisible() await expect(ownedCard.getByRole('button', { name: 'Review Tasks' })).toBeVisible() + + await visibleCard.click() + + await expect(page).toHaveURL(/\/space\/visible-no-role$/) + await expect(page.getByRole('heading', { name: 'Visible Without Membership' })).toBeVisible() + await expect(page.getByText('@visible-no-role')).toBeVisible() }) }) diff --git a/web/src/shared/components/user-menu.test.tsx b/web/src/shared/components/user-menu.test.tsx index 3487bd8b..fe70125e 100644 --- a/web/src/shared/components/user-menu.test.tsx +++ b/web/src/shared/components/user-menu.test.tsx @@ -1,9 +1,11 @@ import type { ReactNode } from 'react' import { renderToStaticMarkup } from 'react-dom/server' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import * as mod from './user-menu' import { UserMenu } from './user-menu' +const useMyNamespacesMock = vi.hoisted(() => vi.fn(() => ({ data: [] }))) + vi.mock('react', async () => { const actual = await vi.importActual('react') return { @@ -63,7 +65,7 @@ vi.mock('@/api/client', () => ({ })) vi.mock('@/shared/hooks/use-namespace-queries', () => ({ - useMyNamespaces: () => ({ data: [] }), + useMyNamespaces: useMyNamespacesMock, })) /** @@ -77,6 +79,10 @@ describe('user-menu module exports', () => { }) describe('UserMenu security settings visibility', () => { + beforeEach(() => { + useMyNamespacesMock.mockClear() + }) + it('shows security settings when password changes are allowed, independent of OAuth provider', () => { const html = renderToStaticMarkup( { expect(html).not.toContain('user.menu.security') }) + + it('does not fetch namespace memberships while rendering the global menu', () => { + renderToStaticMarkup( + , + ) + + expect(useMyNamespacesMock).not.toHaveBeenCalled() + }) }) diff --git a/web/src/shared/components/user-menu.tsx b/web/src/shared/components/user-menu.tsx index d3eb3d39..76229930 100644 --- a/web/src/shared/components/user-menu.tsx +++ b/web/src/shared/components/user-menu.tsx @@ -3,8 +3,7 @@ import { useTranslation } from 'react-i18next' import { Link } from '@tanstack/react-router' import { useQueryClient } from '@tanstack/react-query' import { authApi } from '@/api/client' -import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' -import { buildGlobalReviewsPath, canAccessReviewCenter } from '@/features/review/review-paths' +import { buildGlobalReviewsPath, canAccessGlobalReviewCenter } from '@/features/review/review-paths' import { clearSessionScopedQueries } from '@/features/notification/notification-session' import { canViewGovernanceCenter } from '@/shared/lib/governance-access' import { cn } from '@/shared/lib/utils' @@ -25,7 +24,6 @@ interface UserMenuProps { export function UserMenu({ user, triggerClassName }: UserMenuProps) { const { t } = useTranslation() const queryClient = useQueryClient() - const { data: myNamespaces } = useMyNamespaces() const rootRef = useRef(null) const closeTimerRef = useRef(null) const [isHovered, setIsHovered] = useState(false) @@ -37,7 +35,7 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) { const isUserAdmin = hasRole('USER_ADMIN') || hasRole('SUPER_ADMIN') const isAuditor = hasRole('AUDITOR') || hasRole('SUPER_ADMIN') const isSuperAdmin = hasRole('SUPER_ADMIN') - const reviewCenterVisible = canAccessReviewCenter(user.platformRoles, myNamespaces) + const reviewCenterVisible = canAccessGlobalReviewCenter(user.platformRoles) const canChangePassword = user.canChangePassword === true const open = isHovered || isClickOpen