From c6d9fb98e5f2fb421147427324617fc9620f2e87 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 30 Aug 2026 00:10:26 +0000 Subject: [PATCH] fix(sso): enforce hub-only SSO config when remote node is active (#1865) SSO configuration is control-plane state and must not follow the active remote node. Add /api/sso/ to hub-only prefixes with case-insensitive matching, hide the Settings section on remotes, and use localOnly on every SSOSection fetch as defense in depth. --- backend/src/__tests__/hub-only-guard.test.ts | 84 ++++++++++++++++++- .../__tests__/proxy-sso-config-authz.test.ts | 35 ++++---- backend/src/helpers/proxyExemptPaths.ts | 10 ++- docs/reference/settings.mdx | 4 + frontend/src/components/SSOSection.tsx | 13 +-- .../components/__tests__/SSOSection.test.tsx | 22 ++--- .../settingsVisibilityMatrix.test.ts | 5 ++ frontend/src/components/settings/registry.ts | 1 + frontend/src/lib/routing/reachability.test.ts | 5 ++ 9 files changed, 136 insertions(+), 43 deletions(-) diff --git a/backend/src/__tests__/hub-only-guard.test.ts b/backend/src/__tests__/hub-only-guard.test.ts index 3a67cd0c..b9c1e965 100644 --- a/backend/src/__tests__/hub-only-guard.test.ts +++ b/backend/src/__tests__/hub-only-guard.test.ts @@ -22,13 +22,16 @@ describe('hubOnlyGuard', () => { let app: import('express').Express; let authHeader: string; let remoteNodeId: number; + let proxyRemoteNodeId: number; + let pilotRemoteNodeId: number; beforeAll(async () => { tmpDir = await setupTestDb(); ({ app } = await import('../index')); const { DatabaseService } = await import('../services/DatabaseService'); - remoteNodeId = DatabaseService.getInstance().addNode({ + const db = DatabaseService.getInstance(); + remoteNodeId = db.addNode({ name: 'hub-only-remote', type: 'remote', compose_dir: '/tmp', @@ -36,6 +39,23 @@ describe('hubOnlyGuard', () => { api_url: 'http://127.0.0.1:1', api_token: 'hub-only-token', }); + proxyRemoteNodeId = db.addNode({ + name: 'hub-only-sso-proxy', + type: 'remote', + mode: 'proxy', + compose_dir: '/tmp', + is_default: false, + api_url: 'http://127.0.0.1:1', + api_token: 'hub-only-sso-proxy-token', + }); + pilotRemoteNodeId = db.addNode({ + name: 'hub-only-sso-pilot', + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/tmp', + is_default: false, + api_token: 'hub-only-sso-pilot-token', + }); const token = jwt.sign({ username: TEST_USERNAME }, TEST_JWT_SECRET, { expiresIn: '1m' }); authHeader = `Bearer ${token}`; @@ -286,4 +306,66 @@ describe('hubOnlyGuard', () => { expect(res.body?.code).not.toBe('HUB_ONLY_ENDPOINT'); }); + + it('rejects mixed-case /api/Secrets with 403 when nodeId targets a remote node', async () => { + const res = await request(app) + .get('/api/Secrets') + .set('Authorization', authHeader) + .set('x-node-id', String(remoteNodeId)); + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + }); + + const ssoCases = [ + { method: 'get' as const, path: '/api/sso/config' }, + { method: 'get' as const, path: '/api/sso/config/ldap' }, + { method: 'put' as const, path: '/api/sso/config/ldap', body: { enabled: true } }, + { method: 'delete' as const, path: '/api/sso/config/ldap' }, + { method: 'post' as const, path: '/api/sso/config/ldap/test' }, + { method: 'get' as const, path: '/api/sso/config/role-sync' }, + { method: 'put' as const, path: '/api/sso/config/role-sync', body: { enabled: true } }, + { method: 'get' as const, path: '/api/sso/auth-mode' }, + { method: 'put' as const, path: '/api/sso/auth-mode', body: { mode: 'local_and_sso' } }, + ]; + + for (const remoteLabel of ['proxy', 'pilot_agent'] as const) { + const nodeIdForLabel = () => (remoteLabel === 'proxy' ? proxyRemoteNodeId : pilotRemoteNodeId); + + for (const { method, path, body } of ssoCases) { + it(`rejects ${method.toUpperCase()} ${path} with 403 for ${remoteLabel} remote node`, async () => { + const req = request(app)[method](path) + .set('Authorization', authHeader) + .set('x-node-id', String(nodeIdForLabel())); + if (body !== undefined) { + req.send(body); + } + const res = await req; + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + }); + } + } + + it('rejects mixed-case GET /api/SSO/config with 403 for proxy remote node', async () => { + const res = await request(app) + .get('/api/SSO/config') + .set('Authorization', authHeader) + .set('x-node-id', String(proxyRemoteNodeId)); + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + }); + + it('returns guard 403 (not proxy 503) for pilot_agent without a live tunnel', async () => { + const res = await request(app) + .get('/api/sso/config') + .set('Authorization', authHeader) + .set('x-node-id', String(pilotRemoteNodeId)); + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + expect(res.status).not.toBe(503); + }); }); diff --git a/backend/src/__tests__/proxy-sso-config-authz.test.ts b/backend/src/__tests__/proxy-sso-config-authz.test.ts index 7b78a099..6b214729 100644 --- a/backend/src/__tests__/proxy-sso-config-authz.test.ts +++ b/backend/src/__tests__/proxy-sso-config-authz.test.ts @@ -1,9 +1,8 @@ /** - * Hub → remote proxy coverage for SSO configuration API-token rejection. - * The remote proxy replaces incoming credentials with a node-to-node JWT, - * so destination-side rejectApiTokenScope cannot identify the original API - * token. This test proves that the hub-side guard in remoteNodeProxy blocks - * full-admin API tokens targeting /api/sso/config on remote nodes. + * Hub → remote proxy coverage for SSO configuration rejection. + * API tokens are blocked by the proxy-side rejectApiTokenScope gate (SCOPE_DENIED). + * Browser sessions with a remote x-node-id are blocked by hubOnlyGuard (HUB_ONLY_ENDPOINT) + * before any upstream hop, since SSO config is control-plane identity state. */ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; import http from 'http'; @@ -98,7 +97,7 @@ beforeEach(() => { capturedHops.length = 0; }); -describe('Hub-side API-token rejection for remote SSO config', () => { +describe('Hub-side rejection for remote SSO config', () => { const blockedRequests: Array<{ method: 'get' | 'put'; path: string; body?: Record }> = [ { method: 'get', path: '/api/sso/config/role-sync' }, { method: 'get', path: '/api/sso/config' }, @@ -114,7 +113,7 @@ describe('Hub-side API-token rejection for remote SSO config', () => { ]; for (const { method, path, body } of blockedRequests) { - it(`returns 403 SCOPE_DENIED for ${method.toUpperCase()} ${path}, no upstream hop`, async () => { + it(`returns 403 HUB_ONLY_ENDPOINT for ${method.toUpperCase()} ${path} (API token), no upstream hop`, async () => { let req = request(app)[method](path) .set('Authorization', `Bearer ${fullAdminApiToken}`) .set('x-node-id', String(remoteNodeId)); @@ -122,35 +121,31 @@ describe('Hub-side API-token rejection for remote SSO config', () => { const res = await req; expect(res.status).toBe(403); - expect(res.body.code).toBe('SCOPE_DENIED'); + expect(res.body.code).toBe('HUB_ONLY_ENDPOINT'); expect(capturedHops).toHaveLength(0); }); } - it('Browser admin session reaches upstream for GET /api/sso/config/role-sync', async () => { + it('Browser admin session is rejected by hubOnlyGuard for GET /api/sso/config/role-sync', async () => { const res = await request(app) .get('/api/sso/config/role-sync') .set('Authorization', `Bearer ${adminToken}`) .set('x-node-id', String(remoteNodeId)); - expect(res.status).toBe(200); - const hop = capturedHops.find((h) => h.url?.includes('/sso/config/role-sync')); - expect(hop).toBeDefined(); - expect(hop!.method).toBe('GET'); - expect(hop!.roleHeader).toBe('admin'); + expect(res.status).toBe(403); + expect(res.body.code).toBe('HUB_ONLY_ENDPOINT'); + expect(capturedHops).toHaveLength(0); }); - it('Browser admin session reaches upstream for PUT /api/sso/config/role-sync', async () => { + it('Browser admin session is rejected by hubOnlyGuard for PUT /api/sso/config/role-sync', async () => { const res = await request(app) .put('/api/sso/config/role-sync') .set('Authorization', `Bearer ${adminToken}`) .set('x-node-id', String(remoteNodeId)) .send({ enabled: true }); - expect(res.status).toBe(200); - const hop = capturedHops.find((h) => h.url?.includes('/sso/config/role-sync')); - expect(hop).toBeDefined(); - expect(hop!.method).toBe('PUT'); - expect(hop!.roleHeader).toBe('admin'); + expect(res.status).toBe(403); + expect(res.body.code).toBe('HUB_ONLY_ENDPOINT'); + expect(capturedHops).toHaveLength(0); }); }); diff --git a/backend/src/helpers/proxyExemptPaths.ts b/backend/src/helpers/proxyExemptPaths.ts index 094bf073..0b9bce8d 100644 --- a/backend/src/helpers/proxyExemptPaths.ts +++ b/backend/src/helpers/proxyExemptPaths.ts @@ -26,7 +26,8 @@ export function isProxyExemptPath(path: string): boolean { // Path prefixes that are hub-only: they must be served on the instance you are // signed into and never proxied to a remote node. This covers state owned by // the local hub (centralized audit, fleet schedules, notification routing -// rules, the admin-only aggregated logs feed and its stream counters) and +// rules, the admin-only aggregated logs feed and its stream counters), SSO +// configuration and authentication mode (control-plane identity state), and // private registry credentials, which are stored and managed per instance. // Blueprints and node labels are hub-owned too: the hub is the only instance // that holds the desired-state definitions and the label set its placement @@ -64,13 +65,16 @@ export const HUB_ONLY_PREFIXES: readonly string[] = [ '/api/blueprints/', '/api/node-labels/', '/api/registry-delivery/', + '/api/sso/', ]; /** Returns true when the path is hub-only and must not be proxied to a remote node. */ export function isHubOnlyPath(path: string): boolean { + const normalized = path.toLowerCase(); for (const prefix of HUB_ONLY_PREFIXES) { - if (path.startsWith(prefix)) return true; - if (path === prefix.slice(0, -1)) return true; + const normalizedPrefix = prefix.toLowerCase(); + if (normalized.startsWith(normalizedPrefix)) return true; + if (normalized === normalizedPrefix.slice(0, -1)) return true; } return false; } diff --git a/docs/reference/settings.mdx b/docs/reference/settings.mdx index 8731bcb1..82e74f43 100644 --- a/docs/reference/settings.mdx +++ b/docs/reference/settings.mdx @@ -220,6 +220,10 @@ See [RBAC & User Management](/features/rbac) for details on what each role can a Custom OIDC and the preset providers (Google, GitHub, Okta) are available on Community; LDAP / Active Directory requires Admiral. + + SSO configuration is control-plane state and is not available while a remote node is selected. + + **Scope:** Global, admin-only Configure Single Sign-On providers for centralized authentication. Each provider type has its own configuration card with connection fields, a test button, and an active toggle. diff --git a/frontend/src/components/SSOSection.tsx b/frontend/src/components/SSOSection.tsx index b8522b97..4a326dad 100644 --- a/frontend/src/components/SSOSection.tsx +++ b/frontend/src/components/SSOSection.tsx @@ -93,6 +93,7 @@ function ProviderCard({ providerId, type, label, initialConfig, onSave }: { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), + localOnly: true, }); if (res.ok) { toast.success('SSO configuration saved'); @@ -112,7 +113,7 @@ function ProviderCard({ providerId, type, label, initialConfig, onSave }: { setTesting(true); setTestResult(null); try { - const res = await apiFetch(`/sso/config/${providerId}/test`, { method: 'POST' }); + const res = await apiFetch(`/sso/config/${providerId}/test`, { method: 'POST', localOnly: true }); const data = await res.json().catch(() => null); if (!res.ok) { const message = data?.error || data?.message || 'Connection test failed'; @@ -137,7 +138,7 @@ function ProviderCard({ providerId, type, label, initialConfig, onSave }: { const handleDelete = async () => { try { - const res = await apiFetch(`/sso/config/${providerId}`, { method: 'DELETE' }); + const res = await apiFetch(`/sso/config/${providerId}`, { method: 'DELETE', localOnly: true }); if (res.ok) { toast.success('SSO provider removed'); setConfig({ enabled: false }); @@ -432,7 +433,7 @@ function RoleSyncToggle() { let cancelled = false; void (async () => { try { - const res = await apiFetch('/sso/config/role-sync'); + const res = await apiFetch('/sso/config/role-sync', { localOnly: true }); if (!res.ok) { if (!cancelled) { const data = await res.json().catch(() => null); @@ -460,6 +461,7 @@ function RoleSyncToggle() { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ enabled: next }), + localOnly: true, }); if (!res.ok) { const data = await res.json().catch(() => null); @@ -521,7 +523,7 @@ function AuthenticationModePanel({ const loadMode = async () => { try { - const res = await apiFetch('/sso/auth-mode'); + const res = await apiFetch('/sso/auth-mode', { localOnly: true }); if (!res.ok) { const data = await res.json().catch(() => null); toast.error(data?.error || 'Failed to load authentication mode'); @@ -551,6 +553,7 @@ function AuthenticationModePanel({ method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), + localOnly: true, }); const data = await res.json().catch(() => null); if (!res.ok) { @@ -676,7 +679,7 @@ export function SSOSection() { const fetchConfigs = async () => { try { - const res = await apiFetch('/sso/config'); + const res = await apiFetch('/sso/config', { localOnly: true }); if (res.ok) { setConfigs(await res.json()); } else { diff --git a/frontend/src/components/__tests__/SSOSection.test.tsx b/frontend/src/components/__tests__/SSOSection.test.tsx index 73a2db33..c7d9b0e6 100644 --- a/frontend/src/components/__tests__/SSOSection.test.tsx +++ b/frontend/src/components/__tests__/SSOSection.test.tsx @@ -181,7 +181,7 @@ describe('SSOSection role sync toggle', () => { }); render(); await waitFor(() => { - expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync'); + expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync', { localOnly: true }); }); await waitFor(() => { const roleSyncToggle = getRoleSyncSwitch(); @@ -250,7 +250,7 @@ describe('SSOSection role sync toggle', () => { render(); await waitFor(() => { - expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync'); + expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync', { localOnly: true }); }); // Wait for the toggle to load, then click it @@ -332,24 +332,18 @@ describe('SSOSection role sync toggle', () => { }); }); - it('active-instance targeting: role-sync apiFetch calls do not pass localOnly', async () => { + it('hub-local targeting: every apiFetch call passes localOnly: true', async () => { mockBaseSsoLoad((path: string) => { if (path === '/sso/config/role-sync') return res(true, { enabled: false }); return undefined; }); render(); await waitFor(() => { - // Find the role-sync GET call - const calls = mockedFetch.mock.calls; - const roleSyncCalls = calls.filter(([path]) => path === '/sso/config/role-sync'); - expect(roleSyncCalls.length).toBeGreaterThan(0); - // Assert no localOnly: true in any role-sync call (opts may be undefined - // on the mount GET, which is fine: the point is it never targets the hub) - for (const [, opts] of roleSyncCalls) { - const rest = opts as { localOnly?: boolean } | undefined; - expect(rest?.localOnly).not.toBe(true); - } + expect(mockedFetch.mock.calls.length).toBeGreaterThan(0); }); + for (const [, opts] of mockedFetch.mock.calls) { + expect((opts as { localOnly?: boolean } | undefined)?.localOnly).toBe(true); + } }); it('exact payload: PUT sends only { enabled: boolean }', async () => { @@ -372,7 +366,7 @@ describe('SSOSection role sync toggle', () => { render(); await waitFor(() => { - expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync'); + expect(mockedFetch).toHaveBeenCalledWith('/sso/config/role-sync', { localOnly: true }); }); // Wait for the ON toggle to load, then click to turn off diff --git a/frontend/src/components/settings/__tests__/settingsVisibilityMatrix.test.ts b/frontend/src/components/settings/__tests__/settingsVisibilityMatrix.test.ts index 7ecd46c8..4af71759 100644 --- a/frontend/src/components/settings/__tests__/settingsVisibilityMatrix.test.ts +++ b/frontend/src/components/settings/__tests__/settingsVisibilityMatrix.test.ts @@ -75,4 +75,9 @@ describe('settings section visibility by role', () => { expect(isItemVisible(item, visibilityFor('admin'))).toBe(true); } }); + + it('hides SSO from admins when a remote node is active', () => { + const sso = SETTINGS_ITEMS.find(i => i.id === 'sso')!; + expect(isItemVisible(sso, visibilityFor('admin', { isRemote: true }))).toBe(false); + }); }); diff --git a/frontend/src/components/settings/registry.ts b/frontend/src/components/settings/registry.ts index 07622273..117d1a23 100644 --- a/frontend/src/components/settings/registry.ts +++ b/frontend/src/components/settings/registry.ts @@ -102,6 +102,7 @@ export const SETTINGS_ITEMS: readonly SettingsItemMeta[] = [ tier: null, scope: 'global', adminOnly: true, + hiddenOnRemote: true, }, { id: 'api-tokens', diff --git a/frontend/src/lib/routing/reachability.test.ts b/frontend/src/lib/routing/reachability.test.ts index 951ea1e4..459b0d6d 100644 --- a/frontend/src/lib/routing/reachability.test.ts +++ b/frontend/src/lib/routing/reachability.test.ts @@ -151,4 +151,9 @@ describe('reachability', () => { expect(isSettingsSectionHidden('sso', nodeAdmin)).toBe(true); expect(isSettingsSectionHidden('recovery', nodeAdmin)).toBe(true); }); + + it('hides SSO for admins when a remote node is active', () => { + const adminRemote = ctx({ isAdmin: true, isRemote: true }); + expect(isSettingsSectionHidden('sso', adminRemote)).toBe(true); + }); });