From f0d971669f2114c86c8328ab95abee0915119502 Mon Sep 17 00:00:00 2001 From: Andreas Hippler Date: Thu, 17 Nov 2022 18:04:39 +0100 Subject: [PATCH] fix: getAllowedResources for all namespaces using SelfSubjectRulesReview Signed-off-by: Andreas Hippler --- ...thorization-namespace-review.injectable.ts | 70 +++++++++++++++++++ .../authorization-review.injectable.ts | 63 ++++++++++------- src/common/cluster/cluster.ts | 45 +++++++----- src/main/__test__/cluster.test.ts | 3 + .../create-cluster.injectable.ts | 2 + .../create-cluster.injectable.ts | 1 + 6 files changed, 142 insertions(+), 42 deletions(-) create mode 100644 src/common/cluster/authorization-namespace-review.injectable.ts diff --git a/src/common/cluster/authorization-namespace-review.injectable.ts b/src/common/cluster/authorization-namespace-review.injectable.ts new file mode 100644 index 0000000000..2537331fe5 --- /dev/null +++ b/src/common/cluster/authorization-namespace-review.injectable.ts @@ -0,0 +1,70 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import type { KubeConfig } from "@kubernetes/client-node"; +import { AuthorizationV1Api } from "@kubernetes/client-node"; +import { getInjectable } from "@ogre-tools/injectable"; +import type { Logger } from "../logger"; +import loggerInjectable from "../logger.injectable"; + +export type RequestNamespaceResources = (namespace: string) => Promise; + +/** + * @param proxyConfig This config's `currentContext` field must be set, and will be used as the target cluster + */ +export type AuthorizationNamespaceReview = (proxyConfig: KubeConfig) => RequestNamespaceResources; + +interface Dependencies { + logger: Logger; +} + +const authorizationNamespaceReview = ({ logger }: Dependencies): AuthorizationNamespaceReview => { + return (proxyConfig) => { + + const api = proxyConfig.makeApiClient(AuthorizationV1Api); + + /** + * Requests the permissions for actions on the kube cluster + * @param namespace The namespace of the resources + * @returns list of allowed resources + */ + return async (namespace: string): Promise => { + try { + const { body } = await api.createSelfSubjectRulesReview({ + apiVersion: "authorization.k8s.io/v1", + kind: "SelfSubjectRulesReview", + spec: { namespace }, + }); + + const resources = new Set(); + + body.status?.resourceRules.forEach(resourceRule => { + if (resourceRule.verbs.some(verb => ["*", "list"].includes(verb))) { + resourceRule.resources?.forEach(resource => resources.add(resource)); + } + }); + + resources.delete("*"); + + return [...resources]; + } catch (error) { + logger.error(`[AUTHORIZATION-NAMESPACE-REVIEW]: failed to create subject rules review: ${error}`, { namespace }); + + return []; + } + }; + }; +}; + +const authorizationNamespaceReviewInjectable = getInjectable({ + id: "authorization-namespace-review", + instantiate: (di) => { + const logger = di.inject(loggerInjectable); + + return authorizationNamespaceReview({ logger }); + }, +}); + +export default authorizationNamespaceReviewInjectable; diff --git a/src/common/cluster/authorization-review.injectable.ts b/src/common/cluster/authorization-review.injectable.ts index c622893b63..4c9b83330d 100644 --- a/src/common/cluster/authorization-review.injectable.ts +++ b/src/common/cluster/authorization-review.injectable.ts @@ -5,42 +5,55 @@ import type { KubeConfig, V1ResourceAttributes } from "@kubernetes/client-node"; import { AuthorizationV1Api } from "@kubernetes/client-node"; -import logger from "../logger"; import { getInjectable } from "@ogre-tools/injectable"; +import type { Logger } from "../logger"; +import loggerInjectable from "../logger.injectable"; +/** + * Requests the permissions for actions on the kube cluster + * @param resourceAttributes The descriptor of the action that is desired to be known if it is allowed + * @returns `true` if the actions described are allowed + */ export type CanI = (resourceAttributes: V1ResourceAttributes) => Promise; /** * @param proxyConfig This config's `currentContext` field must be set, and will be used as the target cluster - */ -export function authorizationReview(proxyConfig: KubeConfig): CanI { - const api = proxyConfig.makeApiClient(AuthorizationV1Api); + */ +export type AuthorizationReview = (proxyConfig: KubeConfig) => CanI; - /** - * Requests the permissions for actions on the kube cluster - * @param resourceAttributes The descriptor of the action that is desired to be known if it is allowed - * @returns `true` if the actions described are allowed - */ - return async (resourceAttributes: V1ResourceAttributes): Promise => { - try { - const { body } = await api.createSelfSubjectAccessReview({ - apiVersion: "authorization.k8s.io/v1", - kind: "SelfSubjectAccessReview", - spec: { resourceAttributes }, - }); - - return body.status?.allowed ?? false; - } catch (error) { - logger.error(`[AUTHORIZATION-REVIEW]: failed to create access review: ${error}`, { resourceAttributes }); - - return false; - } - }; +interface Dependencies { + logger: Logger; } +const authorizationReview = ({ logger }: Dependencies): AuthorizationReview => { + return (proxyConfig) => { + const api = proxyConfig.makeApiClient(AuthorizationV1Api); + + return async (resourceAttributes: V1ResourceAttributes): Promise => { + try { + const { body } = await api.createSelfSubjectAccessReview({ + apiVersion: "authorization.k8s.io/v1", + kind: "SelfSubjectAccessReview", + spec: { resourceAttributes }, + }); + + return body.status?.allowed ?? false; + } catch (error) { + logger.error(`[AUTHORIZATION-REVIEW]: failed to create access review: ${error}`, { resourceAttributes }); + + return false; + } + }; + }; +}; + const authorizationReviewInjectable = getInjectable({ id: "authorization-review", - instantiate: () => authorizationReview, + instantiate: (di) => { + const logger = di.inject(loggerInjectable); + + return authorizationReview({ logger }); + }, }); export default authorizationReviewInjectable; diff --git a/src/common/cluster/cluster.ts b/src/common/cluster/cluster.ts index b0d7fbfbc3..3d9f4be437 100644 --- a/src/common/cluster/cluster.ts +++ b/src/common/cluster/cluster.ts @@ -25,6 +25,7 @@ import assert from "assert"; import type { Logger } from "../logger"; import type { BroadcastMessage } from "../ipc/broadcast-message.injectable"; import type { LoadConfigfromFile } from "../kube-helpers/load-config-from-file.injectable"; +import type { RequestNamespaceResources } from "./authorization-namespace-review.injectable"; export interface ClusterDependencies { readonly directoryForKubeConfigs: string; @@ -34,6 +35,7 @@ export interface ClusterDependencies { createContextHandler: (cluster: Cluster) => ClusterContextHandler; createKubectl: (clusterVersion: string) => Kubectl; createAuthorizationReview: (config: KubeConfig) => CanI; + createAuthorizationNamespaceReview: (config: KubeConfig) => RequestNamespaceResources; createListNamespaces: (config: KubeConfig) => ListNamespaces; createVersionDetector: (cluster: Cluster) => VersionDetector; broadcastMessage: BroadcastMessage; @@ -472,8 +474,10 @@ export class Cluster implements ClusterModel, ClusterState { * @internal */ private async refreshAccessibility(): Promise { + this.dependencies.logger.info(`[CLUSTER]: refreshAccessibility`, this.getMeta()); const proxyConfig = await this.getProxyKubeconfig(); const canI = this.dependencies.createAuthorizationReview(proxyConfig); + const requestNamespaceResources = this.dependencies.createAuthorizationNamespaceReview(proxyConfig); this.isAdmin = await canI({ namespace: "kube-system", @@ -485,7 +489,7 @@ export class Cluster implements ClusterModel, ClusterState { resource: "*", }); this.allowedNamespaces = await this.getAllowedNamespaces(proxyConfig); - this.allowedResources = await this.getAllowedResources(canI); + this.allowedResources = await this.getAllowedResources(requestNamespaceResources); this.ready = true; } @@ -667,32 +671,39 @@ export class Cluster implements ClusterModel, ClusterState { } } - protected async getAllowedResources(canI: CanI) { + protected async getAllowedResources(requestNamespaceResources: RequestNamespaceResources) { try { if (!this.allowedNamespaces.length) { return []; } + const resources = apiResources.filter((resource) => this.resourceAccessStatuses.get(resource) === undefined); - const apiLimit = plimit(5); // 5 concurrent api requests - const requests = []; + const unknownResources = new Map(resources.map(resource => ([resource.apiName, resource]))); - for (const apiResource of resources) { - requests.push(apiLimit(async () => { - for (const namespace of this.allowedNamespaces.slice(0, 10)) { - if (!this.resourceAccessStatuses.get(apiResource)) { - const result = await canI({ - resource: apiResource.apiName, - group: apiResource.group, - verb: "list", - namespace, - }); + if (unknownResources.size > 0) { + const apiLimit = plimit(5); // 5 concurrent api requests - this.resourceAccessStatuses.set(apiResource, result); + await Promise.all(this.allowedNamespaces.map(namespace => apiLimit(async () => { + if (unknownResources.size === 0) { + return; + } + + const namespaceResources = await requestNamespaceResources(namespace); + + for (const resourceName of namespaceResources) { + const unknownResource = unknownResources.get(resourceName); + + if (unknownResource) { + this.resourceAccessStatuses.set(unknownResource, true); + unknownResources.delete(resourceName); } } - })); + }))); + + for (const forbiddenResource of unknownResources.values()) { + this.resourceAccessStatuses.set(forbiddenResource, false); + } } - await Promise.all(requests); return apiResources .filter((resource) => this.resourceAccessStatuses.get(resource)) diff --git a/src/main/__test__/cluster.test.ts b/src/main/__test__/cluster.test.ts index 0244141ab3..d6510bbb4a 100644 --- a/src/main/__test__/cluster.test.ts +++ b/src/main/__test__/cluster.test.ts @@ -10,6 +10,7 @@ import { getDiForUnitTesting } from "../getDiForUnitTesting"; import type { CreateCluster } from "../../common/cluster/create-cluster-injection-token"; import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token"; import authorizationReviewInjectable from "../../common/cluster/authorization-review.injectable"; +import authorizationNamespaceReviewInjectable from "../../common/cluster/authorization-namespace-review.injectable"; import listNamespacesInjectable from "../../common/cluster/list-namespaces.injectable"; import createContextHandlerInjectable from "../context-handler/create-context-handler.injectable"; import type { ClusterContextHandler } from "../context-handler/context-handler"; @@ -19,6 +20,7 @@ import directoryForTempInjectable from "../../common/app-paths/directory-for-tem import normalizedPlatformInjectable from "../../common/vars/normalized-platform.injectable"; import kubectlBinaryNameInjectable from "../kubectl/binary-name.injectable"; import kubectlDownloadingNormalizedArchInjectable from "../kubectl/normalized-arch.injectable"; +import { apiResourceRecord } from "../../common/rbac"; console = new Console(process.stdout, process.stderr); // fix mockFS @@ -39,6 +41,7 @@ describe("create clusters", () => { di.override(normalizedPlatformInjectable, () => "darwin"); di.override(broadcastMessageInjectable, () => async () => {}); di.override(authorizationReviewInjectable, () => () => () => Promise.resolve(true)); + di.override(authorizationNamespaceReviewInjectable, () => () => () => Promise.resolve(Object.keys(apiResourceRecord))); di.override(listNamespacesInjectable, () => () => () => Promise.resolve([ "default" ])); di.override(createContextHandlerInjectable, () => (cluster) => ({ restartServer: jest.fn(), diff --git a/src/main/create-cluster/create-cluster.injectable.ts b/src/main/create-cluster/create-cluster.injectable.ts index 5290e92db3..0f22cd4c51 100644 --- a/src/main/create-cluster/create-cluster.injectable.ts +++ b/src/main/create-cluster/create-cluster.injectable.ts @@ -11,6 +11,7 @@ import createKubectlInjectable from "../kubectl/create-kubectl.injectable"; import createContextHandlerInjectable from "../context-handler/create-context-handler.injectable"; import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token"; import authorizationReviewInjectable from "../../common/cluster/authorization-review.injectable"; +import createAuthorizationNamespaceReview from "../../common/cluster/authorization-namespace-review.injectable"; import listNamespacesInjectable from "../../common/cluster/list-namespaces.injectable"; import loggerInjectable from "../../common/logger.injectable"; import detectorRegistryInjectable from "../cluster-detectors/detector-registry.injectable"; @@ -28,6 +29,7 @@ const createClusterInjectable = getInjectable({ createKubectl: di.inject(createKubectlInjectable), createContextHandler: di.inject(createContextHandlerInjectable), createAuthorizationReview: di.inject(authorizationReviewInjectable), + createAuthorizationNamespaceReview: di.inject(createAuthorizationNamespaceReview), createListNamespaces: di.inject(listNamespacesInjectable), logger: di.inject(loggerInjectable), detectorRegistry: di.inject(detectorRegistryInjectable), diff --git a/src/renderer/create-cluster/create-cluster.injectable.ts b/src/renderer/create-cluster/create-cluster.injectable.ts index 0c73eb82fa..0b5e51d2d8 100644 --- a/src/renderer/create-cluster/create-cluster.injectable.ts +++ b/src/renderer/create-cluster/create-cluster.injectable.ts @@ -27,6 +27,7 @@ const createClusterInjectable = getInjectable({ createKubectl: () => { throw new Error("Tried to access back-end feature in front-end.");}, createContextHandler: () => undefined as never, createAuthorizationReview: () => { throw new Error("Tried to access back-end feature in front-end."); }, + createAuthorizationNamespaceReview: () => { throw new Error("Tried to access back-end feature in front-end."); }, createListNamespaces: () => { throw new Error("Tried to access back-end feature in front-end."); }, detectorRegistry: undefined as never, createVersionDetector: () => { throw new Error("Tried to access back-end feature in front-end."); },