From fd1524488819fe487883c13729c9b80da81f39a4 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Thu, 6 Aug 2026 06:01:30 +0200 Subject: [PATCH 01/10] ACM-39327: bound non-admin SSE memory and RBAC checks on release-2.13 Port SelfSubjectRulesReview short-circuit and access-cache hardening from main (#6638) so restricted users no longer trigger O(N) SSARs on /events. Omit compression/meta/filter-before-inflate pieces not present on 2.13. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/routes/events.ts | 328 +++++++++++++++++++++++++++++++++-- 1 file changed, 309 insertions(+), 19 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index bf3ab63adbb..49e84bc29b0 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -1,6 +1,7 @@ /* Copyright Contributors to the Open Cluster Management project */ /* eslint-disable no-constant-condition */ +import { createHash } from 'node:crypto' import eventStream from 'event-stream' import get from 'get-value' import got, { CancelError, HTTPError, TimeoutError } from 'got' @@ -133,6 +134,135 @@ export let resourceCache: ResourceCache = {} const accessCache: Record }>> = {} +interface SubjectRulesStatus { + incomplete: boolean + /** True when the SelfSubjectRulesReview request itself failed. */ + unavailable?: boolean + resourceRules: Array<{ + verbs?: string[] + apiGroups?: string[] + resources?: string[] + resourceNames?: string[] + }> +} + +type KindGetAccess = + | { type: 'deny-all' } + | { type: 'allow-all' } + | { type: 'allow-names'; names: Set } + | { type: 'incomplete' } + +const subjectRulesCache: Record }> = {} +const kindGetAccessCache: Record }> = {} + +/** Clear all cached RBAC access checks. Used for test isolation. */ +export function resetAccessCache() { + for (const key in accessCache) { + delete accessCache[key] + } + for (const key in subjectRulesCache) { + delete subjectRulesCache[key] + } + for (const key in kindGetAccessCache) { + delete kindGetAccessCache[key] + } +} + +export function getAccessCache() { + return accessCache +} + +export const ACCESS_CACHE_TTL = 60 * 1000 // 60 seconds +export const ACCESS_CACHE_CLEANUP_INTERVAL = 90 * 1000 // 90 seconds +export const ACCESS_CACHE_MAX_TOKENS = 1000 // Maximum number of token entries to keep +export const ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN = 2000 // Cap RBAC keys retained per token + +/** Hash bearer tokens so the access cache does not retain full JWTs as object keys. */ +export function hashAccessToken(token: string): string { + return createHash('sha256').update(token).digest('hex') +} + +function enforceAccessCacheEntryCap(tokenCache: Record }>) { + const keys = Object.keys(tokenCache) + if (keys.length <= ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) return + keys.sort((a, b) => tokenCache[a].time - tokenCache[b].time) + const toRemove = keys.length - ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + for (let i = 0; i < toRemove; i++) { + delete tokenCache[keys[i]] + } +} + +let accessCacheCleanupTimer: NodeJS.Timeout | undefined + +export function cleanupAccessCache() { + const now = Date.now() + const cutoffTime = now - ACCESS_CACHE_TTL + const tokenStats: Array<{ token: string; newestTime: number }> = [] + + for (const token in accessCache) { + const tokenCache = accessCache[token] + let newestTime = 0 + + for (const key in tokenCache) { + if (tokenCache[key].time < cutoffTime) { + delete tokenCache[key] + } else if (tokenCache[key].time > newestTime) { + newestTime = tokenCache[key].time + } + } + + if (Object.keys(tokenCache).length === 0) { + delete accessCache[token] + } else { + enforceAccessCacheEntryCap(tokenCache) + tokenStats.push({ token, newestTime }) + } + } + + for (const key in subjectRulesCache) { + if (subjectRulesCache[key].time < cutoffTime) { + delete subjectRulesCache[key] + } + } + for (const key in kindGetAccessCache) { + if (kindGetAccessCache[key].time < cutoffTime) { + delete kindGetAccessCache[key] + } + } + + if (tokenStats.length > ACCESS_CACHE_MAX_TOKENS) { + tokenStats.sort((a, b) => a.newestTime - b.newestTime) + const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS + + for (let i = 0; i < tokensToRemove; i++) { + delete accessCache[tokenStats[i].token] + } + } +} + +function startAccessCacheCleanup() { + if (accessCacheCleanupTimer) return + + accessCacheCleanupTimer = setInterval(() => { + try { + cleanupAccessCache() + } catch (err: unknown) { + logger.error({ msg: 'accessCache cleanup failed', error: err }) + } + }, ACCESS_CACHE_CLEANUP_INTERVAL) + + accessCacheCleanupTimer.unref() + logger.info({ msg: 'accessCache cleanup started', interval: ACCESS_CACHE_CLEANUP_INTERVAL }) +} + +function stopAccessCacheCleanup() { + if (accessCacheCleanupTimer) { + clearInterval(accessCacheCleanupTimer) + accessCacheCleanupTimer = undefined + logger.info({ msg: 'accessCache cleanup stopped' }) + } +} + const definitions: IWatchOptions[] = [ { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, @@ -223,6 +353,7 @@ const definitions: IWatchOptions[] = [ export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter + startAccessCacheCleanup() for (const definition of definitions) { void listAndWatch(definition) @@ -626,14 +757,25 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { if (allowed) return true - return canListNamespacedScopedKind(resource, token).then((allowed) => { - if (allowed) return true - return canGetResource(resource, token) - }) + // After cluster list is denied, use one SelfSubjectRulesReview per token/kind. + // Do NOT fall through to namespaced list SSAR first — that is O(namespaces) and + // OOMs/hangs restricted users when MOCK_CLUSTERS (or real inventory) is large. + return resolveKindGetAccess(resource.kind, resource.apiVersion, token).then((access) => + applyKindGetAccess(access, resource, token, () => + // Authorizer could not enumerate rules; keep prior namespaced-list then get fallback. + canListNamespacedScopedKind(resource, token).then((nsAllowed) => { + if (nsAllowed) return true + return canAccess(resource, 'get', token) + }) + ) + ) }) } default: @@ -642,11 +784,17 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { +function canListClusterScopedKind( + resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, + token: string +): Promise { return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) } -function canListNamespacedScopedKind(resource: IResource, token: string): Promise { +function canListNamespacedScopedKind( + resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, + token: string +): Promise { if (!resource.metadata?.namespace) return Promise.resolve(false) return canAccess( { @@ -659,8 +807,142 @@ function canListNamespacedScopedKind(resource: IResource, token: string): Promis ) } -function canGetResource(resource: IResource, token: string): Promise { - return canAccess(resource, 'get', token) +/** Used by SSE eventFilter after list checks fail; prefers SelfSubjectRulesReview over N SSARs. */ +export function canGetResource( + resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, + token: string +): Promise { + return resolveKindGetAccess(resource.kind, resource.apiVersion, token).then((access) => + applyKindGetAccess(access, resource, token) + ) +} + +function applyKindGetAccess( + access: KindGetAccess, + resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, + token: string, + onIncomplete?: () => Promise +): Promise { + switch (access.type) { + case 'deny-all': + return Promise.resolve(false) + case 'allow-all': + return Promise.resolve(true) + case 'allow-names': + return Promise.resolve(resource.metadata?.name ? access.names.has(resource.metadata.name) : false) + case 'incomplete': + return onIncomplete ? onIncomplete() : canAccess(resource, 'get', token) + } +} + +/** + * One SelfSubjectRulesReview per token (namespace "default"). + * ClusterRole bindings appear here; avoids an SSRR storm across MOCK_CLUSTERS namespaces. + */ +function getSubjectRules(token: string): Promise { + const cacheKey = hashAccessToken(token) + const existing = subjectRulesCache[cacheKey] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing.promise + } + + const promise = jsonPost<{ + status?: { + incomplete?: boolean + resourceRules?: SubjectRulesStatus['resourceRules'] + } + }>( + process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', + { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectRulesReview', + metadata: {}, + // Namespace is required by the API; ClusterRole bindings are included for any namespace. + spec: { namespace: 'default' }, + }, + token + ) + .then((result) => { + // jsonPost resolves on HTTP errors; treat non-2xx as review unavailable (SSAR fallback). + if (result.statusCode < 200 || result.statusCode >= 300) { + throw new Error(`SelfSubjectRulesReview failed with status ${result.statusCode}`) + } + return { + incomplete: result.body?.status?.incomplete ?? false, + resourceRules: result.body?.status?.resourceRules ?? [], + } + }) + .catch((err: unknown) => { + logger.warn({ msg: 'SelfSubjectRulesReview failed; falling back to per-object SSAR', error: err }) + // Do not retain a failed review under ACCESS_CACHE_TTL; next call should retry SSRR. + delete subjectRulesCache[cacheKey] + return { + incomplete: true, + unavailable: true, + resourceRules: [] as SubjectRulesStatus['resourceRules'], + } + }) + + subjectRulesCache[cacheKey] = { time: Date.now(), promise } + return promise +} + +function evaluateKindGetAccess(rules: SubjectRulesStatus, kind: string, apiVersion: string): KindGetAccess { + const group = apiVersion.includes('/') ? apiVersion.split('/')[0] : '' + const resourcePlural = pluralize(kind.toLowerCase()) + const accessVerbs = new Set(['get', 'list', 'watch']) + + let allowAll = false + const names = new Set() + + for (const rule of rules.resourceRules) { + const verbs = rule.verbs ?? [] + if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) continue + + const groups = rule.apiGroups ?? [] + if (!groups.includes('*') && !groups.includes(group)) continue + + const resources = rule.resources ?? [] + if (!resources.includes('*') && !resources.includes(resourcePlural)) continue + + const resourceNames = rule.resourceNames + if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { + allowAll = true + break + } + for (const name of resourceNames) names.add(name) + } + + switch (true) { + case allowAll: + return { type: 'allow-all' } + case names.size > 0: + return { type: 'allow-names', names } + // The review request failed; defer to the per-object SSAR fallback. + case rules.unavailable === true: + return { type: 'incomplete' } + // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules + // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. + case rules.resourceRules.length === 0: + return { type: 'deny-all' } + // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. + case rules.incomplete: + return { type: 'incomplete' } + default: + return { type: 'deny-all' } + } +} + +function resolveKindGetAccess(kind: string, apiVersion: string, token: string): Promise { + const cacheKey = `${hashAccessToken(token)}:${kind}:${apiVersion}` + const existing = kindGetAccessCache[cacheKey] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing.promise + } + + const promise = getSubjectRules(token).then((rules) => evaluateKindGetAccess(rules, kind, apiVersion)) + kindGetAccessCache[cacheKey] = { time: Date.now(), promise } + return promise } export function canAccess( @@ -668,12 +950,12 @@ export function canAccess( verb: 'get' | 'list' | 'create', token: string ): Promise { - // TODO make sure old cache items get cleaned up - - const key = `${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` - if (!accessCache[token]) accessCache[token] = {} - const existing = accessCache[token][key] - if (existing && existing.time > Date.now() - 60 * 1000) { + // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth + const tokenKey = hashAccessToken(token) + const key = `${verb}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` + if (!accessCache[tokenKey]) accessCache[tokenKey] = {} + const existing = accessCache[tokenKey][key] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { return existing.promise } @@ -696,29 +978,37 @@ export function canAccess( }, token ).then((result) => { + const allowed = result.body.status.allowed if (process.env.LOG_ACCESS === 'true') { logger.debug({ msg: 'access', - allowed: result.body.status.allowed, + allowed, verb, resource: pluralize(resource.kind.toLowerCase()), name: resource.metadata?.name, namespace: resource.metadata?.namespace, }) } - return result.body.status.allowed + // Replace in-flight promise with a settled boolean promise to drop large closures. + const entry = accessCache[tokenKey]?.[key] + if (entry && entry.promise === promise) { + entry.promise = Promise.resolve(allowed) + } + return allowed }) - accessCache[token][key] = { + accessCache[tokenKey][key] = { time: Date.now(), promise, } + enforceAccessCacheEntryCap(accessCache[tokenKey]) return promise } let stopping = false export function stopWatching(): void { stopping = true + stopAccessCacheCleanup() for (const request of requests) { request.cancel() } From c9379e94792032c35af45dfbba89b0b2f947c645 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Thu, 6 Aug 2026 06:01:30 +0200 Subject: [PATCH 02/10] ACM-39327: add events RBAC unit tests for release-2.13 Cover hashed access-cache behavior and SelfSubjectRulesReview short-circuit paths, including SSRR HTTP failure fallback to SSAR. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/test/routes/events.test.ts | 322 +++++++++++++++++++++++++++++ 1 file changed, 322 insertions(+) create mode 100644 backend/test/routes/events.test.ts diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts new file mode 100644 index 00000000000..6aac77c562c --- /dev/null +++ b/backend/test/routes/events.test.ts @@ -0,0 +1,322 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import nock from 'nock' +import { + canAccess, + canGetResource, + resetAccessCache, + getAccessCache, + cleanupAccessCache, + hashAccessToken, + ACCESS_CACHE_TTL, + ACCESS_CACHE_MAX_TOKENS, + ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, +} from '../../src/routes/events' + +describe('events Route RBAC (ACM-39327)', () => { + describe('Access Cache Cleanup', () => { + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) + + afterEach(() => { + resetAccessCache() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) + + it('should cache RBAC access check results under hashed token keys', async () => { + const mockToken = 'test-token-123' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + const result1 = await canAccess(resource, 'get', mockToken) + const result2 = await canAccess(resource, 'get', mockToken) + + expect(result1).toBe(true) + expect(result1).toBe(result2) + expect(getAccessCache()[mockToken]).toBeUndefined() + expect(getAccessCache()[hashAccessToken(mockToken)]['get:Pod:default:test-pod']).toBeDefined() + }) + + it('should use distinct cache keys per verb', async () => { + const mockToken = 'test-token-verb' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canAccess(resource, 'get', mockToken)).toBe(true) + expect(await canAccess(resource, 'list', mockToken)).toBe(false) + + const tokenCache = getAccessCache()[hashAccessToken(mockToken)] + expect(tokenCache['get:Pod:default:test-pod']).toBeDefined() + expect(tokenCache['list:Pod:default:test-pod']).toBeDefined() + }) + + it('should respect TTL and refetch after expiry', async () => { + const cache = getAccessCache() + const mockToken = 'test-token-ttl' + const tokenKey = hashAccessToken(mockToken) + + cache[tokenKey] = { + 'get:Secret:default:credentials': { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve(true), + }, + } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + const result = await canAccess( + { kind: 'Secret', apiVersion: 'v1', metadata: { namespace: 'default', name: 'credentials' } }, + 'get', + mockToken + ) + expect(result).toBe(false) + }) + + it('should remove stale cache entries during cleanup', () => { + const cache = getAccessCache() + const now = Date.now() + + cache['token1'] = { + stale: { time: now - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, + fresh: { time: now - 30000, promise: Promise.resolve(true) }, + } + cache['token2'] = { 'stale-only': { time: now - ACCESS_CACHE_TTL - 5000, promise: Promise.resolve(false) } } + + cleanupAccessCache() + + expect(cache['token1']['stale']).toBeUndefined() + expect(cache['token1']['fresh']).toBeDefined() + expect(cache['token2']).toBeUndefined() + }) + + it('should enforce maximum token limit with LRU eviction', () => { + const cache = getAccessCache() + const now = Date.now() + const tokenCount = ACCESS_CACHE_MAX_TOKENS + 100 + + for (let i = 0; i < tokenCount; i++) { + cache[`token-${i}`] = { + 'Pod:default:test': { time: now - (i / tokenCount) * 50 * 1000, promise: Promise.resolve(true) }, + } + } + + cleanupAccessCache() + + expect(Object.keys(cache).length).toBe(ACCESS_CACHE_MAX_TOKENS) + expect(cache['token-0']).toBeDefined() + expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() + }) + + it('should enforce maximum entries per token', () => { + const cache = getAccessCache() + const tokenKey = hashAccessToken('test-token-entry-cap') + const now = Date.now() + cache[tokenKey] = {} + + for (let i = 0; i < ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + 50; i++) { + cache[tokenKey][`get:Pod:default:pod-${i}`] = { + time: now - i, + promise: Promise.resolve(false), + } + } + + cleanupAccessCache() + + expect(Object.keys(cache[tokenKey]).length).toBeLessThanOrEqual(ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) + }) + }) + + /** + * ACM-39327: restricted users must not trigger O(N) SelfSubjectAccessReviews when the SSE + * filter falls through after cluster-scoped list is denied. One SelfSubjectRulesReview per + * token/kind short-circuits deny-all / allow-names without per-object or per-namespace SSARs. + */ + describe('SelfSubjectRulesReview short-circuit (ACM-39327)', () => { + const managedCluster = (name: string) => ({ + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name }, + }) + + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) + + afterEach(() => { + resetAccessCache() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) + + it('should deny all gets from complete empty rules without per-object SSAR', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, { status: { incomplete: false, resourceRules: [] } }) + + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'none-user-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should use a single rules review for many gets of the same kind', async () => { + let rulesCalls = 0 + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, () => { + rulesCalls++ + return { status: { incomplete: false, resourceRules: [] } } + }) + + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .times(1) + .reply(200, { status: { allowed: true } }) + + const results = await Promise.all( + Array.from({ length: 500 }, (_, i) => canGetResource(managedCluster(`cluster-${i}`), 'scale-none-token')) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(rulesCalls).toBe(1) + // Regression guard: must not fall back to per-object SSAR for deny-all. + expect(ssarScope.isDone()).toBe(false) + }) + + it('should use a single rules review across many namespaces of the same kind', async () => { + let rulesCalls = 0 + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, () => { + rulesCalls++ + return { status: { incomplete: true, resourceRules: [] } } + }) + + const results = await Promise.all( + Array.from({ length: 200 }, (_, i) => + canGetResource( + { + kind: 'ManagedClusterInfo', + apiVersion: 'internal.open-cluster-management.io/v1beta1', + metadata: { name: `cluster-${i}`, namespace: `cluster-${i}` }, + }, + 'namespaced-none-token' + ) + ) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(rulesCalls).toBe(1) + }) + + it('should allow only named resources from resourceNames rules', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, { + status: { + incomplete: false, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: ['allowed-cluster'], + }, + ], + }, + }) + + expect(await canGetResource(managedCluster('allowed-cluster'), 'partial-user-token')).toBe(true) + expect(await canGetResource(managedCluster('other-cluster'), 'partial-user-token')).toBe(false) + }) + + it('should allow all resources when rules grant unrestricted get/list/watch', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, { + status: { + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + }, + ], + }, + }) + + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(managedCluster('any-cluster'), 'viewer-token')).toBe(true) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should fall back to SSAR when rules review is incomplete with non-empty rules', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, { + status: { + incomplete: true, + resourceRules: [ + { + verbs: ['get'], + apiGroups: [''], + resources: ['pods'], + }, + ], + }, + }) + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should deny-all when rules are empty even if incomplete is true (OpenShift none user)', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, { status: { incomplete: true, resourceRules: [] } }) + + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'openshift-none-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) + + it('should fall back to SSAR when SelfSubjectRulesReview request fails', async () => { + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(500, { message: 'internal error' }) + + const ssarScope = nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'ssrr-fail-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + }) +}) From 7b72184c384bc98b43c66130b3e13bc30142ce9e Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Thu, 6 Aug 2026 17:16:48 +0200 Subject: [PATCH 03/10] fix sonar issues Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/events.ts | 144 +++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 64 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 49e84bc29b0..838ec566c81 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -194,49 +194,59 @@ function enforceAccessCacheEntryCap(tokenCache: Record = [] - - for (const token in accessCache) { - const tokenCache = accessCache[token] - let newestTime = 0 - - for (const key in tokenCache) { - if (tokenCache[key].time < cutoffTime) { - delete tokenCache[key] - } else if (tokenCache[key].time > newestTime) { - newestTime = tokenCache[key].time - } +function expireTimedEntries(cache: Record, cutoffTime: number) { + for (const key in cache) { + if (cache[key].time < cutoffTime) { + delete cache[key] } + } +} - if (Object.keys(tokenCache).length === 0) { - delete accessCache[token] - } else { - enforceAccessCacheEntryCap(tokenCache) - tokenStats.push({ token, newestTime }) +/** Prune one token's SSAR entries; returns newest remaining time, or undefined if the token was removed. */ +function pruneAccessCacheToken( + token: string, + tokenCache: Record }>, + cutoffTime: number +): number | undefined { + let newestTime = 0 + + for (const key in tokenCache) { + if (tokenCache[key].time < cutoffTime) { + delete tokenCache[key] + } else if (tokenCache[key].time > newestTime) { + newestTime = tokenCache[key].time } } - for (const key in subjectRulesCache) { - if (subjectRulesCache[key].time < cutoffTime) { - delete subjectRulesCache[key] - } + if (Object.keys(tokenCache).length === 0) { + delete accessCache[token] + return undefined } - for (const key in kindGetAccessCache) { - if (kindGetAccessCache[key].time < cutoffTime) { - delete kindGetAccessCache[key] + + enforceAccessCacheEntryCap(tokenCache) + return newestTime +} + +export function cleanupAccessCache() { + const cutoffTime = Date.now() - ACCESS_CACHE_TTL + const tokenStats: Array<{ token: string; newestTime: number }> = [] + + for (const token in accessCache) { + const newestTime = pruneAccessCacheToken(token, accessCache[token], cutoffTime) + if (newestTime !== undefined) { + tokenStats.push({ token, newestTime }) } } - if (tokenStats.length > ACCESS_CACHE_MAX_TOKENS) { - tokenStats.sort((a, b) => a.newestTime - b.newestTime) - const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS + expireTimedEntries(subjectRulesCache, cutoffTime) + expireTimedEntries(kindGetAccessCache, cutoffTime) - for (let i = 0; i < tokensToRemove; i++) { - delete accessCache[tokenStats[i].token] - } + if (tokenStats.length <= ACCESS_CACHE_MAX_TOKENS) return + + tokenStats.sort((a, b) => a.newestTime - b.newestTime) + const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS + for (let i = 0; i < tokensToRemove; i++) { + delete accessCache[tokenStats[i].token] } } @@ -887,6 +897,28 @@ function getSubjectRules(token: string): Promise { return promise } +function ruleGrantsKindAccess( + rule: SubjectRulesStatus['resourceRules'][number], + group: string, + resourcePlural: string, + accessVerbs: Set +): { allowAll: true } | { names: string[] } | null { + const verbs = rule.verbs ?? [] + if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) return null + + const groups = rule.apiGroups ?? [] + if (!groups.includes('*') && !groups.includes(group)) return null + + const resources = rule.resources ?? [] + if (!resources.includes('*') && !resources.includes(resourcePlural)) return null + + const resourceNames = rule.resourceNames + if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { + return { allowAll: true } + } + return { names: resourceNames } +} + function evaluateKindGetAccess(rules: SubjectRulesStatus, kind: string, apiVersion: string): KindGetAccess { const group = apiVersion.includes('/') ? apiVersion.split('/')[0] : '' const resourcePlural = pluralize(kind.toLowerCase()) @@ -896,41 +928,25 @@ function evaluateKindGetAccess(rules: SubjectRulesStatus, kind: string, apiVersi const names = new Set() for (const rule of rules.resourceRules) { - const verbs = rule.verbs ?? [] - if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) continue - - const groups = rule.apiGroups ?? [] - if (!groups.includes('*') && !groups.includes(group)) continue - - const resources = rule.resources ?? [] - if (!resources.includes('*') && !resources.includes(resourcePlural)) continue - - const resourceNames = rule.resourceNames - if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { + const match = ruleGrantsKindAccess(rule, group, resourcePlural, accessVerbs) + if (!match) continue + if ('allowAll' in match) { allowAll = true break } - for (const name of resourceNames) names.add(name) + for (const name of match.names) names.add(name) } - switch (true) { - case allowAll: - return { type: 'allow-all' } - case names.size > 0: - return { type: 'allow-names', names } - // The review request failed; defer to the per-object SSAR fallback. - case rules.unavailable === true: - return { type: 'incomplete' } - // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules - // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. - case rules.resourceRules.length === 0: - return { type: 'deny-all' } - // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. - case rules.incomplete: - return { type: 'incomplete' } - default: - return { type: 'deny-all' } - } + if (allowAll) return { type: 'allow-all' } + if (names.size > 0) return { type: 'allow-names', names } + // The review request failed; defer to the per-object SSAR fallback. + if (rules.unavailable === true) return { type: 'incomplete' } + // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules + // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. + if (rules.resourceRules.length === 0) return { type: 'deny-all' } + // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. + if (rules.incomplete) return { type: 'incomplete' } + return { type: 'deny-all' } } function resolveKindGetAccess(kind: string, apiVersion: string, token: string): Promise { @@ -991,7 +1007,7 @@ export function canAccess( } // Replace in-flight promise with a settled boolean promise to drop large closures. const entry = accessCache[tokenKey]?.[key] - if (entry && entry.promise === promise) { + if (entry?.promise === promise) { entry.promise = Promise.resolve(allowed) } return allowed From f7c100a95dacc1ae105e5d7bc63c9106a95d7f45 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Mon, 17 Aug 2026 18:13:42 +0200 Subject: [PATCH 04/10] ACM-39327: namespace-aware SSRR and cluster-scoped SSAR confirmation - Cache SelfSubjectRulesReview per token+namespace instead of a single `default`-namespace review, so namespaced permissions are evaluated in the resource's own namespace. - For cluster-scoped resources, confirm unrestricted SSRR grants with SSAR to prevent RoleBindings in `default` from impersonating cluster-scoped access. - Update unit tests to cover per-namespace caching and namespace isolation. Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/events.ts | 114 +++++++------ backend/test/routes/events.test.ts | 261 ++++++++++++++++++++--------- 2 files changed, 249 insertions(+), 126 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 838ec566c81..8d5ee8d0823 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -152,6 +152,11 @@ type KindGetAccess = | { type: 'allow-names'; names: Set } | { type: 'incomplete' } +type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } + +/** SSRR requires a namespace; ClusterRoleBindings are included in every namespace review. */ +const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' + const subjectRulesCache: Record }> = {} const kindGetAccessCache: Record }> = {} @@ -774,18 +779,8 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { if (allowed) return true - // After cluster list is denied, use one SelfSubjectRulesReview per token/kind. - // Do NOT fall through to namespaced list SSAR first — that is O(namespaces) and - // OOMs/hangs restricted users when MOCK_CLUSTERS (or real inventory) is large. - return resolveKindGetAccess(resource.kind, resource.apiVersion, token).then((access) => - applyKindGetAccess(access, resource, token, () => - // Authorizer could not enumerate rules; keep prior namespaced-list then get fallback. - canListNamespacedScopedKind(resource, token).then((nsAllowed) => { - if (nsAllowed) return true - return canAccess(resource, 'get', token) - }) - ) - ) + // After cluster list is denied, use SelfSubjectRulesReview instead of O(N) SSARs. + return canGetResource(resource, token) }) } default: @@ -794,17 +789,11 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { +function canListClusterScopedKind(resource: AccessResource, token: string): Promise { return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) } -function canListNamespacedScopedKind( - resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, - token: string -): Promise { +function canListNamespacedScopedKind(resource: AccessResource, token: string): Promise { if (!resource.metadata?.namespace) return Promise.resolve(false) return canAccess( { @@ -817,19 +806,48 @@ function canListNamespacedScopedKind( ) } -/** Used by SSE eventFilter after list checks fail; prefers SelfSubjectRulesReview over N SSARs. */ -export function canGetResource( - resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, - token: string -): Promise { - return resolveKindGetAccess(resource.kind, resource.apiVersion, token).then((access) => - applyKindGetAccess(access, resource, token) - ) +function apiGroupFromVersion(apiVersion: string): string { + return apiVersion.includes('/') ? apiVersion.split('/')[0] : '' +} + +function resourcePluralName(kind: string): string { + return pluralize(kind.toLowerCase()) +} + +function isNamespacedResource(resource: AccessResource): boolean { + return Boolean(resource.metadata?.namespace) +} + +function rulesNamespaceFor(resource: AccessResource): string { + return resource.metadata?.namespace || CLUSTER_SCOPED_RULES_NAMESPACE +} + +/** + * Used by SSE eventFilter after cluster-scoped list is denied. + * Namespaced resources are reviewed in the resource's namespace (cached per token+namespace). + * Cluster-scoped resources use a probe-namespace review only as a negative/named-binding cache; + * unrestricted grants from that probe are confirmed with SSAR so RoleBindings in `default` + * cannot impersonate cluster-scoped access. + */ +export function canGetResource(resource: AccessResource, token: string): Promise { + return resolveKindGetAccess(resource, token).then((access) => { + // Probe-namespace SSRR cannot distinguish RoleBindings from ClusterRoleBindings. + // Confirm unrestricted cluster-scoped grants with SSAR to close the default-ns proxy hole. + if (!isNamespacedResource(resource) && access.type === 'allow-all') { + return canAccess(resource, 'get', token) + } + return applyKindGetAccess(access, resource, token, () => + canListNamespacedScopedKind(resource, token).then((nsAllowed) => { + if (nsAllowed) return true + return canAccess(resource, 'get', token) + }) + ) + }) } function applyKindGetAccess( access: KindGetAccess, - resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, + resource: AccessResource, token: string, onIncomplete?: () => Promise ): Promise { @@ -846,11 +864,11 @@ function applyKindGetAccess( } /** - * One SelfSubjectRulesReview per token (namespace "default"). - * ClusterRole bindings appear here; avoids an SSRR storm across MOCK_CLUSTERS namespaces. + * One SelfSubjectRulesReview per token+namespace. + * ClusterRoleBindings appear in every namespace; RoleBindings appear only in their namespace. */ -function getSubjectRules(token: string): Promise { - const cacheKey = hashAccessToken(token) +function getSubjectRules(token: string, namespace: string): Promise { + const cacheKey = `${hashAccessToken(token)}:${namespace}` const existing = subjectRulesCache[cacheKey] if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { return existing.promise @@ -867,8 +885,7 @@ function getSubjectRules(token: string): Promise { apiVersion: 'authorization.k8s.io/v1', kind: 'SelfSubjectRulesReview', metadata: {}, - // Namespace is required by the API; ClusterRole bindings are included for any namespace. - spec: { namespace: 'default' }, + spec: { namespace }, }, token ) @@ -919,9 +936,7 @@ function ruleGrantsKindAccess( return { names: resourceNames } } -function evaluateKindGetAccess(rules: SubjectRulesStatus, kind: string, apiVersion: string): KindGetAccess { - const group = apiVersion.includes('/') ? apiVersion.split('/')[0] : '' - const resourcePlural = pluralize(kind.toLowerCase()) +function evaluateKindGetAccess(rules: SubjectRulesStatus, group: string, resourcePlural: string): KindGetAccess { const accessVerbs = new Set(['get', 'list', 'watch']) let allowAll = false @@ -949,23 +964,23 @@ function evaluateKindGetAccess(rules: SubjectRulesStatus, kind: string, apiVersi return { type: 'deny-all' } } -function resolveKindGetAccess(kind: string, apiVersion: string, token: string): Promise { - const cacheKey = `${hashAccessToken(token)}:${kind}:${apiVersion}` +function resolveKindGetAccess(resource: AccessResource, token: string): Promise { + const group = apiGroupFromVersion(resource.apiVersion) + const plural = resourcePluralName(resource.kind) + const namespace = rulesNamespaceFor(resource) + // Permission checks are by API group, not version; keep cache keys version-free. + const cacheKey = `${hashAccessToken(token)}:${namespace}:${group}:${plural}` const existing = kindGetAccessCache[cacheKey] if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { return existing.promise } - const promise = getSubjectRules(token).then((rules) => evaluateKindGetAccess(rules, kind, apiVersion)) + const promise = getSubjectRules(token, namespace).then((rules) => evaluateKindGetAccess(rules, group, plural)) kindGetAccessCache[cacheKey] = { time: Date.now(), promise } return promise } -export function canAccess( - resource: { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } }, - verb: 'get' | 'list' | 'create', - token: string -): Promise { +export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'create', token: string): Promise { // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth const tokenKey = hashAccessToken(token) const key = `${verb}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` @@ -975,6 +990,7 @@ export function canAccess( return existing.promise } + const resourceName = resourcePluralName(resource.kind) const promise = jsonPost<{ status: { allowed: boolean } }>( process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', { @@ -983,11 +999,11 @@ export function canAccess( metadata: {}, spec: { resourceAttributes: { - group: resource.apiVersion.includes('/') ? resource.apiVersion.split('/')[0] : '', + group: apiGroupFromVersion(resource.apiVersion), name: resource.metadata?.name, namespace: resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), - resource: pluralize(resource.kind.toLowerCase()), + resource: resourceName, verb, }, }, @@ -1000,7 +1016,7 @@ export function canAccess( msg: 'access', allowed, verb, - resource: pluralize(resource.kind.toLowerCase()), + resource: resourceName, name: resource.metadata?.name, namespace: resource.metadata?.namespace, }) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index 6aac77c562c..e8702ab1b6a 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -141,15 +141,54 @@ describe('events Route RBAC (ACM-39327)', () => { /** * ACM-39327: restricted users must not trigger O(N) SelfSubjectAccessReviews when the SSE - * filter falls through after cluster-scoped list is denied. One SelfSubjectRulesReview per - * token/kind short-circuits deny-all / allow-names without per-object or per-namespace SSARs. + * filter falls through after cluster-scoped list is denied. SelfSubjectRulesReview is + * namespaced: cache one review per token+namespace, never treat `default` as global allow. */ describe('SelfSubjectRulesReview short-circuit (ACM-39327)', () => { + const apiUrl = () => process.env.CLUSTER_API_URL || '' const managedCluster = (name: string) => ({ kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', metadata: { name }, }) + const secret = (namespace: string, name: string) => ({ + kind: 'Secret', + apiVersion: 'v1', + metadata: { namespace, name }, + }) + const managedClusterInfo = (cluster: string) => ({ + kind: 'ManagedClusterInfo', + apiVersion: 'internal.open-cluster-management.io/v1beta1', + metadata: { name: cluster, namespace: cluster }, + }) + + const emptyRules = { incomplete: false, resourceRules: [] as unknown[] } + const secretGetInNamespace = { + incomplete: false, + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['secrets'] }], + } + const clusterAdminRules = { + incomplete: false, + resourceRules: [{ verbs: ['*'], apiGroups: ['*'], resources: ['*'] }], + } + + function rulesReviewNamespace(body: unknown): string { + let parsed = body + if (typeof body === 'string') { + try { + parsed = JSON.parse(body) as unknown + } catch { + return '' + } + } + return (parsed as { spec?: { namespace?: string } })?.spec?.namespace || '' + } + + function nockRulesReview(replyFn: (namespace: string) => { incomplete?: boolean; resourceRules?: unknown[] }) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) + } beforeEach(() => { resetAccessCache() @@ -163,11 +202,9 @@ describe('events Route RBAC (ACM-39327)', () => { }) it('should deny all gets from complete empty rules without per-object SSAR', async () => { - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, { status: { incomplete: false, resourceRules: [] } }) + nockRulesReview(() => emptyRules) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: true } }) @@ -175,16 +212,14 @@ describe('events Route RBAC (ACM-39327)', () => { expect(ssarScope.isDone()).toBe(false) }) - it('should use a single rules review for many gets of the same kind', async () => { + it('should use a single rules review for many cluster-scoped gets of the same kind', async () => { let rulesCalls = 0 - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, () => { - rulesCalls++ - return { status: { incomplete: false, resourceRules: [] } } - }) + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .times(1) .reply(200, { status: { allowed: true } }) @@ -195,18 +230,15 @@ describe('events Route RBAC (ACM-39327)', () => { expect(results.every((allowed) => allowed === false)).toBe(true) expect(rulesCalls).toBe(1) - // Regression guard: must not fall back to per-object SSAR for deny-all. expect(ssarScope.isDone()).toBe(false) }) - it('should use a single rules review across many namespaces of the same kind', async () => { + it('should use a single rules review for many namespaced gets in the same namespace', async () => { let rulesCalls = 0 - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, () => { - rulesCalls++ - return { status: { incomplete: true, resourceRules: [] } } - }) + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) const results = await Promise.all( Array.from({ length: 200 }, (_, i) => @@ -214,9 +246,9 @@ describe('events Route RBAC (ACM-39327)', () => { { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1', - metadata: { name: `cluster-${i}`, namespace: `cluster-${i}` }, + metadata: { name: `info-${i}`, namespace: 'acm39327-mc-01' }, }, - 'namespaced-none-token' + 'same-ns-none-token' ) ) ) @@ -225,67 +257,144 @@ describe('events Route RBAC (ACM-39327)', () => { expect(rulesCalls).toBe(1) }) - it('should allow only named resources from resourceNames rules', async () => { - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, { - status: { - incomplete: false, - resourceRules: [ - { - verbs: ['get'], - apiGroups: ['cluster.open-cluster-management.io'], - resources: ['managedclusters'], - resourceNames: ['allowed-cluster'], - }, - ], - }, + it('should issue one rules review per namespace for a none user, without per-object SSAR', async () => { + const namespaces = new Set() + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + namespaces.add(rulesReviewNamespace(body)) + return true + }) + .times(50) + .reply(200, { status: emptyRules }) + + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + const results = await Promise.all( + Array.from({ length: 50 }, (_, i) => + canGetResource(managedClusterInfo(`cluster-${i}`), 'namespaced-none-token') + ) + ) + + expect(results.every((allowed) => allowed === false)).toBe(true) + expect(namespaces.size).toBe(50) + expect(ssarScope.isDone()).toBe(false) + }) + + it('must not treat get secrets in default as access to Credentials in other namespaces', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: secretGetInNamespace }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) !== 'default' }) + .times(2) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(secret('default', 'default-cred'), 'user1-token')).toBe(true) + expect(await canGetResource(secret('kube-system', 'other-cred'), 'user1-token')).toBe(false) + expect(await canGetResource(secret('acm39327-mc-01', 'cluster-cred'), 'user1-token')).toBe(false) + }) + + it('should allow namespaced cluster resources when the user is admin in that cluster namespace', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'acm39327-mc-01' + }) + .reply(200, { status: clusterAdminRules }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) !== 'acm39327-mc-01' + }) + .times(2) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(managedClusterInfo('acm39327-mc-01'), 'cluster-admin-token')).toBe(true) + expect(await canGetResource(managedClusterInfo('other-cluster'), 'cluster-admin-token')).toBe(false) + expect(await canGetResource(managedCluster('acm39327-mc-01'), 'cluster-admin-token')).toBe(false) + }) + + it('should allow only named cluster-scoped resources from resourceNames rules', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: ['allowed-cluster'], + }, + ], + })) expect(await canGetResource(managedCluster('allowed-cluster'), 'partial-user-token')).toBe(true) expect(await canGetResource(managedCluster('other-cluster'), 'partial-user-token')).toBe(false) }) - it('should allow all resources when rules grant unrestricted get/list/watch', async () => { - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, { - status: { - incomplete: false, - resourceRules: [ - { - verbs: ['get', 'list', 'watch'], - apiGroups: ['cluster.open-cluster-management.io'], - resources: ['managedclusters'], - }, - ], - }, - }) + it('should allow namespaced resources when rules grant unrestricted get/list/watch in that namespace', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [{ verbs: ['get', 'list', 'watch'], apiGroups: [''], resources: ['secrets'] }], + })) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: false } }) - expect(await canGetResource(managedCluster('any-cluster'), 'viewer-token')).toBe(true) + expect(await canGetResource(secret('default', 'any-secret'), 'viewer-token')).toBe(true) expect(ssarScope.isDone()).toBe(false) }) - it('should fall back to SSAR when rules review is incomplete with non-empty rules', async () => { - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, { - status: { - incomplete: true, - resourceRules: [ - { - verbs: ['get'], - apiGroups: [''], - resources: ['pods'], - }, - ], + it('should confirm cluster-scoped allow-all with SSAR so default RoleBindings are not treated as cluster access', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], }, - }) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + ], + })) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) + }) + + it('should reuse kind access across API versions of the same group', async () => { + let rulesCalls = 0 + nockRulesReview(() => { + rulesCalls++ + return emptyRules + }) + + expect( + await canGetResource( + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1', metadata: { name: 'p1' } }, + 'version-token' + ) + ).toBe(false) + expect( + await canGetResource( + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1alpha1', metadata: { name: 'p2' } }, + 'version-token' + ) + ).toBe(false) + expect(rulesCalls).toBe(1) + }) + + it('should fall back to SSAR when rules review is incomplete with non-empty rules', async () => { + nockRulesReview(() => ({ + incomplete: true, + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['pods'] }], + })) + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: true } }) @@ -294,11 +403,9 @@ describe('events Route RBAC (ACM-39327)', () => { }) it('should deny-all when rules are empty even if incomplete is true (OpenShift none user)', async () => { - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, { status: { incomplete: true, resourceRules: [] } }) + nockRulesReview(() => ({ incomplete: true, resourceRules: [] })) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: true } }) @@ -307,11 +414,11 @@ describe('events Route RBAC (ACM-39327)', () => { }) it('should fall back to SSAR when SelfSubjectRulesReview request fails', async () => { - nock(process.env.CLUSTER_API_URL || '') + nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') .reply(500, { message: 'internal error' }) - const ssarScope = nock(process.env.CLUSTER_API_URL || '') + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: true } }) From 3d7a4886a22969eb7df7a55f3eec00972b4af56e Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 18 Aug 2026 10:19:47 +0200 Subject: [PATCH 05/10] Extract events RBAC access and cache logic into dedicated modules Split `events.ts` into `eventsAccess.ts` for RBAC evaluation and `eventsCache.ts` for access cache management, and add unit tests for both new modules. Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/events.ts | 410 +----------------- backend/src/routes/eventsAccess.ts | 284 ++++++++++++ backend/src/routes/eventsCache.ts | 187 ++++++++ .../{events.test.ts => eventsAccess.test.ts} | 291 +++++-------- backend/test/routes/eventsCache.test.ts | 250 +++++++++++ 5 files changed, 850 insertions(+), 572 deletions(-) create mode 100644 backend/src/routes/eventsAccess.ts create mode 100644 backend/src/routes/eventsCache.ts rename backend/test/routes/{events.test.ts => eventsAccess.test.ts} (60%) create mode 100644 backend/test/routes/eventsCache.test.ts diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 8d5ee8d0823..e5f402620a2 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -1,7 +1,6 @@ /* Copyright Contributors to the Open Cluster Management project */ /* eslint-disable no-constant-condition */ -import { createHash } from 'node:crypto' import eventStream from 'event-stream' import get from 'get-value' import got, { CancelError, HTTPError, TimeoutError } from 'got' @@ -9,13 +8,26 @@ import { Http2ServerRequest, Http2ServerResponse } from 'node:http2' import pluralize from 'pluralize' import { Stream } from 'stream' import { promisify } from 'util' -import { jsonPost } from '../lib/json-request' import { logger } from '../lib/logger' import type { ITransformedResource } from '../lib/pagination' import { type ServerSideEvent, ServerSideEvents } from '../lib/server-side-events' import { getCACertificate, getServiceAccountToken } from '../lib/serviceAccountToken' import { getAuthenticatedToken } from '../lib/token' import type { IResource } from '../resources/resource' +import { canAccess, canGetResource, canListClusterScopedKind, canListNamespacedScopedKind } from './eventsAccess' +import { startAccessCacheCleanup, stopAccessCacheCleanup } from './eventsCache' + +export { + ACCESS_CACHE_TTL, + ACCESS_CACHE_CLEANUP_INTERVAL, + ACCESS_CACHE_MAX_TOKENS, + ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, + cleanupAccessCache, + getAccessCache, + hashAccessToken, + resetAccessCache, +} from './eventsCache' +export { canAccess, canGetResource } from './eventsAccess' const { map, split } = eventStream const pipeline = promisify(Stream.pipeline) @@ -132,152 +144,6 @@ export function initResourceCache(cache: ResourceCache) { export let resourceCache: ResourceCache = {} -const accessCache: Record }>> = {} - -interface SubjectRulesStatus { - incomplete: boolean - /** True when the SelfSubjectRulesReview request itself failed. */ - unavailable?: boolean - resourceRules: Array<{ - verbs?: string[] - apiGroups?: string[] - resources?: string[] - resourceNames?: string[] - }> -} - -type KindGetAccess = - | { type: 'deny-all' } - | { type: 'allow-all' } - | { type: 'allow-names'; names: Set } - | { type: 'incomplete' } - -type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } - -/** SSRR requires a namespace; ClusterRoleBindings are included in every namespace review. */ -const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' - -const subjectRulesCache: Record }> = {} -const kindGetAccessCache: Record }> = {} - -/** Clear all cached RBAC access checks. Used for test isolation. */ -export function resetAccessCache() { - for (const key in accessCache) { - delete accessCache[key] - } - for (const key in subjectRulesCache) { - delete subjectRulesCache[key] - } - for (const key in kindGetAccessCache) { - delete kindGetAccessCache[key] - } -} - -export function getAccessCache() { - return accessCache -} - -export const ACCESS_CACHE_TTL = 60 * 1000 // 60 seconds -export const ACCESS_CACHE_CLEANUP_INTERVAL = 90 * 1000 // 90 seconds -export const ACCESS_CACHE_MAX_TOKENS = 1000 // Maximum number of token entries to keep -export const ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN = 2000 // Cap RBAC keys retained per token - -/** Hash bearer tokens so the access cache does not retain full JWTs as object keys. */ -export function hashAccessToken(token: string): string { - return createHash('sha256').update(token).digest('hex') -} - -function enforceAccessCacheEntryCap(tokenCache: Record }>) { - const keys = Object.keys(tokenCache) - if (keys.length <= ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) return - keys.sort((a, b) => tokenCache[a].time - tokenCache[b].time) - const toRemove = keys.length - ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN - for (let i = 0; i < toRemove; i++) { - delete tokenCache[keys[i]] - } -} - -let accessCacheCleanupTimer: NodeJS.Timeout | undefined - -function expireTimedEntries(cache: Record, cutoffTime: number) { - for (const key in cache) { - if (cache[key].time < cutoffTime) { - delete cache[key] - } - } -} - -/** Prune one token's SSAR entries; returns newest remaining time, or undefined if the token was removed. */ -function pruneAccessCacheToken( - token: string, - tokenCache: Record }>, - cutoffTime: number -): number | undefined { - let newestTime = 0 - - for (const key in tokenCache) { - if (tokenCache[key].time < cutoffTime) { - delete tokenCache[key] - } else if (tokenCache[key].time > newestTime) { - newestTime = tokenCache[key].time - } - } - - if (Object.keys(tokenCache).length === 0) { - delete accessCache[token] - return undefined - } - - enforceAccessCacheEntryCap(tokenCache) - return newestTime -} - -export function cleanupAccessCache() { - const cutoffTime = Date.now() - ACCESS_CACHE_TTL - const tokenStats: Array<{ token: string; newestTime: number }> = [] - - for (const token in accessCache) { - const newestTime = pruneAccessCacheToken(token, accessCache[token], cutoffTime) - if (newestTime !== undefined) { - tokenStats.push({ token, newestTime }) - } - } - - expireTimedEntries(subjectRulesCache, cutoffTime) - expireTimedEntries(kindGetAccessCache, cutoffTime) - - if (tokenStats.length <= ACCESS_CACHE_MAX_TOKENS) return - - tokenStats.sort((a, b) => a.newestTime - b.newestTime) - const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS - for (let i = 0; i < tokensToRemove; i++) { - delete accessCache[tokenStats[i].token] - } -} - -function startAccessCacheCleanup() { - if (accessCacheCleanupTimer) return - - accessCacheCleanupTimer = setInterval(() => { - try { - cleanupAccessCache() - } catch (err: unknown) { - logger.error({ msg: 'accessCache cleanup failed', error: err }) - } - }, ACCESS_CACHE_CLEANUP_INTERVAL) - - accessCacheCleanupTimer.unref() - logger.info({ msg: 'accessCache cleanup started', interval: ACCESS_CACHE_CLEANUP_INTERVAL }) -} - -function stopAccessCacheCleanup() { - if (accessCacheCleanupTimer) { - clearInterval(accessCacheCleanupTimer) - accessCacheCleanupTimer = undefined - logger.info({ msg: 'accessCache cleanup stopped' }) - } -} - const definitions: IWatchOptions[] = [ { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, @@ -789,254 +655,6 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { - return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) -} - -function canListNamespacedScopedKind(resource: AccessResource, token: string): Promise { - if (!resource.metadata?.namespace) return Promise.resolve(false) - return canAccess( - { - kind: resource.kind, - apiVersion: resource.apiVersion, - metadata: { namespace: resource.metadata.namespace }, - }, - 'list', - token - ) -} - -function apiGroupFromVersion(apiVersion: string): string { - return apiVersion.includes('/') ? apiVersion.split('/')[0] : '' -} - -function resourcePluralName(kind: string): string { - return pluralize(kind.toLowerCase()) -} - -function isNamespacedResource(resource: AccessResource): boolean { - return Boolean(resource.metadata?.namespace) -} - -function rulesNamespaceFor(resource: AccessResource): string { - return resource.metadata?.namespace || CLUSTER_SCOPED_RULES_NAMESPACE -} - -/** - * Used by SSE eventFilter after cluster-scoped list is denied. - * Namespaced resources are reviewed in the resource's namespace (cached per token+namespace). - * Cluster-scoped resources use a probe-namespace review only as a negative/named-binding cache; - * unrestricted grants from that probe are confirmed with SSAR so RoleBindings in `default` - * cannot impersonate cluster-scoped access. - */ -export function canGetResource(resource: AccessResource, token: string): Promise { - return resolveKindGetAccess(resource, token).then((access) => { - // Probe-namespace SSRR cannot distinguish RoleBindings from ClusterRoleBindings. - // Confirm unrestricted cluster-scoped grants with SSAR to close the default-ns proxy hole. - if (!isNamespacedResource(resource) && access.type === 'allow-all') { - return canAccess(resource, 'get', token) - } - return applyKindGetAccess(access, resource, token, () => - canListNamespacedScopedKind(resource, token).then((nsAllowed) => { - if (nsAllowed) return true - return canAccess(resource, 'get', token) - }) - ) - }) -} - -function applyKindGetAccess( - access: KindGetAccess, - resource: AccessResource, - token: string, - onIncomplete?: () => Promise -): Promise { - switch (access.type) { - case 'deny-all': - return Promise.resolve(false) - case 'allow-all': - return Promise.resolve(true) - case 'allow-names': - return Promise.resolve(resource.metadata?.name ? access.names.has(resource.metadata.name) : false) - case 'incomplete': - return onIncomplete ? onIncomplete() : canAccess(resource, 'get', token) - } -} - -/** - * One SelfSubjectRulesReview per token+namespace. - * ClusterRoleBindings appear in every namespace; RoleBindings appear only in their namespace. - */ -function getSubjectRules(token: string, namespace: string): Promise { - const cacheKey = `${hashAccessToken(token)}:${namespace}` - const existing = subjectRulesCache[cacheKey] - if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { - return existing.promise - } - - const promise = jsonPost<{ - status?: { - incomplete?: boolean - resourceRules?: SubjectRulesStatus['resourceRules'] - } - }>( - process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', - { - apiVersion: 'authorization.k8s.io/v1', - kind: 'SelfSubjectRulesReview', - metadata: {}, - spec: { namespace }, - }, - token - ) - .then((result) => { - // jsonPost resolves on HTTP errors; treat non-2xx as review unavailable (SSAR fallback). - if (result.statusCode < 200 || result.statusCode >= 300) { - throw new Error(`SelfSubjectRulesReview failed with status ${result.statusCode}`) - } - return { - incomplete: result.body?.status?.incomplete ?? false, - resourceRules: result.body?.status?.resourceRules ?? [], - } - }) - .catch((err: unknown) => { - logger.warn({ msg: 'SelfSubjectRulesReview failed; falling back to per-object SSAR', error: err }) - // Do not retain a failed review under ACCESS_CACHE_TTL; next call should retry SSRR. - delete subjectRulesCache[cacheKey] - return { - incomplete: true, - unavailable: true, - resourceRules: [] as SubjectRulesStatus['resourceRules'], - } - }) - - subjectRulesCache[cacheKey] = { time: Date.now(), promise } - return promise -} - -function ruleGrantsKindAccess( - rule: SubjectRulesStatus['resourceRules'][number], - group: string, - resourcePlural: string, - accessVerbs: Set -): { allowAll: true } | { names: string[] } | null { - const verbs = rule.verbs ?? [] - if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) return null - - const groups = rule.apiGroups ?? [] - if (!groups.includes('*') && !groups.includes(group)) return null - - const resources = rule.resources ?? [] - if (!resources.includes('*') && !resources.includes(resourcePlural)) return null - - const resourceNames = rule.resourceNames - if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { - return { allowAll: true } - } - return { names: resourceNames } -} - -function evaluateKindGetAccess(rules: SubjectRulesStatus, group: string, resourcePlural: string): KindGetAccess { - const accessVerbs = new Set(['get', 'list', 'watch']) - - let allowAll = false - const names = new Set() - - for (const rule of rules.resourceRules) { - const match = ruleGrantsKindAccess(rule, group, resourcePlural, accessVerbs) - if (!match) continue - if ('allowAll' in match) { - allowAll = true - break - } - for (const name of match.names) names.add(name) - } - - if (allowAll) return { type: 'allow-all' } - if (names.size > 0) return { type: 'allow-names', names } - // The review request failed; defer to the per-object SSAR fallback. - if (rules.unavailable === true) return { type: 'incomplete' } - // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules - // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. - if (rules.resourceRules.length === 0) return { type: 'deny-all' } - // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. - if (rules.incomplete) return { type: 'incomplete' } - return { type: 'deny-all' } -} - -function resolveKindGetAccess(resource: AccessResource, token: string): Promise { - const group = apiGroupFromVersion(resource.apiVersion) - const plural = resourcePluralName(resource.kind) - const namespace = rulesNamespaceFor(resource) - // Permission checks are by API group, not version; keep cache keys version-free. - const cacheKey = `${hashAccessToken(token)}:${namespace}:${group}:${plural}` - const existing = kindGetAccessCache[cacheKey] - if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { - return existing.promise - } - - const promise = getSubjectRules(token, namespace).then((rules) => evaluateKindGetAccess(rules, group, plural)) - kindGetAccessCache[cacheKey] = { time: Date.now(), promise } - return promise -} - -export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'create', token: string): Promise { - // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth - const tokenKey = hashAccessToken(token) - const key = `${verb}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` - if (!accessCache[tokenKey]) accessCache[tokenKey] = {} - const existing = accessCache[tokenKey][key] - if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { - return existing.promise - } - - const resourceName = resourcePluralName(resource.kind) - const promise = jsonPost<{ status: { allowed: boolean } }>( - process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', - { - apiVersion: 'authorization.k8s.io/v1', - kind: 'SelfSubjectAccessReview', - metadata: {}, - spec: { - resourceAttributes: { - group: apiGroupFromVersion(resource.apiVersion), - name: resource.metadata?.name, - namespace: - resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), - resource: resourceName, - verb, - }, - }, - }, - token - ).then((result) => { - const allowed = result.body.status.allowed - if (process.env.LOG_ACCESS === 'true') { - logger.debug({ - msg: 'access', - allowed, - verb, - resource: resourceName, - name: resource.metadata?.name, - namespace: resource.metadata?.namespace, - }) - } - // Replace in-flight promise with a settled boolean promise to drop large closures. - const entry = accessCache[tokenKey]?.[key] - if (entry?.promise === promise) { - entry.promise = Promise.resolve(allowed) - } - return allowed - }) - - accessCache[tokenKey][key] = { - time: Date.now(), - promise, - } - enforceAccessCacheEntryCap(accessCache[tokenKey]) - return promise -} - let stopping = false export function stopWatching(): void { stopping = true diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts new file mode 100644 index 00000000000..c02321085ac --- /dev/null +++ b/backend/src/routes/eventsAccess.ts @@ -0,0 +1,284 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import pluralize from 'pluralize' +import { jsonPost } from '../lib/json-request' +import { logger } from '../lib/logger' +import { + deleteTimedCacheEntry, + getKindGetAccessCacheStore, + getSubjectRulesCacheStore, + getSsarCacheEntry, + getTimedCacheEntry, + hashAccessToken, + replaceSsarCachePromise, + setSsarCacheEntry, + setTimedCacheEntry, +} from './eventsCache' + +export interface SubjectRulesStatus { + incomplete: boolean + /** True when the SelfSubjectRulesReview request itself failed. */ + unavailable?: boolean + resourceRules: Array<{ + verbs?: string[] + apiGroups?: string[] + resources?: string[] + resourceNames?: string[] + }> +} + +export type KindGetAccess = + | { type: 'deny-all' } + | { type: 'allow-all' } + | { type: 'allow-names'; names: Set } + | { type: 'incomplete' } + +export type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } + +/** SSRR requires a namespace; ClusterRoleBindings are included in every namespace review. */ +const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' + +const subjectRulesCache = getSubjectRulesCacheStore() +const kindGetAccessCache = getKindGetAccessCacheStore() + +export function canListClusterScopedKind(resource: AccessResource, token: string): Promise { + return canAccess({ kind: resource.kind, apiVersion: resource.apiVersion }, 'list', token) +} + +export function canListNamespacedScopedKind(resource: AccessResource, token: string): Promise { + if (!resource.metadata?.namespace) return Promise.resolve(false) + return canAccess( + { + kind: resource.kind, + apiVersion: resource.apiVersion, + metadata: { namespace: resource.metadata.namespace }, + }, + 'list', + token + ) +} + +function apiGroupFromVersion(apiVersion: string): string { + return apiVersion.includes('/') ? apiVersion.split('/')[0] : '' +} + +function resourcePluralName(kind: string): string { + return pluralize(kind.toLowerCase()) +} + +function isNamespacedResource(resource: AccessResource): boolean { + return Boolean(resource.metadata?.namespace) +} + +function rulesNamespaceFor(resource: AccessResource): string { + return resource.metadata?.namespace || CLUSTER_SCOPED_RULES_NAMESPACE +} + +/** + * Used by SSE eventFilter after cluster-scoped list is denied. + * Namespaced resources are reviewed in the resource's namespace (cached per token+namespace). + * Cluster-scoped resources use a probe-namespace review only as a negative/named-binding cache; + * unrestricted grants from that probe are confirmed with SSAR so RoleBindings in `default` + * cannot impersonate cluster-scoped access. + */ +export function canGetResource(resource: AccessResource, token: string): Promise { + return resolveKindGetAccess(resource, token).then((access) => { + // Probe-namespace SSRR cannot distinguish RoleBindings from ClusterRoleBindings. + // Confirm unrestricted cluster-scoped grants with SSAR to close the default-ns proxy hole. + if (!isNamespacedResource(resource) && access.type === 'allow-all') { + return canAccess(resource, 'get', token) + } + return applyKindGetAccess(access, resource, token, () => + canListNamespacedScopedKind(resource, token).then((nsAllowed) => { + if (nsAllowed) return true + return canAccess(resource, 'get', token) + }) + ) + }) +} + +function applyKindGetAccess( + access: KindGetAccess, + resource: AccessResource, + token: string, + onIncomplete?: () => Promise +): Promise { + switch (access.type) { + case 'deny-all': + return Promise.resolve(false) + case 'allow-all': + return Promise.resolve(true) + case 'allow-names': + return Promise.resolve(resource.metadata?.name ? access.names.has(resource.metadata.name) : false) + case 'incomplete': + return onIncomplete ? onIncomplete() : canAccess(resource, 'get', token) + } +} + +/** + * One SelfSubjectRulesReview per token+namespace. + * ClusterRoleBindings appear in every namespace; RoleBindings appear only in their namespace. + */ +function getSubjectRules(token: string, namespace: string): Promise { + const cacheKey = `${hashAccessToken(token)}:${namespace}` + const existing = getTimedCacheEntry(cacheKey, subjectRulesCache) + if (existing) { + return existing.promise + } + + const promise = jsonPost<{ + status?: { + incomplete?: boolean + resourceRules?: SubjectRulesStatus['resourceRules'] + } + }>( + process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', + { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectRulesReview', + metadata: {}, + spec: { namespace }, + }, + token + ) + .then((result) => { + // jsonPost resolves on HTTP errors; treat non-2xx as review unavailable (SSAR fallback). + if (result.statusCode < 200 || result.statusCode >= 300) { + throw new Error(`SelfSubjectRulesReview failed with status ${result.statusCode}`) + } + return { + incomplete: result.body?.status?.incomplete ?? false, + resourceRules: result.body?.status?.resourceRules ?? [], + } + }) + .catch((err: unknown) => { + logger.warn({ msg: 'SelfSubjectRulesReview failed; falling back to per-object SSAR', error: err }) + // Do not retain a failed review under ACCESS_CACHE_TTL; next call should retry SSRR. + deleteTimedCacheEntry(cacheKey, subjectRulesCache) + return { + incomplete: true, + unavailable: true, + resourceRules: [] as SubjectRulesStatus['resourceRules'], + } + }) + + setTimedCacheEntry(cacheKey, subjectRulesCache, { time: Date.now(), promise }) + return promise +} + +function ruleGrantsKindAccess( + rule: SubjectRulesStatus['resourceRules'][number], + group: string, + resourcePlural: string, + accessVerbs: Set +): { allowAll: true } | { names: string[] } | null { + const verbs = rule.verbs ?? [] + if (!verbs.includes('*') && !verbs.some((verb) => accessVerbs.has(verb))) return null + + const groups = rule.apiGroups ?? [] + if (!groups.includes('*') && !groups.includes(group)) return null + + const resources = rule.resources ?? [] + if (!resources.includes('*') && !resources.includes(resourcePlural)) return null + + const resourceNames = rule.resourceNames + if (!resourceNames || resourceNames.length === 0 || resourceNames.includes('*')) { + return { allowAll: true } + } + return { names: resourceNames } +} + +function evaluateKindGetAccess(rules: SubjectRulesStatus, group: string, resourcePlural: string): KindGetAccess { + const accessVerbs = new Set(['get', 'list', 'watch']) + + let allowAll = false + const names = new Set() + + for (const rule of rules.resourceRules) { + const match = ruleGrantsKindAccess(rule, group, resourcePlural, accessVerbs) + if (!match) continue + if ('allowAll' in match) { + allowAll = true + break + } + for (const name of match.names) names.add(name) + } + + if (allowAll) return { type: 'allow-all' } + if (names.size > 0) return { type: 'allow-names', names } + // The review request failed; defer to the per-object SSAR fallback. + if (rules.unavailable === true) return { type: 'incomplete' } + // OpenShift often sets incomplete=true even when the user has no bindings and resourceRules + // is empty. Treat empty rules as deny-all so we do not fall back to O(N) namespaced SSARs. + if (rules.resourceRules.length === 0) return { type: 'deny-all' } + // Non-empty but incomplete: authorizer may have omitted grants for this kind — fall back. + if (rules.incomplete) return { type: 'incomplete' } + return { type: 'deny-all' } +} + +function resolveKindGetAccess(resource: AccessResource, token: string): Promise { + const group = apiGroupFromVersion(resource.apiVersion) + const plural = resourcePluralName(resource.kind) + const namespace = rulesNamespaceFor(resource) + // Permission checks are by API group, not version; keep cache keys version-free. + const cacheKey = `${hashAccessToken(token)}:${namespace}:${group}:${plural}` + const existing = getTimedCacheEntry(cacheKey, kindGetAccessCache) + if (existing) { + return existing.promise + } + + const promise = getSubjectRules(token, namespace).then((rules) => evaluateKindGetAccess(rules, group, plural)) + setTimedCacheEntry(cacheKey, kindGetAccessCache, { time: Date.now(), promise }) + return promise +} + +export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'create', token: string): Promise { + // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth + const tokenKey = hashAccessToken(token) + const key = `${verb}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` + const existing = getSsarCacheEntry(tokenKey, key) + if (existing) { + return existing.promise + } + + const resourceName = resourcePluralName(resource.kind) + const promise = jsonPost<{ status: { allowed: boolean } }>( + process.env.CLUSTER_API_URL + '/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', + { + apiVersion: 'authorization.k8s.io/v1', + kind: 'SelfSubjectAccessReview', + metadata: {}, + spec: { + resourceAttributes: { + group: apiGroupFromVersion(resource.apiVersion), + name: resource.metadata?.name, + namespace: + resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), + resource: resourceName, + verb, + }, + }, + }, + token + ).then((result) => { + const allowed = result.body.status.allowed + if (process.env.LOG_ACCESS === 'true') { + logger.debug({ + msg: 'access', + allowed, + verb, + resource: resourceName, + name: resource.metadata?.name, + namespace: resource.metadata?.namespace, + }) + } + // Replace in-flight promise with a settled boolean promise to drop large closures. + replaceSsarCachePromise(tokenKey, key, promise, allowed) + return allowed + }) + + setSsarCacheEntry(tokenKey, key, { + time: Date.now(), + promise, + }) + return promise +} diff --git a/backend/src/routes/eventsCache.ts b/backend/src/routes/eventsCache.ts new file mode 100644 index 00000000000..5c01ad4943f --- /dev/null +++ b/backend/src/routes/eventsCache.ts @@ -0,0 +1,187 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { createHash } from 'node:crypto' +import { logger } from '../lib/logger' + +export const ACCESS_CACHE_TTL = 60 * 1000 // 60 seconds +export const ACCESS_CACHE_CLEANUP_INTERVAL = 90 * 1000 // 90 seconds +export const ACCESS_CACHE_MAX_TOKENS = 1000 // Maximum number of token entries to keep +export const ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN = 2000 // Cap RBAC keys retained per token + +export interface TimedCacheEntry { + time: number + promise: Promise +} + +const accessCache: Record>> = {} +const subjectRulesCache: Record> = {} +const kindGetAccessCache: Record> = {} + +let accessCacheCleanupTimer: NodeJS.Timeout | undefined + +/** Hash bearer tokens so the access cache does not retain full JWTs as object keys. */ +export function hashAccessToken(token: string): string { + return createHash('sha256').update(token).digest('hex') +} + +/** Clear all cached RBAC access checks. Used for test isolation. */ +export function resetAccessCache() { + for (const key in accessCache) { + delete accessCache[key] + } + for (const key in subjectRulesCache) { + delete subjectRulesCache[key] + } + for (const key in kindGetAccessCache) { + delete kindGetAccessCache[key] + } +} + +export function getAccessCache() { + return accessCache +} + +function enforceAccessCacheEntryCap(tokenCache: Record>) { + const keys = Object.keys(tokenCache) + if (keys.length <= ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) return + keys.sort((a, b) => tokenCache[a].time - tokenCache[b].time) + const toRemove = keys.length - ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + for (let i = 0; i < toRemove; i++) { + delete tokenCache[keys[i]] + } +} + +function expireTimedEntries(cache: Record, cutoffTime: number) { + for (const key in cache) { + if (cache[key].time < cutoffTime) { + delete cache[key] + } + } +} + +/** Prune one token's SSAR entries; returns newest remaining time, or undefined if the token was removed. */ +function pruneAccessCacheToken( + token: string, + tokenCache: Record>, + cutoffTime: number +): number | undefined { + let newestTime = 0 + + for (const key in tokenCache) { + if (tokenCache[key].time < cutoffTime) { + delete tokenCache[key] + } else if (tokenCache[key].time > newestTime) { + newestTime = tokenCache[key].time + } + } + + if (Object.keys(tokenCache).length === 0) { + delete accessCache[token] + return undefined + } + + enforceAccessCacheEntryCap(tokenCache) + return newestTime +} + +export function cleanupAccessCache() { + const cutoffTime = Date.now() - ACCESS_CACHE_TTL + const tokenStats: Array<{ token: string; newestTime: number }> = [] + + for (const token in accessCache) { + const newestTime = pruneAccessCacheToken(token, accessCache[token], cutoffTime) + if (newestTime !== undefined) { + tokenStats.push({ token, newestTime }) + } + } + + expireTimedEntries(subjectRulesCache, cutoffTime) + expireTimedEntries(kindGetAccessCache, cutoffTime) + + if (tokenStats.length <= ACCESS_CACHE_MAX_TOKENS) return + + tokenStats.sort((a, b) => a.newestTime - b.newestTime) + const tokensToRemove = tokenStats.length - ACCESS_CACHE_MAX_TOKENS + for (let i = 0; i < tokensToRemove; i++) { + delete accessCache[tokenStats[i].token] + } +} + +export function startAccessCacheCleanup() { + if (accessCacheCleanupTimer) return + + accessCacheCleanupTimer = setInterval(() => { + try { + cleanupAccessCache() + } catch (err: unknown) { + logger.error({ msg: 'accessCache cleanup failed', error: err }) + } + }, ACCESS_CACHE_CLEANUP_INTERVAL) + + accessCacheCleanupTimer.unref() + logger.info({ msg: 'accessCache cleanup started', interval: ACCESS_CACHE_CLEANUP_INTERVAL }) +} + +export function stopAccessCacheCleanup() { + if (accessCacheCleanupTimer) { + clearInterval(accessCacheCleanupTimer) + accessCacheCleanupTimer = undefined + logger.info({ msg: 'accessCache cleanup stopped' }) + } +} + +export function getSsarCacheEntry(tokenKey: string, key: string): TimedCacheEntry | undefined { + const existing = accessCache[tokenKey]?.[key] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing + } + return undefined +} + +export function setSsarCacheEntry(tokenKey: string, key: string, entry: TimedCacheEntry) { + if (!accessCache[tokenKey]) accessCache[tokenKey] = {} + accessCache[tokenKey][key] = entry + enforceAccessCacheEntryCap(accessCache[tokenKey]) +} + +export function replaceSsarCachePromise( + tokenKey: string, + key: string, + inFlightPromise: Promise, + allowed: boolean +) { + const entry = accessCache[tokenKey]?.[key] + if (entry?.promise === inFlightPromise) { + entry.promise = Promise.resolve(allowed) + } +} + +export function getTimedCacheEntry( + cacheKey: string, + cache: Record> +): TimedCacheEntry | undefined { + const existing = cache[cacheKey] + if (existing && existing.time > Date.now() - ACCESS_CACHE_TTL) { + return existing as TimedCacheEntry + } + return undefined +} + +export function setTimedCacheEntry( + cacheKey: string, + cache: Record>, + entry: TimedCacheEntry +) { + cache[cacheKey] = entry as TimedCacheEntry +} + +export function deleteTimedCacheEntry(cacheKey: string, cache: Record>) { + delete cache[cacheKey] +} + +export function getSubjectRulesCacheStore() { + return subjectRulesCache +} + +export function getKindGetAccessCacheStore() { + return kindGetAccessCache +} diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/eventsAccess.test.ts similarity index 60% rename from backend/test/routes/events.test.ts rename to backend/test/routes/eventsAccess.test.ts index e8702ab1b6a..62bb6b99632 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -3,139 +3,114 @@ import nock from 'nock' import { canAccess, canGetResource, - resetAccessCache, - getAccessCache, - cleanupAccessCache, - hashAccessToken, - ACCESS_CACHE_TTL, - ACCESS_CACHE_MAX_TOKENS, - ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, -} from '../../src/routes/events' - -describe('events Route RBAC (ACM-39327)', () => { - describe('Access Cache Cleanup', () => { - beforeEach(() => { - resetAccessCache() - process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' - }) + canListClusterScopedKind, + canListNamespacedScopedKind, +} from '../../src/routes/eventsAccess' +import { resetAccessCache } from '../../src/routes/eventsCache' + +describe('eventsAccess', () => { + const apiUrl = () => process.env.CLUSTER_API_URL || '' + const managedCluster = (name: string) => ({ + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name }, + }) + const secret = (namespace: string, name: string) => ({ + kind: 'Secret', + apiVersion: 'v1', + metadata: { namespace, name }, + }) + const managedClusterInfo = (cluster: string) => ({ + kind: 'ManagedClusterInfo', + apiVersion: 'internal.open-cluster-management.io/v1beta1', + metadata: { name: cluster, namespace: cluster }, + }) - afterEach(() => { - resetAccessCache() - delete process.env.CLUSTER_API_URL - nock.cleanAll() - }) + const emptyRules = { incomplete: false, resourceRules: [] as unknown[] } + const secretGetInNamespace = { + incomplete: false, + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['secrets'] }], + } + const clusterAdminRules = { + incomplete: false, + resourceRules: [{ verbs: ['*'], apiGroups: ['*'], resources: ['*'] }], + } + + function rulesReviewNamespace(body: unknown): string { + let parsed = body + if (typeof body === 'string') { + try { + parsed = JSON.parse(body) as unknown + } catch { + return '' + } + } + return (parsed as { spec?: { namespace?: string } })?.spec?.namespace || '' + } + + function nockRulesReview(replyFn: (namespace: string) => { incomplete?: boolean; resourceRules?: unknown[] }) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) + } + + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) - it('should cache RBAC access check results under hashed token keys', async () => { - const mockToken = 'test-token-123' - const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + afterEach(() => { + resetAccessCache() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + describe('canListClusterScopedKind', () => { + it('should issue a cluster-scoped list SSAR', async () => { + const resource = managedCluster('cluster-1') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { verb?: string } } }) + : body + return ( + (parsed as { spec?: { resourceAttributes?: { verb?: string } } })?.spec?.resourceAttributes?.verb === 'list' + ) + }) .reply(200, { status: { allowed: true } }) - const result1 = await canAccess(resource, 'get', mockToken) - const result2 = await canAccess(resource, 'get', mockToken) - - expect(result1).toBe(true) - expect(result1).toBe(result2) - expect(getAccessCache()[mockToken]).toBeUndefined() - expect(getAccessCache()[hashAccessToken(mockToken)]['get:Pod:default:test-pod']).toBeDefined() + expect(await canListClusterScopedKind(resource, 'list-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) }) + }) - it('should use distinct cache keys per verb', async () => { - const mockToken = 'test-token-verb' - const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } - - nock(process.env.CLUSTER_API_URL || '') + describe('canListNamespacedScopedKind', () => { + it('should return false when the resource has no namespace', async () => { + const ssarScope = nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') .reply(200, { status: { allowed: true } }) - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') - .reply(200, { status: { allowed: false } }) - - expect(await canAccess(resource, 'get', mockToken)).toBe(true) - expect(await canAccess(resource, 'list', mockToken)).toBe(false) - - const tokenCache = getAccessCache()[hashAccessToken(mockToken)] - expect(tokenCache['get:Pod:default:test-pod']).toBeDefined() - expect(tokenCache['list:Pod:default:test-pod']).toBeDefined() - }) - - it('should respect TTL and refetch after expiry', async () => { - const cache = getAccessCache() - const mockToken = 'test-token-ttl' - const tokenKey = hashAccessToken(mockToken) - - cache[tokenKey] = { - 'get:Secret:default:credentials': { - time: Date.now() - ACCESS_CACHE_TTL - 1000, - promise: Promise.resolve(true), - }, - } - - nock(process.env.CLUSTER_API_URL || '') - .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') - .reply(200, { status: { allowed: false } }) - - const result = await canAccess( - { kind: 'Secret', apiVersion: 'v1', metadata: { namespace: 'default', name: 'credentials' } }, - 'get', - mockToken - ) - expect(result).toBe(false) - }) - - it('should remove stale cache entries during cleanup', () => { - const cache = getAccessCache() - const now = Date.now() - - cache['token1'] = { - stale: { time: now - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, - fresh: { time: now - 30000, promise: Promise.resolve(true) }, - } - cache['token2'] = { 'stale-only': { time: now - ACCESS_CACHE_TTL - 5000, promise: Promise.resolve(false) } } - - cleanupAccessCache() - - expect(cache['token1']['stale']).toBeUndefined() - expect(cache['token1']['fresh']).toBeDefined() - expect(cache['token2']).toBeUndefined() - }) - - it('should enforce maximum token limit with LRU eviction', () => { - const cache = getAccessCache() - const now = Date.now() - const tokenCount = ACCESS_CACHE_MAX_TOKENS + 100 - - for (let i = 0; i < tokenCount; i++) { - cache[`token-${i}`] = { - 'Pod:default:test': { time: now - (i / tokenCount) * 50 * 1000, promise: Promise.resolve(true) }, - } - } - - cleanupAccessCache() - expect(Object.keys(cache).length).toBe(ACCESS_CACHE_MAX_TOKENS) - expect(cache['token-0']).toBeDefined() - expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() + expect(await canListNamespacedScopedKind(managedCluster('cluster-1'), 'list-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) }) - it('should enforce maximum entries per token', () => { - const cache = getAccessCache() - const tokenKey = hashAccessToken('test-token-entry-cap') - const now = Date.now() - cache[tokenKey] = {} - - for (let i = 0; i < ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + 50; i++) { - cache[tokenKey][`get:Pod:default:pod-${i}`] = { - time: now - i, - promise: Promise.resolve(false), - } - } - - cleanupAccessCache() + it('should issue a namespaced list SSAR', async () => { + const resource = managedClusterInfo('acm39327-mc-01') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { namespace?: string; verb?: string } } }) + : body + const attrs = (parsed as { spec?: { resourceAttributes?: { namespace?: string; verb?: string } } })?.spec + ?.resourceAttributes + return attrs?.namespace === 'acm39327-mc-01' && attrs?.verb === 'list' + }) + .reply(200, { status: { allowed: true } }) - expect(Object.keys(cache[tokenKey]).length).toBeLessThanOrEqual(ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) + expect(await canListNamespacedScopedKind(resource, 'list-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) }) }) @@ -145,62 +120,6 @@ describe('events Route RBAC (ACM-39327)', () => { * namespaced: cache one review per token+namespace, never treat `default` as global allow. */ describe('SelfSubjectRulesReview short-circuit (ACM-39327)', () => { - const apiUrl = () => process.env.CLUSTER_API_URL || '' - const managedCluster = (name: string) => ({ - kind: 'ManagedCluster', - apiVersion: 'cluster.open-cluster-management.io/v1', - metadata: { name }, - }) - const secret = (namespace: string, name: string) => ({ - kind: 'Secret', - apiVersion: 'v1', - metadata: { namespace, name }, - }) - const managedClusterInfo = (cluster: string) => ({ - kind: 'ManagedClusterInfo', - apiVersion: 'internal.open-cluster-management.io/v1beta1', - metadata: { name: cluster, namespace: cluster }, - }) - - const emptyRules = { incomplete: false, resourceRules: [] as unknown[] } - const secretGetInNamespace = { - incomplete: false, - resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['secrets'] }], - } - const clusterAdminRules = { - incomplete: false, - resourceRules: [{ verbs: ['*'], apiGroups: ['*'], resources: ['*'] }], - } - - function rulesReviewNamespace(body: unknown): string { - let parsed = body - if (typeof body === 'string') { - try { - parsed = JSON.parse(body) as unknown - } catch { - return '' - } - } - return (parsed as { spec?: { namespace?: string } })?.spec?.namespace || '' - } - - function nockRulesReview(replyFn: (namespace: string) => { incomplete?: boolean; resourceRules?: unknown[] }) { - return nock(apiUrl()) - .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') - .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) - } - - beforeEach(() => { - resetAccessCache() - process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' - }) - - afterEach(() => { - resetAccessCache() - delete process.env.CLUSTER_API_URL - nock.cleanAll() - }) - it('should deny all gets from complete empty rules without per-object SSAR', async () => { nockRulesReview(() => emptyRules) @@ -426,4 +345,24 @@ describe('events Route RBAC (ACM-39327)', () => { expect(ssarScope.isDone()).toBe(true) }) }) + + describe('canAccess', () => { + it('should post a SelfSubjectAccessReview for the requested verb and resource', async () => { + const resource = secret('default', 'test-secret') + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: { verb?: string; resource?: string } } }) + : body + const attrs = (parsed as { spec?: { resourceAttributes?: { verb?: string; resource?: string } } })?.spec + ?.resourceAttributes + return attrs?.verb === 'create' && attrs?.resource === 'secrets' + }) + .reply(200, { status: { allowed: true } }) + + expect(await canAccess(resource, 'create', 'create-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + }) }) diff --git a/backend/test/routes/eventsCache.test.ts b/backend/test/routes/eventsCache.test.ts new file mode 100644 index 00000000000..aa1820145dc --- /dev/null +++ b/backend/test/routes/eventsCache.test.ts @@ -0,0 +1,250 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { createHash } from 'node:crypto' +import nock from 'nock' +import { canAccess } from '../../src/routes/eventsAccess' +import { + ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN, + ACCESS_CACHE_MAX_TOKENS, + ACCESS_CACHE_TTL, + cleanupAccessCache, + deleteTimedCacheEntry, + getAccessCache, + getKindGetAccessCacheStore, + getSsarCacheEntry, + getSubjectRulesCacheStore, + getTimedCacheEntry, + hashAccessToken, + replaceSsarCachePromise, + resetAccessCache, + setSsarCacheEntry, + setTimedCacheEntry, + startAccessCacheCleanup, + stopAccessCacheCleanup, +} from '../../src/routes/eventsCache' + +describe('eventsCache', () => { + beforeEach(() => { + resetAccessCache() + process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' + }) + + afterEach(() => { + resetAccessCache() + stopAccessCacheCleanup() + delete process.env.CLUSTER_API_URL + nock.cleanAll() + }) + + describe('hashAccessToken', () => { + it('should return a stable SHA-256 hex digest', () => { + const token = 'test-token-123' + const expected = createHash('sha256').update(token).digest('hex') + expect(hashAccessToken(token)).toBe(expected) + expect(hashAccessToken(token)).toBe(hashAccessToken(token)) + }) + }) + + describe('SSAR cache helpers', () => { + it('should return undefined for expired SSAR cache entries', () => { + const tokenKey = hashAccessToken('expired-token') + setSsarCacheEntry(tokenKey, 'get:Pod:default:pod', { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve(true), + }) + expect(getSsarCacheEntry(tokenKey, 'get:Pod:default:pod')).toBeUndefined() + }) + + it('should replace in-flight SSAR promises with settled booleans', async () => { + const tokenKey = hashAccessToken('replace-token') + const key = 'get:Pod:default:pod' + const inFlight = Promise.resolve(true) + setSsarCacheEntry(tokenKey, key, { time: Date.now(), promise: inFlight }) + + replaceSsarCachePromise(tokenKey, key, inFlight, true) + const cached = getSsarCacheEntry(tokenKey, key) + if (!cached) throw new Error('expected SSAR cache entry') + expect(await cached.promise).toBe(true) + }) + + it('should enforce per-token entry cap when setting SSAR cache entries', () => { + const tokenKey = hashAccessToken('cap-token') + const now = Date.now() + for (let i = 0; i < ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN + 50; i++) { + setSsarCacheEntry(tokenKey, `get:Pod:default:pod-${i}`, { + time: now - i, + promise: Promise.resolve(false), + }) + } + cleanupAccessCache() + expect(Object.keys(getAccessCache()[tokenKey]).length).toBeLessThanOrEqual(ACCESS_CACHE_MAX_ENTRIES_PER_TOKEN) + }) + }) + + describe('timed cache helpers', () => { + it('should get, set, and delete timed cache entries with TTL', async () => { + const store = getSubjectRulesCacheStore() + const cacheKey = 'token:default' + + setTimedCacheEntry(cacheKey, store, { time: Date.now(), promise: Promise.resolve('rules') }) + const cached = getTimedCacheEntry(cacheKey, store) + if (!cached) throw new Error('expected timed cache entry') + expect(await cached.promise).toBe('rules') + + setTimedCacheEntry(cacheKey, store, { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve('stale'), + }) + expect(getTimedCacheEntry(cacheKey, store)).toBeUndefined() + + setTimedCacheEntry(cacheKey, store, { time: Date.now(), promise: Promise.resolve('fresh') }) + deleteTimedCacheEntry(cacheKey, store) + expect(getTimedCacheEntry(cacheKey, store)).toBeUndefined() + }) + }) + + describe('resetAccessCache', () => { + it('should clear SSAR, subject rules, and kind-get-access stores', () => { + const tokenKey = hashAccessToken('reset-token') + setSsarCacheEntry(tokenKey, 'get:Pod:default:pod', { time: Date.now(), promise: Promise.resolve(true) }) + setTimedCacheEntry('rules-key', getSubjectRulesCacheStore(), { + time: Date.now(), + promise: Promise.resolve({ incomplete: false, resourceRules: [] }), + }) + setTimedCacheEntry('kind-key', getKindGetAccessCacheStore(), { + time: Date.now(), + promise: Promise.resolve({ type: 'deny-all' }), + }) + + resetAccessCache() + + expect(getAccessCache()[tokenKey]).toBeUndefined() + expect(Object.keys(getSubjectRulesCacheStore()).length).toBe(0) + expect(Object.keys(getKindGetAccessCacheStore()).length).toBe(0) + }) + }) + + describe('cleanupAccessCache', () => { + it('should remove stale SSAR entries during cleanup', () => { + const cache = getAccessCache() + const now = Date.now() + + cache['token1'] = { + stale: { time: now - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true) }, + fresh: { time: now - 30000, promise: Promise.resolve(true) }, + } + cache['token2'] = { 'stale-only': { time: now - ACCESS_CACHE_TTL - 5000, promise: Promise.resolve(false) } } + + cleanupAccessCache() + + expect(cache['token1']['stale']).toBeUndefined() + expect(cache['token1']['fresh']).toBeDefined() + expect(cache['token2']).toBeUndefined() + }) + + it('should enforce maximum token limit with LRU eviction', () => { + const cache = getAccessCache() + const now = Date.now() + const tokenCount = ACCESS_CACHE_MAX_TOKENS + 100 + + for (let i = 0; i < tokenCount; i++) { + cache[`token-${i}`] = { + 'Pod:default:test': { time: now - (i / tokenCount) * 50 * 1000, promise: Promise.resolve(true) }, + } + } + + cleanupAccessCache() + + expect(Object.keys(cache).length).toBe(ACCESS_CACHE_MAX_TOKENS) + expect(cache['token-0']).toBeDefined() + expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() + }) + + it('should expire timed caches during cleanup', () => { + const subjectRules = getSubjectRulesCacheStore() + const kindGetAccess = getKindGetAccessCacheStore() + const staleTime = Date.now() - ACCESS_CACHE_TTL - 1000 + + setTimedCacheEntry('stale-rules', subjectRules, { time: staleTime, promise: Promise.resolve('old') }) + setTimedCacheEntry('stale-kind', kindGetAccess, { + time: staleTime, + promise: Promise.resolve({ type: 'deny-all' }), + }) + + cleanupAccessCache() + + expect(getTimedCacheEntry('stale-rules', subjectRules)).toBeUndefined() + expect(getTimedCacheEntry('stale-kind', kindGetAccess)).toBeUndefined() + }) + }) + + describe('startAccessCacheCleanup', () => { + it('should start and stop periodic cleanup without error', () => { + startAccessCacheCleanup() + startAccessCacheCleanup() + stopAccessCacheCleanup() + stopAccessCacheCleanup() + }) + }) + + describe('SSAR caching via canAccess', () => { + it('should cache RBAC access check results under hashed token keys', async () => { + const mockToken = 'test-token-123' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + const result1 = await canAccess(resource, 'get', mockToken) + const result2 = await canAccess(resource, 'get', mockToken) + + expect(result1).toBe(true) + expect(result1).toBe(result2) + expect(getAccessCache()[mockToken]).toBeUndefined() + expect(getAccessCache()[hashAccessToken(mockToken)]['get:Pod:default:test-pod']).toBeDefined() + }) + + it('should use distinct cache keys per verb', async () => { + const mockToken = 'test-token-verb' + const resource = { kind: 'Pod', apiVersion: 'v1', metadata: { namespace: 'default', name: 'test-pod' } } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canAccess(resource, 'get', mockToken)).toBe(true) + expect(await canAccess(resource, 'list', mockToken)).toBe(false) + + const tokenCache = getAccessCache()[hashAccessToken(mockToken)] + expect(tokenCache['get:Pod:default:test-pod']).toBeDefined() + expect(tokenCache['list:Pod:default:test-pod']).toBeDefined() + }) + + it('should respect TTL and refetch after expiry', async () => { + const cache = getAccessCache() + const mockToken = 'test-token-ttl' + const tokenKey = hashAccessToken(mockToken) + + cache[tokenKey] = { + 'get:Secret:default:credentials': { + time: Date.now() - ACCESS_CACHE_TTL - 1000, + promise: Promise.resolve(true), + }, + } + + nock(process.env.CLUSTER_API_URL || '') + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + const result = await canAccess( + { kind: 'Secret', apiVersion: 'v1', metadata: { namespace: 'default', name: 'credentials' } }, + 'get', + mockToken + ) + expect(result).toBe(false) + }) + }) +}) From fe811a867585c6aacba5f308c246ef8edae29d59 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 19 Aug 2026 07:05:26 +0200 Subject: [PATCH 06/10] ACM-39327: harden cluster-scoped access checks with SSAR confirmation - Maintain an explicit allowlist of cluster-scoped kinds watched by the console and route their SSRR probes through the default namespace. - Require SSAR confirmation for any non-deny cluster-scoped SSRR result, closing the gap where RoleBindings in default could impersonate cluster-scoped grants. - Surface and handle SSRR evaluationError as incomplete rule enumeration, falling back to SSAR when rule lists may be partial. - Expand unit tests for cluster-scoped allow-names, deny paths, and evaluationError behavior. Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/eventsAccess.ts | 52 +++++- backend/test/routes/eventsAccess.test.ts | 203 +++++++++++++++++++++-- 2 files changed, 236 insertions(+), 19 deletions(-) diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts index c02321085ac..7ea0dd3c5c3 100644 --- a/backend/src/routes/eventsAccess.ts +++ b/backend/src/routes/eventsAccess.ts @@ -18,6 +18,8 @@ export interface SubjectRulesStatus { incomplete: boolean /** True when the SelfSubjectRulesReview request itself failed. */ unavailable?: boolean + /** Set when an authorizer could not fully enumerate rules; partial lists must not be trusted as complete. */ + evaluationError?: string resourceRules: Array<{ verbs?: string[] apiGroups?: string[] @@ -34,9 +36,38 @@ export type KindGetAccess = export type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } -/** SSRR requires a namespace; ClusterRoleBindings are included in every namespace review. */ +/** SSRR requires a namespace; cluster-scoped kinds are reviewed in this probe namespace only. */ const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' +/** + * Kinds watched by the console that are cluster-scoped in Kubernetes (not inferred from metadata.namespace). + * Keep aligned with cluster-scoped entries in backend/src/routes/events.ts definitions. + */ +const CLUSTER_SCOPED_KINDS = new Set([ + 'AgentServiceConfig', + 'CertificateSigningRequest', + 'ClusterCurator', + 'ClusterImageSet', + 'ClusterManagementAddOn', + 'ClusterVersion', + 'DiscoveredCluster', + 'DiscoveryConfig', + 'Infrastructure', + 'ManagedCluster', + 'ManagedClusterSet', + 'ManagedClusterSetBinding', + 'MultiClusterEngine', + 'Namespace', + 'Placement', + 'PlacementDecision', + 'Search', + 'StorageClass', +]) + +function isClusterScopedKind(kind: string): boolean { + return CLUSTER_SCOPED_KINDS.has(kind) +} + const subjectRulesCache = getSubjectRulesCacheStore() const kindGetAccessCache = getKindGetAccessCacheStore() @@ -65,11 +96,10 @@ function resourcePluralName(kind: string): string { return pluralize(kind.toLowerCase()) } -function isNamespacedResource(resource: AccessResource): boolean { - return Boolean(resource.metadata?.namespace) -} - function rulesNamespaceFor(resource: AccessResource): string { + if (isClusterScopedKind(resource.kind)) { + return CLUSTER_SCOPED_RULES_NAMESPACE + } return resource.metadata?.namespace || CLUSTER_SCOPED_RULES_NAMESPACE } @@ -83,8 +113,8 @@ function rulesNamespaceFor(resource: AccessResource): string { export function canGetResource(resource: AccessResource, token: string): Promise { return resolveKindGetAccess(resource, token).then((access) => { // Probe-namespace SSRR cannot distinguish RoleBindings from ClusterRoleBindings. - // Confirm unrestricted cluster-scoped grants with SSAR to close the default-ns proxy hole. - if (!isNamespacedResource(resource) && access.type === 'allow-all') { + // Any non-deny cluster-scoped result must be confirmed with SSAR (not only allow-all/allow-names). + if (isClusterScopedKind(resource.kind) && access.type !== 'deny-all') { return canAccess(resource, 'get', token) } return applyKindGetAccess(access, resource, token, () => @@ -128,6 +158,7 @@ function getSubjectRules(token: string, namespace: string): Promise( @@ -147,6 +178,7 @@ function getSubjectRules(token: string, namespace: string): Promise() diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts index 62bb6b99632..79f74f68881 100644 --- a/backend/test/routes/eventsAccess.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -54,6 +54,51 @@ describe('eventsAccess', () => { .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) } + function nockRulesReviewStatus( + replyFn: (namespace: string) => { + incomplete?: boolean + evaluationError?: string + resourceRules?: unknown[] + } + ) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(200, (_uri: string, requestBody: unknown) => ({ status: replyFn(rulesReviewNamespace(requestBody)) })) + } + + function parseSsarResourceAttributes(body: unknown) { + const parsed = + typeof body === 'string' + ? (JSON.parse(body) as { spec?: { resourceAttributes?: Record } }) + : body + return (parsed as { spec?: { resourceAttributes?: Record } })?.spec + ?.resourceAttributes + } + + function nockSsarGet( + matcher: (attrs: Record) => boolean, + allowed: boolean + ) { + return nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + const attrs = parseSsarResourceAttributes(body) + return attrs?.verb === 'get' && matcher(attrs ?? {}) + }) + .reply(200, { status: { allowed } }) + } + + const namedManagedClusterRule = (name: string) => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: [name], + }, + ], + }) + beforeEach(() => { resetAccessCache() process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' @@ -237,21 +282,32 @@ describe('eventsAccess', () => { expect(await canGetResource(managedCluster('acm39327-mc-01'), 'cluster-admin-token')).toBe(false) }) - it('should allow only named cluster-scoped resources from resourceNames rules', async () => { - nockRulesReview(() => ({ - incomplete: false, - resourceRules: [ - { - verbs: ['get'], - apiGroups: ['cluster.open-cluster-management.io'], - resources: ['managedclusters'], - resourceNames: ['allowed-cluster'], - }, - ], - })) + it('should confirm named cluster-scoped resources with SSAR before allowing access', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'allowed-cluster', + true + ) expect(await canGetResource(managedCluster('allowed-cluster'), 'partial-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should deny non-matching names from cluster-scoped allow-names without trusting SSRR alone', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'other-cluster', + false + ) + expect(await canGetResource(managedCluster('other-cluster'), 'partial-user-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) }) it('should allow namespaced resources when rules grant unrestricted get/list/watch in that namespace', async () => { @@ -346,6 +402,129 @@ describe('eventsAccess', () => { }) }) + /** + * TDD: middle-ground security — SSRR deny-all short-circuit only; any non-deny cluster-scoped + * result must be confirmed with SSAR. Implementation pending in eventsAccess.ts. + */ + describe('cluster-scoped SSRR middle-ground security (TDD)', () => { + it('should deny allow-names from a default RoleBinding when SSAR get is false (Kevin)', async () => { + nockRulesReview(() => namedManagedClusterRule('acm39327-mc-02')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'acm39327-mc-02', + false + ) + + expect(await canGetResource(managedCluster('acm39327-mc-02'), 'user1-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should allow allow-names only when SSAR get confirms a real ClusterRoleBinding grant', async () => { + nockRulesReview(() => namedManagedClusterRule('allowed-cluster')) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'allowed-cluster', + true + ) + + expect(await canGetResource(managedCluster('allowed-cluster'), 'clusterrole-user-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should confirm cluster-scoped allow-all with SSAR and deny when SSAR rejects', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => attrs.group === 'cluster.open-cluster-management.io' && attrs.resource === 'managedclusters', + false + ) + + expect(await canGetResource(managedCluster('any-cluster'), 'default-role-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should not trust allow-names on ManagedCluster when metadata.namespace is set without SSAR confirmation', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: namedManagedClusterRule('acm39327-mc-02') }) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'acm39327-mc-02', + false + ) + + expect( + await canGetResource( + { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name: 'acm39327-mc-02', namespace: 'default' }, + }, + 'user1-token' + ) + ).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should treat SSRR evaluationError as incomplete and confirm cluster-scoped access with SSAR', async () => { + nockRulesReviewStatus(() => ({ + incomplete: false, + evaluationError: 'webhook authorizer does not support user rule resolution', + resourceRules: [{ verbs: ['get'], apiGroups: [''], resources: ['pods'] }], + })) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'cluster-1', + true + ) + + expect(await canGetResource(managedCluster('cluster-1'), 'evaluation-error-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should confirm any non-deny-all cluster-scoped SSRR result with SSAR, not applyKindGetAccess alone', async () => { + nockRulesReview(() => ({ + incomplete: true, + resourceRules: [ + { + verbs: ['get'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['managedclusters'], + resourceNames: ['cluster-1'], + }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => + attrs.group === 'cluster.open-cluster-management.io' && + attrs.resource === 'managedclusters' && + attrs.name === 'cluster-1', + false + ) + + expect(await canGetResource(managedCluster('cluster-1'), 'incomplete-named-token')).toBe(false) + expect(ssarScope.isDone()).toBe(true) + }) + }) + describe('canAccess', () => { it('should post a SelfSubjectAccessReview for the requested verb and resource', async () => { const resource = secret('default', 'test-secret') From 7500c8523ee344b7362d3f4df4cfe845e4289e3c Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 19 Aug 2026 07:15:10 +0200 Subject: [PATCH 07/10] prettier error fix Signed-off-by: Enrique Mingorance Cano --- backend/test/routes/eventsAccess.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts index 79f74f68881..f68a676f7c8 100644 --- a/backend/test/routes/eventsAccess.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -71,14 +71,10 @@ describe('eventsAccess', () => { typeof body === 'string' ? (JSON.parse(body) as { spec?: { resourceAttributes?: Record } }) : body - return (parsed as { spec?: { resourceAttributes?: Record } })?.spec - ?.resourceAttributes + return (parsed as { spec?: { resourceAttributes?: Record } })?.spec?.resourceAttributes } - function nockSsarGet( - matcher: (attrs: Record) => boolean, - allowed: boolean - ) { + function nockSsarGet(matcher: (attrs: Record) => boolean, allowed: boolean) { return nock(apiUrl()) .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { const attrs = parseSsarResourceAttributes(body) From f0afd2f86d42a72aa125bc8e687446dc08d39591 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 13:40:38 +0200 Subject: [PATCH 08/10] ACM-39327: derive cluster-scoped kinds from watch definitions Mark cluster-scoped watches on IWatchOptions, include API group in SSAR cache keys, and retry SelfSubjectRulesReview after unavailable results so namespaced kinds cannot impersonate cluster access on release-2.13. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/routes/events.ts | 39 ++++++--- backend/src/routes/eventsAccess.ts | 53 +++++------- backend/src/routes/eventsCache.ts | 2 +- backend/test/routes/eventsAccess.test.ts | 103 +++++++++++++++++++++++ backend/test/routes/eventsCache.test.ts | 8 +- 5 files changed, 155 insertions(+), 50 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index e5f402620a2..14ee7be3155 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -14,7 +14,13 @@ import { type ServerSideEvent, ServerSideEvents } from '../lib/server-side-event import { getCACertificate, getServiceAccountToken } from '../lib/serviceAccountToken' import { getAuthenticatedToken } from '../lib/token' import type { IResource } from '../resources/resource' -import { canAccess, canGetResource, canListClusterScopedKind, canListNamespacedScopedKind } from './eventsAccess' +import { + canAccess, + canGetResource, + canListClusterScopedKind, + canListNamespacedScopedKind, + configureClusterScopedKinds, +} from './eventsAccess' import { startAccessCacheCleanup, stopAccessCacheCleanup } from './eventsCache' export { @@ -145,10 +151,10 @@ export function initResourceCache(cache: ResourceCache) { export let resourceCache: ResourceCache = {} const definitions: IWatchOptions[] = [ - { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, + { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1', clusterScoped: true }, { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1', clusterScoped: true }, { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, @@ -162,19 +168,20 @@ const definitions: IWatchOptions[] = [ { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1' }, { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, { kind: 'MulticlusterApplicationSetReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, - { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1' }, + { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, { kind: 'CertificateSigningRequest', apiVersion: 'certificates.k8s.io/v1', labelSelector: { 'open-cluster-management.io/cluster-name': '' }, + clusterScoped: true, }, - { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1' }, + { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', clusterScoped: true }, { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, - { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, + { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2', clusterScoped: true }, { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, @@ -182,15 +189,15 @@ const definitions: IWatchOptions[] = [ { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1', clusterScoped: true }, { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, - { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1' }, - { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1' }, - { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1' }, + { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1', clusterScoped: true }, { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, @@ -208,7 +215,7 @@ const definitions: IWatchOptions[] = [ fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, }, { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, - { kind: 'Namespace', apiVersion: 'v1' }, + { kind: 'Namespace', apiVersion: 'v1', clusterScoped: true }, { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, @@ -232,6 +239,10 @@ const definitions: IWatchOptions[] = [ }, ] +configureClusterScopedKinds( + definitions.filter((definition) => definition.clusterScoped).map((definition) => definition.kind) +) + export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() @@ -246,6 +257,12 @@ interface IWatchOptions { kind: string labelSelector?: Record fieldSelector?: Record + /** + * True when the Kubernetes resource is cluster-scoped. + * Used by SSE RBAC to decide whether SelfSubjectRulesReview should probe `default` + * (cluster-scoped) or the resource namespace (namespaced). + */ + clusterScoped?: boolean } // https://kubernetes.io/docs/reference/using-api/api-concepts/ diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts index 7ea0dd3c5c3..e8e35ccccab 100644 --- a/backend/src/routes/eventsAccess.ts +++ b/backend/src/routes/eventsAccess.ts @@ -29,43 +29,22 @@ export interface SubjectRulesStatus { } export type KindGetAccess = - | { type: 'deny-all' } - | { type: 'allow-all' } - | { type: 'allow-names'; names: Set } - | { type: 'incomplete' } + { type: 'deny-all' } | { type: 'allow-all' } | { type: 'allow-names'; names: Set } | { type: 'incomplete' } export type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } /** SSRR requires a namespace; cluster-scoped kinds are reviewed in this probe namespace only. */ const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' -/** - * Kinds watched by the console that are cluster-scoped in Kubernetes (not inferred from metadata.namespace). - * Keep aligned with cluster-scoped entries in backend/src/routes/events.ts definitions. - */ -const CLUSTER_SCOPED_KINDS = new Set([ - 'AgentServiceConfig', - 'CertificateSigningRequest', - 'ClusterCurator', - 'ClusterImageSet', - 'ClusterManagementAddOn', - 'ClusterVersion', - 'DiscoveredCluster', - 'DiscoveryConfig', - 'Infrastructure', - 'ManagedCluster', - 'ManagedClusterSet', - 'ManagedClusterSetBinding', - 'MultiClusterEngine', - 'Namespace', - 'Placement', - 'PlacementDecision', - 'Search', - 'StorageClass', -]) - -function isClusterScopedKind(kind: string): boolean { - return CLUSTER_SCOPED_KINDS.has(kind) +let clusterScopedKinds = new Set() + +/** Replace the cluster-scoped kind lookup. Derived from watch definitions in events.ts. */ +export function configureClusterScopedKinds(kinds: Iterable): void { + clusterScopedKinds = new Set(kinds) +} + +export function isClusterScopedKind(kind: string): boolean { + return clusterScopedKinds.has(kind) } const subjectRulesCache = getSubjectRulesCacheStore() @@ -264,7 +243,11 @@ function resolveKindGetAccess(resource: AccessResource, token: string): Promise< return existing.promise } - const promise = getSubjectRules(token, namespace).then((rules) => evaluateKindGetAccess(rules, group, plural)) + const promise = getSubjectRules(token, namespace).then((rules) => { + // Do not retain a decision derived from an unavailable review; allow SSRR retry. + if (rules.unavailable) deleteTimedCacheEntry(cacheKey, kindGetAccessCache) + return evaluateKindGetAccess(rules, group, plural) + }) setTimedCacheEntry(cacheKey, kindGetAccessCache, { time: Date.now(), promise }) return promise } @@ -272,7 +255,9 @@ function resolveKindGetAccess(resource: AccessResource, token: string): Promise< export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'create', token: string): Promise { // Cache is cleaned up periodically by cleanupAccessCache() to prevent unbounded memory growth const tokenKey = hashAccessToken(token) - const key = `${verb}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` + const group = apiGroupFromVersion(resource.apiVersion) + // Include API group: Application/Subscription exist in more than one group with different RBAC. + const key = `${verb}:${group}:${resource.kind}:${resource.metadata?.namespace}:${resource.metadata?.name}` const existing = getSsarCacheEntry(tokenKey, key) if (existing) { return existing.promise @@ -287,7 +272,7 @@ export function canAccess(resource: AccessResource, verb: 'get' | 'list' | 'crea metadata: {}, spec: { resourceAttributes: { - group: apiGroupFromVersion(resource.apiVersion), + group, name: resource.metadata?.name, namespace: resource.metadata?.namespace ?? (resource.kind === 'Namespace' ? resource.metadata?.name : undefined), diff --git a/backend/src/routes/eventsCache.ts b/backend/src/routes/eventsCache.ts index 5c01ad4943f..d71bbf9eb45 100644 --- a/backend/src/routes/eventsCache.ts +++ b/backend/src/routes/eventsCache.ts @@ -171,7 +171,7 @@ export function setTimedCacheEntry( cache: Record>, entry: TimedCacheEntry ) { - cache[cacheKey] = entry as TimedCacheEntry + cache[cacheKey] = entry } export function deleteTimedCacheEntry(cacheKey: string, cache: Record>) { diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts index f68a676f7c8..c4c6c867e75 100644 --- a/backend/test/routes/eventsAccess.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -5,6 +5,7 @@ import { canGetResource, canListClusterScopedKind, canListNamespacedScopedKind, + configureClusterScopedKinds, } from '../../src/routes/eventsAccess' import { resetAccessCache } from '../../src/routes/eventsCache' @@ -97,6 +98,7 @@ describe('eventsAccess', () => { beforeEach(() => { resetAccessCache() + configureClusterScopedKinds(['ManagedCluster', 'Namespace', 'StorageClass']) process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' }) @@ -539,5 +541,106 @@ describe('eventsAccess', () => { expect(await canAccess(resource, 'create', 'create-token')).toBe(true) expect(ssarScope.isDone()).toBe(true) }) + + it('should not reuse SSAR results across API groups that share a kind name', async () => { + const appK8s = { + kind: 'Application', + apiVersion: 'app.k8s.io/v1beta1', + metadata: { namespace: 'ns', name: 'app' }, + } + const argoApp = { + kind: 'Application', + apiVersion: 'argoproj.io/v1alpha1', + metadata: { namespace: 'ns', name: 'app' }, + } + + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + return parseSsarResourceAttributes(body)?.group === 'app.k8s.io' + }) + .reply(200, { status: { allowed: true } }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews', (body: unknown) => { + return parseSsarResourceAttributes(body)?.group === 'argoproj.io' + }) + .reply(200, { status: { allowed: false } }) + + expect(await canAccess(appK8s, 'get', 'group-collision-token')).toBe(true) + expect(await canAccess(argoApp, 'get', 'group-collision-token')).toBe(false) + }) + }) + + describe('namespaced vs cluster-scoped kind routing (ACM-39327)', () => { + const placement = (namespace: string, name: string) => ({ + kind: 'Placement', + apiVersion: 'cluster.open-cluster-management.io/v1beta1', + metadata: { namespace, name }, + }) + const storageClass = (name: string) => ({ + kind: 'StorageClass', + apiVersion: 'storage.k8s.io/v1', + metadata: { name }, + }) + const placementAllowAll = { + incomplete: false, + resourceRules: [ + { + verbs: ['get', 'list', 'watch'], + apiGroups: ['cluster.open-cluster-management.io'], + resources: ['placements'], + }, + ], + } + + it('must not treat Placement allow-all in default as access to other namespaces', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'default' + }) + .reply(200, { status: placementAllowAll }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews', (body: unknown) => { + return rulesReviewNamespace(body) === 'other-ns' + }) + .reply(200, { status: emptyRules }) + + expect(await canGetResource(placement('default', 'p-default'), 'placement-token')).toBe(true) + expect(await canGetResource(placement('other-ns', 'p-other'), 'placement-token')).toBe(false) + }) + + it('should confirm StorageClass cluster-scoped grants with SSAR', async () => { + nockRulesReview(() => ({ + incomplete: false, + resourceRules: [ + { verbs: ['get', 'list', 'watch'], apiGroups: ['storage.k8s.io'], resources: ['storageclasses'] }, + ], + })) + const ssarScope = nockSsarGet( + (attrs) => attrs.group === 'storage.k8s.io' && attrs.resource === 'storageclasses', + true + ) + + expect(await canGetResource(storageClass('gp3'), 'storage-class-token')).toBe(true) + expect(ssarScope.isDone()).toBe(true) + }) + + it('should retry SelfSubjectRulesReview after an unavailable review', async () => { + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews') + .reply(500, { message: 'internal error' }) + nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: false } }) + + expect(await canGetResource(managedCluster('cluster-1'), 'ssrr-retry-token')).toBe(false) + + nock(apiUrl()).post('/apis/authorization.k8s.io/v1/selfsubjectrulesreviews').reply(200, { status: emptyRules }) + const ssarScope = nock(apiUrl()) + .post('/apis/authorization.k8s.io/v1/selfsubjectaccessreviews') + .reply(200, { status: { allowed: true } }) + + expect(await canGetResource(managedCluster('cluster-2'), 'ssrr-retry-token')).toBe(false) + expect(ssarScope.isDone()).toBe(false) + }) }) }) diff --git a/backend/test/routes/eventsCache.test.ts b/backend/test/routes/eventsCache.test.ts index aa1820145dc..3da9befc8a1 100644 --- a/backend/test/routes/eventsCache.test.ts +++ b/backend/test/routes/eventsCache.test.ts @@ -201,7 +201,7 @@ describe('eventsCache', () => { expect(result1).toBe(true) expect(result1).toBe(result2) expect(getAccessCache()[mockToken]).toBeUndefined() - expect(getAccessCache()[hashAccessToken(mockToken)]['get:Pod:default:test-pod']).toBeDefined() + expect(getAccessCache()[hashAccessToken(mockToken)]['get::Pod:default:test-pod']).toBeDefined() }) it('should use distinct cache keys per verb', async () => { @@ -219,8 +219,8 @@ describe('eventsCache', () => { expect(await canAccess(resource, 'list', mockToken)).toBe(false) const tokenCache = getAccessCache()[hashAccessToken(mockToken)] - expect(tokenCache['get:Pod:default:test-pod']).toBeDefined() - expect(tokenCache['list:Pod:default:test-pod']).toBeDefined() + expect(tokenCache['get::Pod:default:test-pod']).toBeDefined() + expect(tokenCache['list::Pod:default:test-pod']).toBeDefined() }) it('should respect TTL and refetch after expiry', async () => { @@ -229,7 +229,7 @@ describe('eventsCache', () => { const tokenKey = hashAccessToken(mockToken) cache[tokenKey] = { - 'get:Secret:default:credentials': { + 'get::Secret:default:credentials': { time: Date.now() - ACCESS_CACHE_TTL - 1000, promise: Promise.resolve(true), }, From c76e385f611ed301fcf728845f79bdf748866e70 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 14:04:34 +0200 Subject: [PATCH 09/10] eventsAccess prettier fix Signed-off-by: Enrique Mingorance Cano --- backend/src/routes/eventsAccess.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts index e8e35ccccab..6c5a92f1506 100644 --- a/backend/src/routes/eventsAccess.ts +++ b/backend/src/routes/eventsAccess.ts @@ -29,7 +29,10 @@ export interface SubjectRulesStatus { } export type KindGetAccess = - { type: 'deny-all' } | { type: 'allow-all' } | { type: 'allow-names'; names: Set } | { type: 'incomplete' } + | { type: 'deny-all' } + | { type: 'allow-all' } + | { type: 'allow-names'; names: Set } + | { type: 'incomplete' } export type AccessResource = { kind: string; apiVersion: string; metadata?: { name?: string; namespace?: string } } From 0a66bdb8a07e5bfff7433dbc225b09837dcb61fe Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Mon, 14 Sep 2026 14:52:40 +0200 Subject: [PATCH 10/10] CLUSTER_SCOPED_KINDS kevin's proposal Signed-off-by: Enrique Mingorance Cano --- .../src/routes/aggregators/applications.ts | 14 +-- backend/src/routes/events.ts | 115 +----------------- backend/src/routes/eventsAccess.ts | 10 +- backend/src/routes/eventsDefinitions.ts | 107 ++++++++++++++++ backend/test/routes/eventsAccess.test.ts | 2 - 5 files changed, 118 insertions(+), 130 deletions(-) create mode 100644 backend/src/routes/eventsDefinitions.ts diff --git a/backend/src/routes/aggregators/applications.ts b/backend/src/routes/aggregators/applications.ts index 10747498242..a304f3a7321 100644 --- a/backend/src/routes/aggregators/applications.ts +++ b/backend/src/routes/aggregators/applications.ts @@ -10,13 +10,13 @@ import { addArgoQueryInputs, cacheArgoApplications } from './applicationsArgo' import { getGiganticApps } from '../../lib/gigantic' export enum AppColumns { - 'name' = 0, - 'type', - 'namespace', - 'clusters', - 'repo', - 'timeWindow', - 'created', + name = 0, + type, + namespace, + clusters, + repo, + timeWindow, + created, } export interface IArgoApplication extends IResource { cluster?: string diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 14ee7be3155..32ab8fffb1d 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -14,13 +14,8 @@ import { type ServerSideEvent, ServerSideEvents } from '../lib/server-side-event import { getCACertificate, getServiceAccountToken } from '../lib/serviceAccountToken' import { getAuthenticatedToken } from '../lib/token' import type { IResource } from '../resources/resource' -import { - canAccess, - canGetResource, - canListClusterScopedKind, - canListNamespacedScopedKind, - configureClusterScopedKinds, -} from './eventsAccess' +import { canAccess, canGetResource, canListClusterScopedKind, canListNamespacedScopedKind } from './eventsAccess' +import { definitions, type IWatchOptions } from './eventsDefinitions' import { startAccessCacheCleanup, stopAccessCacheCleanup } from './eventsCache' export { @@ -150,99 +145,6 @@ export function initResourceCache(cache: ResourceCache) { export let resourceCache: ResourceCache = {} -const definitions: IWatchOptions[] = [ - { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1', clusterScoped: true }, - { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, - { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1', clusterScoped: true }, - { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, - { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, - { kind: 'Channel', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'GitOpsCluster', apiVersion: 'apps.open-cluster-management.io/v1beta1' }, - { kind: 'HelmRelease', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'PlacementRule', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'Subscription', apiVersion: 'apps.open-cluster-management.io/v1' }, - { kind: 'SubscriptionReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, - { kind: 'Application', apiVersion: 'argoproj.io/v1alpha1' }, - { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1' }, - { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, - { kind: 'MulticlusterApplicationSetReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, - { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, - { - kind: 'CertificateSigningRequest', - apiVersion: 'certificates.k8s.io/v1', - labelSelector: { 'open-cluster-management.io/cluster-name': '' }, - clusterScoped: true, - }, - { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', clusterScoped: true }, - { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, - { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, - { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, - { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2', clusterScoped: true }, - { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, - { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, - { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, - { kind: 'DiscoveryConfig', apiVersion: 'discovery.open-cluster-management.io/v1' }, - { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, - { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1', clusterScoped: true }, - { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, - { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, - { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, - { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1', clusterScoped: true }, - { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, - { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1', clusterScoped: true }, - { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, - { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, - { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, - { kind: 'PolicySet', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, - { kind: 'SubmarinerConfig', apiVersion: 'submarineraddon.open-cluster-management.io/v1alpha1' }, - { kind: 'AnsibleJob', apiVersion: 'tower.ansible.com/v1alpha1' }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'assisted-service' }, - }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, - }, - { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, - { kind: 'Namespace', apiVersion: 'v1', clusterScoped: true }, - { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, - // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios - { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, - { kind: 'Secret', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'auto-import-secret' } }, - { kind: 'PolicyReport', apiVersion: 'wgpolicyk8s.io/v1alpha2' }, - { kind: 'HostedCluster', apiVersion: 'hypershift.openshift.io/v1beta1' }, - { kind: 'NodePool', apiVersion: 'hypershift.openshift.io/v1beta1' }, - { kind: 'AgentMachine', apiVersion: 'capi-provider.agent-install.openshift.io/v1alpha1' }, - { kind: 'ConfigMap', apiVersion: 'v1', labelSelector: { 'hypershift.openshift.io/supported-versions': 'true' } }, - { kind: 'Search', apiVersion: 'search.open-cluster-management.io/v1alpha1' }, - // Configmaps that contain Grafana dashboard IDs - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-clusters-overview' }, - }, - { - kind: 'ConfigMap', - apiVersion: 'v1', - fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-single-vm-view' }, - }, -] - -configureClusterScopedKinds( - definitions.filter((definition) => definition.clusterScoped).map((definition) => definition.kind) -) - export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() @@ -252,19 +154,6 @@ export function startWatching(): void { } } -interface IWatchOptions { - apiVersion: string - kind: string - labelSelector?: Record - fieldSelector?: Record - /** - * True when the Kubernetes resource is cluster-scoped. - * Used by SSE RBAC to decide whether SelfSubjectRulesReview should probe `default` - * (cluster-scoped) or the resource namespace (namespaced). - */ - clusterScoped?: boolean -} - // https://kubernetes.io/docs/reference/using-api/api-concepts/ async function listAndWatch(options: IWatchOptions) { while (!stopping) { diff --git a/backend/src/routes/eventsAccess.ts b/backend/src/routes/eventsAccess.ts index 6c5a92f1506..813032daaa5 100644 --- a/backend/src/routes/eventsAccess.ts +++ b/backend/src/routes/eventsAccess.ts @@ -13,6 +13,7 @@ import { setSsarCacheEntry, setTimedCacheEntry, } from './eventsCache' +import { CLUSTER_SCOPED_KINDS } from './eventsDefinitions' export interface SubjectRulesStatus { incomplete: boolean @@ -39,15 +40,8 @@ export type AccessResource = { kind: string; apiVersion: string; metadata?: { na /** SSRR requires a namespace; cluster-scoped kinds are reviewed in this probe namespace only. */ const CLUSTER_SCOPED_RULES_NAMESPACE = 'default' -let clusterScopedKinds = new Set() - -/** Replace the cluster-scoped kind lookup. Derived from watch definitions in events.ts. */ -export function configureClusterScopedKinds(kinds: Iterable): void { - clusterScopedKinds = new Set(kinds) -} - export function isClusterScopedKind(kind: string): boolean { - return clusterScopedKinds.has(kind) + return CLUSTER_SCOPED_KINDS.has(kind) } const subjectRulesCache = getSubjectRulesCacheStore() diff --git a/backend/src/routes/eventsDefinitions.ts b/backend/src/routes/eventsDefinitions.ts new file mode 100644 index 00000000000..e58d9d07247 --- /dev/null +++ b/backend/src/routes/eventsDefinitions.ts @@ -0,0 +1,107 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +export interface IWatchOptions { + apiVersion: string + kind: string + labelSelector?: Record + fieldSelector?: Record + /** + * True when the Kubernetes resource is cluster-scoped. + * Used by SSE RBAC to decide whether SelfSubjectRulesReview should probe `default` + * (cluster-scoped) or the resource namespace (namespaced). + */ + clusterScoped?: boolean +} + +export const definitions: IWatchOptions[] = [ + { kind: 'ClusterManagementAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1', clusterScoped: true }, + { kind: 'ManagedClusterAddOn', apiVersion: 'addon.open-cluster-management.io/v1alpha1' }, + { kind: 'Agent', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'AgentServiceConfig', apiVersion: 'agent-install.openshift.io/v1beta1', clusterScoped: true }, + { kind: 'InfraEnv', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'NMStateConfig', apiVersion: 'agent-install.openshift.io/v1beta1' }, + { kind: 'Application', apiVersion: 'app.k8s.io/v1beta1' }, + { kind: 'Channel', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'GitOpsCluster', apiVersion: 'apps.open-cluster-management.io/v1beta1' }, + { kind: 'HelmRelease', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'PlacementRule', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'Subscription', apiVersion: 'apps.open-cluster-management.io/v1' }, + { kind: 'SubscriptionReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, + { kind: 'Application', apiVersion: 'argoproj.io/v1alpha1' }, + { kind: 'ApplicationSet', apiVersion: 'argoproj.io/v1alpha1' }, + { kind: 'ArgoCD', apiVersion: 'argoproj.io/v1alpha1' }, + { kind: 'MulticlusterApplicationSetReport', apiVersion: 'apps.open-cluster-management.io/v1alpha1' }, + { kind: 'Infrastructure', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { + kind: 'CertificateSigningRequest', + apiVersion: 'certificates.k8s.io/v1', + labelSelector: { 'open-cluster-management.io/cluster-name': '' }, + clusterScoped: true, + }, + { kind: 'ManagedCluster', apiVersion: 'cluster.open-cluster-management.io/v1', clusterScoped: true }, + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'Placement', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, + { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1alpha1' }, + { kind: 'PlacementDecision', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'ManagedClusterSetBinding', apiVersion: 'cluster.open-cluster-management.io/v1beta2' }, + { kind: 'ManagedClusterSet', apiVersion: 'cluster.open-cluster-management.io/v1beta2', clusterScoped: true }, + { kind: 'ClusterCurator', apiVersion: 'cluster.open-cluster-management.io/v1beta1' }, + { kind: 'Subscription', apiVersion: 'operators.coreos.com/v1alpha1' }, + { kind: 'DiscoveredCluster', apiVersion: 'discovery.open-cluster-management.io/v1' }, + { kind: 'DiscoveryConfig', apiVersion: 'discovery.open-cluster-management.io/v1' }, + { kind: 'AgentClusterInstall', apiVersion: 'extensions.hive.openshift.io/v1beta1' }, + { kind: 'ClusterClaim', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterDeployment', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterImageSet', apiVersion: 'hive.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterPool', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ClusterProvision', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'MachinePool', apiVersion: 'hive.openshift.io/v1' }, + { kind: 'ManagedClusterInfo', apiVersion: 'internal.open-cluster-management.io/v1beta1' }, + { kind: 'BareMetalHost', apiVersion: 'metal3.io/v1alpha1' }, + { kind: 'MultiClusterEngine', apiVersion: 'multicluster.openshift.io/v1', clusterScoped: true }, + { kind: 'ClusterVersion', apiVersion: 'config.openshift.io/v1', clusterScoped: true }, + { kind: 'StorageClass', apiVersion: 'storage.k8s.io/v1', clusterScoped: true }, + { kind: 'PlacementBinding', apiVersion: 'policy.open-cluster-management.io/v1' }, + { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1' }, + { kind: 'PolicyAutomation', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, + { kind: 'PolicySet', apiVersion: 'policy.open-cluster-management.io/v1beta1' }, + { kind: 'SubmarinerConfig', apiVersion: 'submarineraddon.open-cluster-management.io/v1alpha1' }, + { kind: 'AnsibleJob', apiVersion: 'tower.ansible.com/v1alpha1' }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'assisted-service' }, + }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.namespace': 'openshift-config-managed', 'metadata.name': 'console-public' }, + }, + { kind: 'ConfigMap', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'console-search-config' } }, + { kind: 'Namespace', apiVersion: 'v1', clusterScoped: true }, + { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/credentials': '' } }, + // **Need to look for creds with: 'cluster.open-cluster-management.io/type': 'ans', for edit scenarios + { kind: 'Secret', apiVersion: 'v1', labelSelector: { 'cluster.open-cluster-management.io/type': 'ans' } }, + { kind: 'Secret', apiVersion: 'v1', fieldSelector: { 'metadata.name': 'auto-import-secret' } }, + { kind: 'PolicyReport', apiVersion: 'wgpolicyk8s.io/v1alpha2' }, + { kind: 'HostedCluster', apiVersion: 'hypershift.openshift.io/v1beta1' }, + { kind: 'NodePool', apiVersion: 'hypershift.openshift.io/v1beta1' }, + { kind: 'AgentMachine', apiVersion: 'capi-provider.agent-install.openshift.io/v1alpha1' }, + { kind: 'ConfigMap', apiVersion: 'v1', labelSelector: { 'hypershift.openshift.io/supported-versions': 'true' } }, + { kind: 'Search', apiVersion: 'search.open-cluster-management.io/v1alpha1' }, + // Configmaps that contain Grafana dashboard IDs + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-clusters-overview' }, + }, + { + kind: 'ConfigMap', + apiVersion: 'v1', + fieldSelector: { 'metadata.name': 'grafana-dashboard-acm-openshift-virtualization-single-vm-view' }, + }, +] + +export const CLUSTER_SCOPED_KINDS = new Set( + definitions.filter((definition) => definition.clusterScoped).map((definition) => definition.kind) +) diff --git a/backend/test/routes/eventsAccess.test.ts b/backend/test/routes/eventsAccess.test.ts index c4c6c867e75..947fd96c931 100644 --- a/backend/test/routes/eventsAccess.test.ts +++ b/backend/test/routes/eventsAccess.test.ts @@ -5,7 +5,6 @@ import { canGetResource, canListClusterScopedKind, canListNamespacedScopedKind, - configureClusterScopedKinds, } from '../../src/routes/eventsAccess' import { resetAccessCache } from '../../src/routes/eventsCache' @@ -98,7 +97,6 @@ describe('eventsAccess', () => { beforeEach(() => { resetAccessCache() - configureClusterScopedKinds(['ManagedCluster', 'Namespace', 'StorageClass']) process.env.CLUSTER_API_URL = 'https://api.test-cluster.com:6443' })