From dd76b13d5532dea7f21123658ee0fca9d7fb7762 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 28 Jun 2026 16:39:11 -0400 Subject: [PATCH] fix: require node:read for fleet topology reads and hide Fleet without it (#1507) The fleet overview, configuration, dependency-map, and networking-summary reads were authentication-only, so a role without node:read (deployer) could read node names, host stats, and cross-node topology. They now require node:read, matching the role model where every role except deployer holds it. For parity, the Fleet nav entry is gated on node:read (hiding it from the top nav, mobile menu, and command palette), the Fleet view redirects to the dashboard when reached without it, and the dashboard fleet heartbeat falls back to the single-node restart map for a role that cannot read fleet data. --- .../src/__tests__/fleet-read-authz.test.ts | 64 +++++++++++++++++++ backend/src/routes/fleet.ts | 11 +++- frontend/src/components/EditorLayout.tsx | 5 ++ .../__tests__/useViewNavigationState.test.tsx | 43 ++++++++++++- .../hooks/useViewNavigationState.ts | 20 ++++-- .../dashboard/DashboardActivityCard.tsx | 7 +- .../__tests__/DashboardActivityCard.test.tsx | 42 ++++++++++++ 7 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 backend/src/__tests__/fleet-read-authz.test.ts create mode 100644 frontend/src/components/dashboard/__tests__/DashboardActivityCard.test.tsx diff --git a/backend/src/__tests__/fleet-read-authz.test.ts b/backend/src/__tests__/fleet-read-authz.test.ts new file mode 100644 index 00000000..75490854 --- /dev/null +++ b/backend/src/__tests__/fleet-read-authz.test.ts @@ -0,0 +1,64 @@ +/** + * Authorization tests for the fleet topology reads. /overview, /configuration, + * /dependency-map, /networking-summary, and /update-status expose node names, + * host stats, versions, and cross-node topology, so they require node:read. + * Every shipped role carries node:read except deployer, the denial persona here. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import bcrypt from 'bcrypt'; +import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb'; + +let tmpDir: string; +let app: import('express').Express; +let viewerToken: string; +let deployerToken: string; + +const NODE_READ_ROUTES = [ + '/api/fleet/overview', + '/api/fleet/configuration', + '/api/fleet/dependency-map', + '/api/fleet/networking-summary', + '/api/fleet/update-status', +]; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ app } = await import('../index')); + const { DatabaseService } = await import('../services/DatabaseService'); + const db = DatabaseService.getInstance(); + const hash = await bcrypt.hash('password123', 1); + db.addUser({ username: 'fleet-viewer', password_hash: hash, role: 'viewer' }); + db.addUser({ username: 'fleet-deployer', password_hash: hash, role: 'deployer' }); + const sign = (username: string, role: string): string => { + const user = db.getUserByUsername(username)!; + return jwt.sign({ username, role, tv: user.token_version }, TEST_JWT_SECRET, { expiresIn: '1m' }); + }; + viewerToken = sign('fleet-viewer', 'viewer'); + deployerToken = sign('fleet-deployer', 'deployer'); +}); + +afterAll(() => cleanupTestDb(tmpDir)); + +describe('fleet topology reads require node:read', () => { + for (const route of NODE_READ_ROUTES) { + it(`denies ${route} for a role without node:read (deployer)`, async () => { + const res = await request(app).get(route).set('Authorization', `Bearer ${deployerToken}`); + expect(res.status).toBe(403); + expect(res.body.code).toBe('PERMISSION_DENIED'); + }); + + it(`allows ${route} for a role with node:read (viewer)`, async () => { + const res = await request(app).get(route).set('Authorization', `Bearer ${viewerToken}`); + // The guard lets the request through; the body may be empty/offline in a + // Docker-less test env, but it must not be a 403. + expect(res.status).not.toBe(403); + }); + } + + it('rejects an unauthenticated request', async () => { + const res = await request(app).get('/api/fleet/overview'); + expect(res.status).toBe(401); + }); +}); diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index 732896a6..fa791639 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -549,7 +549,8 @@ fleetRouter.get('/sync-status', authMiddleware, (req: Request, res: Response): v res.json(DatabaseService.getInstance().getFleetSyncStatuses()); }); -fleetRouter.get('/overview', authMiddleware, async (_req: Request, res: Response): Promise => { +fleetRouter.get('/overview', authMiddleware, async (req: Request, res: Response): Promise => { + if (!requirePermission(req, res, 'node:read')) return; try { const debug = isDebugEnabled(); const db = DatabaseService.getInstance(); @@ -602,6 +603,7 @@ interface FleetNodeConfiguration { } fleetRouter.get('/configuration', authMiddleware, async (req: Request, res: Response): Promise => { + if (!requirePermission(req, res, 'node:read')) return; try { const db = DatabaseService.getInstance(); const nodes = db.getNodes(); @@ -670,7 +672,8 @@ fleetRouter.get('/configuration', authMiddleware, async (req: Request, res: Resp * and via its auth-only per-node route for remotes, then merges with per-node * attribution. Unreachable nodes degrade to nodeErrors so the rest still draws. */ -fleetRouter.get('/dependency-map', authMiddleware, async (_req: Request, res: Response): Promise => { +fleetRouter.get('/dependency-map', authMiddleware, async (req: Request, res: Response): Promise => { + if (!requirePermission(req, res, 'node:read')) return; try { const db = DatabaseService.getInstance(); const nodes = db.getNodes(); @@ -748,7 +751,8 @@ function isNodeNetworkingSummary(v: unknown): v is NodeNetworkingSummary { * 404 and degrades to a skip, so one unreachable or unsupported node never fails * the filter for the rest. */ -fleetRouter.get('/networking-summary', authMiddleware, async (_req: Request, res: Response): Promise => { +fleetRouter.get('/networking-summary', authMiddleware, async (req: Request, res: Response): Promise => { + if (!requirePermission(req, res, 'node:read')) return; try { const db = DatabaseService.getInstance(); const nodes = db.getNodes(); @@ -884,6 +888,7 @@ fleetRouter.get('/node/:nodeId/stacks/:stackName/containers', authMiddleware, as }); fleetRouter.get('/update-status', authMiddleware, async (req: Request, res: Response): Promise => { + if (!requirePermission(req, res, 'node:read')) return; try { const db = DatabaseService.getInstance(); const nodes = db.getNodes(); diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 503eb238..42116fd2 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -806,6 +806,11 @@ export default function EditorLayout() { /> ); case 'fleet': + // Never mount the mobile fleet view without node:read: the redirect + // effect bounces the deep-link to the dashboard, but MobileFleet is a + // static (non-lazy) render, so without this guard it would fire one + // /fleet/overview (now 403) before the redirect unmounts it. + if (!can('node:read')) return null; return ( ); } +// A community non-admin user with node:read (e.g. a viewer): sees Fleet, no +// admin-only items. function mockCommunityUser() { vi.mocked(AuthContext.useAuth).mockReturnValue({ isAdmin: false, - can: () => false, + can: (p: string) => p === 'node:read', + } as unknown as ReturnType); + vi.mocked(LicenseContext.useLicense).mockReturnValue({ + isPaid: false, + } as unknown as ReturnType); +} + +// A deployer: stack permissions but no node:read, so no Fleet affordance. +function mockDeployer() { + vi.mocked(AuthContext.useAuth).mockReturnValue({ + isAdmin: false, + can: (p: string) => p === 'stack:read' || p === 'stack:deploy', } as unknown as ReturnType); vi.mocked(LicenseContext.useLicense).mockReturnValue({ isPaid: false, @@ -29,7 +42,7 @@ function mockCommunityUser() { function mockPaidAdmin() { vi.mocked(AuthContext.useAuth).mockReturnValue({ isAdmin: true, - can: (p: string) => p === 'system:audit', + can: (p: string) => p === 'system:audit' || p === 'node:read', } as unknown as ReturnType); vi.mocked(LicenseContext.useLicense).mockReturnValue({ isPaid: true, @@ -39,7 +52,7 @@ function mockPaidAdmin() { function mockCommunityAdmin() { vi.mocked(AuthContext.useAuth).mockReturnValue({ isAdmin: true, - can: () => false, + can: (p: string) => p === 'node:read', } as unknown as ReturnType); vi.mocked(LicenseContext.useLicense).mockReturnValue({ isPaid: false, @@ -209,6 +222,30 @@ describe('useViewNavigationState', () => { expect(values).not.toContain('scheduled-ops'); }); + it('hides Fleet from a user without node:read (deployer)', () => { + mockDeployer(); + const { result } = renderHook(() => useViewNavigationState()); + const values = result.current.navItems.map(i => i.value); + expect(values).not.toContain('fleet'); + // The other base items remain reachable. + expect(values).toContain('dashboard'); + expect(values).toContain('resources'); + expect(values).toContain('templates'); + }); + + it('redirects a user without node:read off the Fleet view reached via a deep-link event', () => { + const onNavigateToDashboard = vi.fn(); + mockDeployer(); + const { result } = renderHook(() => useViewNavigationState({ onNavigateToDashboard })); + act(() => { + window.dispatchEvent( + new CustomEvent(SENCHO_NAVIGATE_EVENT, { detail: { view: 'fleet' } }), + ); + }); + expect(result.current.activeView).toBe('dashboard'); + expect(onNavigateToDashboard).toHaveBeenCalled(); + }); + it('shows the admin-only Logs entry for an admin on any tier (role gate, not tier gate)', () => { mockCommunityAdmin(); const { result } = renderHook(() => useViewNavigationState()); diff --git a/frontend/src/components/EditorLayout/hooks/useViewNavigationState.ts b/frontend/src/components/EditorLayout/hooks/useViewNavigationState.ts index 778d5591..2e31133e 100644 --- a/frontend/src/components/EditorLayout/hooks/useViewNavigationState.ts +++ b/frontend/src/components/EditorLayout/hooks/useViewNavigationState.ts @@ -115,13 +115,18 @@ export function useViewNavigationState(options?: UseViewNavigationStateOptions) const navItems = useMemo((): NavItem[] => { const items: NavItem[] = [ { value: 'dashboard', label: 'Home', icon: Home }, - { value: 'fleet', label: 'Fleet', icon: Radar }, + ]; + // Fleet surfaces node topology and host stats, so it is gated on node:read + // (held by every role except deployer), matching the backend guard on the + // fleet overview / configuration / dependency / networking reads. + if (can('node:read')) items.push({ value: 'fleet', label: 'Fleet', icon: Radar }); + items.push( { value: 'resources', label: 'Resources', icon: HardDrive }, // Security is a Community, node-scoped review surface (not hub-only), so // it shows for every authenticated user and on remote nodes too. { value: 'security', label: 'Security', icon: ShieldCheck }, { value: 'templates', label: 'App Store', icon: CloudDownload }, - ]; + ); // The aggregated Logs feed crosses every managed stack, so it is an // admin-only operator view (the backend gates the same routes on admin). if (isAdmin) items.push({ value: 'global-observability', label: 'Logs', icon: Activity }); @@ -140,16 +145,19 @@ export function useViewNavigationState(options?: UseViewNavigationStateOptions) useEffect(() => { // Redirect off a view the active context can't reach: a hub-only view while - // a remote node is active, or the admin-only Logs view as a non-admin (e.g. - // arrived via a deep-link event rather than the now-hidden nav item). + // a remote node is active, the admin-only Logs view as a non-admin, or the + // Fleet view without node:read (e.g. arrived via a deep-link event rather + // than the now-hidden nav item). const blockedByRemote = isRemote && HUB_ONLY_VIEWS.has(activeView); - const blockedByRole = !isAdmin && activeView === 'global-observability'; + const blockedByRole = + (!isAdmin && activeView === 'global-observability') + || (!can('node:read') && activeView === 'fleet'); if (blockedByRemote || blockedByRole) { onNavigateToDashboard?.(); setActiveView('dashboard'); setFilterNodeId(null); } - }, [isRemote, isAdmin, activeView, onNavigateToDashboard]); + }, [isRemote, isAdmin, can, activeView, onNavigateToDashboard]); return { activeView, setActiveView, diff --git a/frontend/src/components/dashboard/DashboardActivityCard.tsx b/frontend/src/components/dashboard/DashboardActivityCard.tsx index b0c7e65b..5c46aaa4 100644 --- a/frontend/src/components/dashboard/DashboardActivityCard.tsx +++ b/frontend/src/components/dashboard/DashboardActivityCard.tsx @@ -1,12 +1,17 @@ import { useNodes } from '@/context/NodeContext'; +import { useAuth } from '@/context/AuthContext'; import { FleetHeartbeat } from './FleetHeartbeat'; import { StackRestartMap } from './StackRestartMap'; export function DashboardActivityCard() { const { nodes } = useNodes(); + const { can } = useAuth(); const hasRemoteNodes = nodes.some(n => n.type === 'remote'); - if (hasRemoteNodes) { + // The fleet heartbeat reads /fleet/overview, which is gated on node:read; a + // role without it (deployer) gets the single-node restart map instead so the + // card never shows a fleet view it cannot load. + if (hasRemoteNodes && can('node:read')) { return ; } diff --git a/frontend/src/components/dashboard/__tests__/DashboardActivityCard.test.tsx b/frontend/src/components/dashboard/__tests__/DashboardActivityCard.test.tsx new file mode 100644 index 00000000..bb29e20f --- /dev/null +++ b/frontend/src/components/dashboard/__tests__/DashboardActivityCard.test.tsx @@ -0,0 +1,42 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import * as NodeContext from '@/context/NodeContext'; +import * as AuthContext from '@/context/AuthContext'; +import { DashboardActivityCard } from '../DashboardActivityCard'; + +vi.mock('@/context/NodeContext'); +vi.mock('@/context/AuthContext'); +vi.mock('../FleetHeartbeat', () => ({ FleetHeartbeat: () =>
})); +vi.mock('../StackRestartMap', () => ({ StackRestartMap: () =>
})); + +function setup(opts: { remote: boolean; nodeRead: boolean }) { + vi.mocked(NodeContext.useNodes).mockReturnValue({ + nodes: opts.remote ? [{ type: 'remote' }, { type: 'local' }] : [{ type: 'local' }], + } as unknown as ReturnType); + vi.mocked(AuthContext.useAuth).mockReturnValue({ + can: (p: string) => opts.nodeRead && p === 'node:read', + } as unknown as ReturnType); +} + +describe('DashboardActivityCard', () => { + beforeEach(() => vi.clearAllMocks()); + + it('shows the fleet heartbeat for a multi-node fleet when the user has node:read', () => { + setup({ remote: true, nodeRead: true }); + render(); + expect(screen.getByTestId('fleet-heartbeat')).toBeInTheDocument(); + }); + + it('falls back to the restart map for a multi-node fleet without node:read (deployer)', () => { + setup({ remote: true, nodeRead: false }); + render(); + expect(screen.getByTestId('stack-restart-map')).toBeInTheDocument(); + expect(screen.queryByTestId('fleet-heartbeat')).not.toBeInTheDocument(); + }); + + it('shows the restart map for a single-node setup regardless of node:read', () => { + setup({ remote: false, nodeRead: true }); + render(); + expect(screen.getByTestId('stack-restart-map')).toBeInTheDocument(); + }); +});