From fab03cdc169326887fd210a0d174d170a1fd84bc Mon Sep 17 00:00:00 2001 From: Sergi Massaneda Date: Thu, 31 Oct 2024 16:31:00 +0100 Subject: [PATCH] [SecuritySolution][ProductFeatures] Add support for `security.authz.requiredPrivileges` for the API auth control (#198312) ## Summary Adds support for the new API routes security authorization properties ([docs](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)) to the `ProductFeaturesService` API authorization control Closes: https://github.com/elastic/kibana/issues/194445 Related: https://github.com/elastic/kibana/issues/184674 --------- Co-authored-by: Elastic Machine --- .../product_features_service.test.ts | 364 +++++++++++++----- .../product_features_service.ts | 61 ++- 2 files changed, 327 insertions(+), 98 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts b/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts index a1b71a9c4f04f..8d274a30ca3c9 100644 --- a/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts +++ b/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts @@ -23,6 +23,7 @@ import type { import { ProductFeatureKey } from '@kbn/security-solution-features/keys'; import { httpServiceMock } from '@kbn/core-http-server-mocks'; import type { + AuthzEnabled, KibanaRequest, LifecycleResponseFactory, OnPostAuthHandler, @@ -181,11 +182,6 @@ describe('ProductFeaturesService', () => { lastRegisteredFn = fn; }); - const getReq = (tags: string[] = []) => - ({ - route: { options: { tags } }, - url: { pathname: '', search: '' }, - } as unknown as KibanaRequest); const res = { notFound: jest.fn() } as unknown as LifecycleResponseFactory; const toolkit = httpServiceMock.createOnPostAuthToolkit(); @@ -204,93 +200,281 @@ describe('ProductFeaturesService', () => { expect(mockHttpSetup.registerOnPostAuth).toHaveBeenCalledTimes(1); }); - it('should authorize when no tag matches', async () => { - const experimentalFeatures = {} as ExperimentalFeatures; - const productFeaturesService = new ProductFeaturesService( - loggerMock.create(), - experimentalFeatures - ); - productFeaturesService.registerApiAccessControl(mockHttpSetup); - - await lastRegisteredFn(getReq(['access:something', 'access:securitySolution']), res, toolkit); - - expect(MockedProductFeatures.mock.instances[0].isActionRegistered).not.toHaveBeenCalled(); - expect(res.notFound).not.toHaveBeenCalled(); - expect(toolkit.next).toHaveBeenCalledTimes(1); - }); - - it('should check when tag matches and return not found when not action registered', async () => { - const experimentalFeatures = {} as ExperimentalFeatures; - const productFeaturesService = new ProductFeaturesService( - loggerMock.create(), - experimentalFeatures - ); - productFeaturesService.registerApiAccessControl(mockHttpSetup); - - (MockedProductFeatures.mock.instances[0].isActionRegistered as jest.Mock).mockReturnValueOnce( - false - ); - await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); - - expect(MockedProductFeatures.mock.instances[0].isActionRegistered).toHaveBeenCalledWith( - 'api:securitySolution-foo' - ); - expect(res.notFound).toHaveBeenCalledTimes(1); - expect(toolkit.next).not.toHaveBeenCalled(); - }); - - it('should check when tag matches and continue when action registered', async () => { - const experimentalFeatures = {} as ExperimentalFeatures; - const productFeaturesService = new ProductFeaturesService( - loggerMock.create(), - experimentalFeatures - ); - productFeaturesService.registerApiAccessControl(mockHttpSetup); - - (MockedProductFeatures.mock.instances[0].isActionRegistered as jest.Mock).mockReturnValueOnce( - true - ); - await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); - - expect(MockedProductFeatures.mock.instances[0].isActionRegistered).toHaveBeenCalledWith( - 'api:securitySolution-foo' - ); - expect(res.notFound).not.toHaveBeenCalled(); - expect(toolkit.next).toHaveBeenCalledTimes(1); - }); - - it('should check when productFeature tag when it matches and return not found when not enabled', async () => { - const experimentalFeatures = {} as ExperimentalFeatures; - const productFeaturesService = new ProductFeaturesService( - loggerMock.create(), - experimentalFeatures - ); - productFeaturesService.registerApiAccessControl(mockHttpSetup); - - productFeaturesService.isEnabled = jest.fn().mockReturnValueOnce(false); - - await lastRegisteredFn(getReq(['securitySolutionProductFeature:foo']), res, toolkit); - - expect(productFeaturesService.isEnabled).toHaveBeenCalledWith('foo'); - expect(res.notFound).toHaveBeenCalledTimes(1); - expect(toolkit.next).not.toHaveBeenCalled(); + describe('when using productFeature tag', () => { + const getReq = (tags: string[] = []) => + ({ + route: { options: { tags } }, + url: { pathname: '', search: '' }, + } as unknown as KibanaRequest); + + it('should check when productFeature tag when it matches and return not found when not enabled', async () => { + const experimentalFeatures = {} as ExperimentalFeatures; + const productFeaturesService = new ProductFeaturesService( + loggerMock.create(), + experimentalFeatures + ); + productFeaturesService.registerApiAccessControl(mockHttpSetup); + + productFeaturesService.isEnabled = jest.fn().mockReturnValueOnce(false); + + await lastRegisteredFn(getReq(['securitySolutionProductFeature:foo']), res, toolkit); + + expect(productFeaturesService.isEnabled).toHaveBeenCalledWith('foo'); + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + it('should check when productFeature tag when it matches and continue when enabled', async () => { + const experimentalFeatures = {} as ExperimentalFeatures; + const productFeaturesService = new ProductFeaturesService( + loggerMock.create(), + experimentalFeatures + ); + productFeaturesService.registerApiAccessControl(mockHttpSetup); + + productFeaturesService.isEnabled = jest.fn().mockReturnValueOnce(true); + + await lastRegisteredFn(getReq(['securitySolutionProductFeature:foo']), res, toolkit); + + expect(productFeaturesService.isEnabled).toHaveBeenCalledWith('foo'); + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); }); - it('should check when productFeature tag when it matches and continue when enabled', async () => { - const experimentalFeatures = {} as ExperimentalFeatures; - const productFeaturesService = new ProductFeaturesService( - loggerMock.create(), - experimentalFeatures - ); - productFeaturesService.registerApiAccessControl(mockHttpSetup); - - productFeaturesService.isEnabled = jest.fn().mockReturnValueOnce(true); - - await lastRegisteredFn(getReq(['securitySolutionProductFeature:foo']), res, toolkit); - - expect(productFeaturesService.isEnabled).toHaveBeenCalledWith('foo'); - expect(res.notFound).not.toHaveBeenCalled(); - expect(toolkit.next).toHaveBeenCalledTimes(1); + // Documentation: https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization + describe('when using authorization', () => { + let productFeaturesService: ProductFeaturesService; + let mockIsActionRegistered: jest.Mock; + + beforeEach(() => { + const experimentalFeatures = {} as ExperimentalFeatures; + productFeaturesService = new ProductFeaturesService( + loggerMock.create(), + experimentalFeatures + ); + productFeaturesService.registerApiAccessControl(mockHttpSetup); + mockIsActionRegistered = MockedProductFeatures.mock.instances[0] + .isActionRegistered as jest.Mock; + }); + + describe('when using access tag', () => { + const getReq = (tags: string[] = []) => + ({ + route: { options: { tags } }, + url: { pathname: '', search: '' }, + } as unknown as KibanaRequest); + + it('should authorize when no tag matches', async () => { + await lastRegisteredFn( + getReq(['access:something', 'access:securitySolution']), + res, + toolkit + ); + + expect(mockIsActionRegistered).not.toHaveBeenCalled(); + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should check when tag matches and return not found when not action registered', async () => { + mockIsActionRegistered.mockReturnValueOnce(false); + await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-foo'); + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + it('should check when tag matches and continue when action registered', async () => { + mockIsActionRegistered.mockReturnValueOnce(true); + await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-foo'); + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + }); + + describe('when using security authz', () => { + beforeEach(() => { + mockIsActionRegistered.mockImplementation((action: string) => action.includes('enabled')); + }); + + const getReq = (requiredPrivileges?: AuthzEnabled['requiredPrivileges']) => + ({ + route: { options: { security: { authz: { requiredPrivileges } } } }, + url: { pathname: '', search: '' }, + } as unknown as KibanaRequest); + + it('should authorize when no privilege matches', async () => { + await lastRegisteredFn(getReq(['something', 'securitySolution']), res, toolkit); + + expect(mockIsActionRegistered).not.toHaveBeenCalled(); + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should check when privilege matches and return not found when not action registered', async () => { + await lastRegisteredFn(getReq(['securitySolution-disabled']), res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + it('should check when privilege matches and continue when action registered', async () => { + mockIsActionRegistered.mockReturnValueOnce(true); + await lastRegisteredFn(getReq(['securitySolution-enabled']), res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should restrict access when one action is not registered', async () => { + mockIsActionRegistered.mockReturnValueOnce(true); + await lastRegisteredFn( + getReq([ + 'securitySolution-enabled', + 'securitySolution-disabled', + 'securitySolution-enabled2', + ]), + res, + toolkit + ); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(2); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + describe('when using nested requiredPrivileges', () => { + describe('when using allRequired', () => { + it('should allow access when all actions are registered', async () => { + const req = getReq([ + { + allRequired: [ + 'securitySolution-enabled', + 'securitySolution-enabled2', + 'securitySolution-enabled3', + ], + }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(3); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled2'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled3'); + + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should restrict access if one action is not registered', async () => { + const req = getReq([ + { + allRequired: [ + 'securitySolution-enabled', + 'securitySolution-disabled', + 'securitySolution-notCalled', + ], + }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(2); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + it('should allow only based on security privileges and ignore non-security', async () => { + const req = getReq([ + { allRequired: ['notSecurityPrivilege', 'securitySolution-enabled'] }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(1); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should restrict only based on security privileges and ignore non-security', async () => { + const req = getReq([ + { allRequired: ['notSecurityPrivilege', 'securitySolution-disabled'] }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(1); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + }); + + describe('when using anyRequired', () => { + it('should allow access when one action is registered', async () => { + const req = getReq([ + { + anyRequired: [ + 'securitySolution-disabled', + 'securitySolution-enabled', + 'securitySolution-notCalled', + ], + }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(2); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-enabled'); + + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + + it('should restrict access when no action is registered', async () => { + const req = getReq([ + { + anyRequired: ['securitySolution-disabled', 'securitySolution-disabled2'], + }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).toHaveBeenCalledTimes(2); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled'); + expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-disabled2'); + + expect(res.notFound).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalled(); + }); + + it('should restrict only based on security privileges and allow when non-security privilege is present', async () => { + const req = getReq([ + { + anyRequired: ['notSecurityPrivilege', 'securitySolution-disabled'], + }, + ]); + await lastRegisteredFn(req, res, toolkit); + + expect(mockIsActionRegistered).not.toHaveBeenCalled(); + + expect(res.notFound).not.toHaveBeenCalled(); + expect(toolkit.next).toHaveBeenCalledTimes(1); + }); + }); + }); + }); }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.ts b/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.ts index 29ef513b40bb3..86928ff905545 100644 --- a/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.ts +++ b/x-pack/plugins/security_solution/server/lib/product_features_service/product_features_service.ts @@ -11,7 +11,7 @@ * 2.0. */ -import type { HttpServiceSetup, Logger } from '@kbn/core/server'; +import type { AuthzEnabled, HttpServiceSetup, Logger, RouteAuthz } from '@kbn/core/server'; import { hiddenTypes as filesSavedObjectTypes } from '@kbn/files-plugin/server/saved_objects'; import type { FeaturesPluginSetup } from '@kbn/features-plugin/server'; import type { ProductFeatureKeyType } from '@kbn/security-solution-features'; @@ -21,6 +21,7 @@ import { getCasesFeature, getSecurityFeature, } from '@kbn/security-solution-features/product_features'; +import type { RecursiveReadonly } from '@kbn/utility-types'; import type { ExperimentalFeatures } from '../../../common'; import { APP_ID } from '../../../common'; import { ProductFeatures } from './product_features'; @@ -28,6 +29,9 @@ import type { ProductFeaturesConfigurator } from './types'; import { securityDefaultSavedObjects } from './security_saved_objects'; import { casesApiTags, casesUiCapabilities } from './cases_privileges'; +// The prefix ("securitySolution-") used by all the Security Solution API action privileges. +export const API_ACTION_PREFIX = `${APP_ID}-`; + export class ProductFeaturesService { private securityProductFeatures: ProductFeatures; private casesProductFeatures: ProductFeatures; @@ -116,8 +120,6 @@ export class ProductFeaturesService { return this.productFeatures.has(productFeatureKey); } - public getApiActionName = (apiPrivilege: string) => `api:${APP_ID}-${apiPrivilege}`; - public isActionRegistered(action: string) { return ( this.securityProductFeatures.isActionRegistered(action) || @@ -127,6 +129,9 @@ export class ProductFeaturesService { ); } + public getApiActionName = (apiPrivilege: string) => `api:${API_ACTION_PREFIX}${apiPrivilege}`; + + /** @deprecated Use security.authz.requiredPrivileges instead */ public isApiPrivilegeEnabled(apiPrivilege: string) { return this.isActionRegistered(this.getApiActionName(apiPrivilege)); } @@ -135,14 +140,24 @@ export class ProductFeaturesService { // The `securitySolutionProductFeature:` prefix is used for ProductFeature based control. // Should be used only by routes that do not need RBAC, only direct productFeature control. const APP_FEATURE_TAG_PREFIX = 'securitySolutionProductFeature:'; - // The "access:securitySolution-" prefix is used for API action based control. - // Should be used by routes that need RBAC, extending the `access:` role privilege check from the security plugin. - // An additional check is performed to ensure the privilege has been registered by the productFeature service, - // preventing full access (`*`) roles, such as superuser, from bypassing productFeature controls. + + /** @deprecated Use security.authz.requiredPrivileges instead */ const API_ACTION_TAG_PREFIX = `access:${APP_ID}-`; + const isAuthzEnabled = (authz?: RecursiveReadonly): authz is AuthzEnabled => { + return Boolean((authz as AuthzEnabled)?.requiredPrivileges); + }; + + /** Returns true only if the API privilege is a security action and is disabled */ + const isApiPrivilegeSecurityAndDisabled = (apiPrivilege: string): boolean => { + if (apiPrivilege.startsWith(API_ACTION_PREFIX)) { + return !this.isActionRegistered(`api:${apiPrivilege}`); + } + return false; + }; + http.registerOnPostAuth((request, response, toolkit) => { - for (const tag of request.route.options.tags) { + for (const tag of request.route.options.tags ?? []) { let isEnabled = true; if (tag.startsWith(APP_FEATURE_TAG_PREFIX)) { isEnabled = this.isEnabled( @@ -159,6 +174,36 @@ export class ProductFeaturesService { return response.notFound(); } } + + // This control ensures the action privileges have been registered by the productFeature service, + // preventing full access (`*`) roles, such as superuser, from bypassing productFeature controls. + const authz = request.route.options.security?.authz; + if (isAuthzEnabled(authz)) { + const disabled = authz.requiredPrivileges.some((privilegeEntry) => { + if (typeof privilegeEntry === 'object') { + if (privilegeEntry.allRequired) { + if (privilegeEntry.allRequired.some(isApiPrivilegeSecurityAndDisabled)) { + return true; + } + } + if (privilegeEntry.anyRequired) { + if (privilegeEntry.anyRequired.every(isApiPrivilegeSecurityAndDisabled)) { + return true; + } + } + return false; + } else { + return isApiPrivilegeSecurityAndDisabled(privilegeEntry); + } + }); + if (disabled) { + this.logger.warn( + `Accessing disabled route "${request.url.pathname}${request.url.search}": responding with 404` + ); + return response.notFound(); + } + } + return toolkit.next(); }); }