From 45f102f0febae8ba1f921b59252784bcc58467c2 Mon Sep 17 00:00:00 2001 From: Jeramy Soucy Date: Thu, 20 Apr 2023 16:28:06 -0400 Subject: [PATCH] Fixes security plugin capabilities switcher to handle opt-out and default behaviors (#154098) Closes https://github.com/elastic/kibana/issues/153817 ## Summary This PR implements logical checks within the security plugin's capabilities switcher to account for features that opt out of the Kibana security model (e.g. Enterprise Search features). It also more explicitly handles default cases (when a feature is neither a Kibana or ES feature), exclusions (features handled exclusively by other plugins), and the catalogue feature (we now qualify each catalogue feature capability). In these cases (opt-out, default, exclusion, etc.), the capabilities switcher will ignore the capability and neither enable nor disable it (see detailed list below). We are now effectively ignoring only these: - `spaces` feature ID (handled by spaces plugin capabilities switcher) - `fileUpload` feature ID (handled by file_upload plugin capabilities switcher) - `catalogue` capabilities that are not 'spaces' and are not referenced by at least one Kibana or ES feature - `navLinks` that are not referenced by at least one Kibana feature - Anything that is not a global settings, management, catalogue, nav link, Kibana, or ES feature On the flip side we always affect everything under the `management` feature. This PR _should_ unblock the ability to implement parallel execution of capabilities switchers, https://github.com/elastic/kibana/pull/152982. ### Related Tests - x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts - x-pack/test/ui_capabilities/security_and_spaces/config.ts - x-pack/test/functional/apps/home/config.ts --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../disable_ui_capabilities.test.ts | 559 +++++++++--------- .../authorization/disable_ui_capabilities.ts | 56 +- 2 files changed, 329 insertions(+), 286 deletions(-) diff --git a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts index dd07324835303..7d0039c9106d2 100644 --- a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts +++ b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts @@ -66,8 +66,145 @@ const createMockUser = (user: Partial = {}) => ...user, } as AuthenticatedUser); +const kibanaFeature1 = new KibanaFeature({ + id: 'kibanaFeature1', + name: 'KibanaFeature1', + app: ['app1'], + category: { id: 'foo', label: 'foo' }, + privileges: { + all: { + app: ['foo'], + catalogue: ['foo'], + savedObject: { + all: ['foo'], + read: [], + }, + ui: ['save', 'show'], + }, + read: { + app: ['foo'], + catalogue: ['foo'], + savedObject: { + all: [], + read: ['foo'], + }, + ui: ['show'], + }, + }, +}); + +const kibanaFeature2 = new KibanaFeature({ + id: 'kibanaFeature2', + name: 'KibanaFeature2', + app: ['app1', 'app2'], + category: { id: 'foo', label: 'foo' }, + privileges: { + all: { + app: ['foo'], + catalogue: ['foo'], + savedObject: { + all: ['foo'], + read: [], + }, + ui: ['save', 'show'], + }, + read: { + app: ['foo'], + catalogue: ['foo'], + savedObject: { + all: [], + read: ['foo'], + }, + ui: ['show'], + }, + }, +}); + +const optOutKibanaFeature = new KibanaFeature({ + id: 'optOutFeature', + name: 'Feature that opts out of Kibana sec model', + app: [], + category: { id: 'optOut', label: 'optOut' }, + privileges: null, +}); + +const esManagementFeature = new ElasticsearchFeature({ + id: 'esManagementFeature', + management: { + kibana: ['esManagement'], + }, + privileges: [ + { + requiredClusterPrivileges: ['manage_security'], + ui: [], + }, + ], +}); + describe('usingPrivileges', () => { describe('checkPrivileges errors', () => { + const inputCapabilities = Object.freeze({ + navLinks: { + app1: true, + app2: true, + app3: true, + }, + management: { + kibana: { + indices: true, + }, + }, + catalogue: {}, + kibanaFeature2: { + foo: true, + bar: true, + }, + optOutFeature: { + foo: true, + bar: true, + }, + esManagementFeature: { + foo: true, + bar: true, + }, + unregisteredFeature: { + foo: true, + bar: true, + }, + }); + + const expectedDisabled = Object.freeze({ + navLinks: { + app1: false, + app2: false, + app3: true, // will not diable unregistered app link + }, + management: { + kibana: { + indices: false, + }, + }, + catalogue: {}, + kibanaFeature2: { + foo: false, + bar: false, + }, + optOutFeature: { + // will not disbale features that opt out of Kibana security + foo: true, + bar: true, + }, + esManagementFeature: { + foo: false, + bar: false, + }, + unregisteredFeature: { + // will not disble unregistered features + foo: true, + bar: true, + }, + }); + test(`disables uiCapabilities when a 401 is thrown`, async () => { const mockAuthz = createMockAuthz({ rejectCheckPrivileges: { statusCode: 401, message: 'super informative message' }, @@ -76,76 +213,16 @@ describe('usingPrivileges', () => { const { usingPrivileges } = disableUICapabilitiesFactory( mockRequest, - [ - new KibanaFeature({ - id: 'fooFeature', - name: 'Foo KibanaFeature', - app: ['fooApp', 'foo'], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - ], - [ - new ElasticsearchFeature({ - id: 'esFeature', - privileges: [ - { - requiredClusterPrivileges: [], - ui: [], - }, - ], - }), - ], + [kibanaFeature2, optOutKibanaFeature], + [esManagementFeature], mockLoggers.get(), mockAuthz, createMockUser() ); - const result = await usingPrivileges( - Object.freeze({ - navLinks: { - foo: true, - fooApp: true, - bar: true, - }, - management: { - kibana: { - indices: true, - }, - }, - catalogue: {}, - fooFeature: { - foo: true, - bar: true, - }, - barFeature: { - foo: true, - bar: true, - }, - }) - ); + const result = await usingPrivileges(inputCapabilities); - expect(result).toEqual({ - navLinks: { - foo: false, - fooApp: false, - bar: true, - }, - management: { - kibana: { - indices: false, - }, - }, - catalogue: {}, - fooFeature: { - foo: false, - bar: false, - }, - barFeature: { - foo: false, - bar: false, - }, - }); + expect(result).toEqual(expectedDisabled); expect(loggingSystemMock.collect(mockLoggers).debug).toMatchInlineSnapshot(` Array [ @@ -164,74 +241,17 @@ describe('usingPrivileges', () => { const { usingPrivileges } = disableUICapabilitiesFactory( mockRequest, - [ - new KibanaFeature({ - id: 'fooFeature', - name: 'Foo KibanaFeature', - app: ['foo'], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - ], - [ - new ElasticsearchFeature({ - id: 'esFeature', - privileges: [ - { - requiredClusterPrivileges: [], - ui: [], - }, - ], - }), - ], + [kibanaFeature2, optOutKibanaFeature], + [esManagementFeature], mockLoggers.get(), mockAuthz, createMockUser() ); - const result = await usingPrivileges( - Object.freeze({ - navLinks: { - foo: true, - bar: true, - }, - management: { - kibana: { - indices: true, - }, - }, - catalogue: {}, - fooFeature: { - foo: true, - bar: true, - }, - barFeature: { - foo: true, - bar: true, - }, - }) - ); + const result = await usingPrivileges(inputCapabilities); + + expect(result).toEqual(expectedDisabled); - expect(result).toEqual({ - navLinks: { - foo: false, - bar: true, - }, - management: { - kibana: { - indices: false, - }, - }, - catalogue: {}, - fooFeature: { - foo: false, - bar: false, - }, - barFeature: { - foo: false, - bar: false, - }, - }); expect(loggingSystemMock.collect(mockLoggers).debug).toMatchInlineSnapshot(` Array [ Array [ @@ -284,24 +304,64 @@ describe('usingPrivileges', () => { }); }); + const esFeatures = [ + new ElasticsearchFeature({ + id: 'esFeature', + privileges: [ + { + requiredClusterPrivileges: ['manage'], + ui: ['es_manage'], + }, + { + requiredClusterPrivileges: ['monitor'], + ui: ['es_monitor'], + }, + ], + }), + new ElasticsearchFeature({ + id: 'esSecurityFeature', + privileges: [ + { + requiredClusterPrivileges: ['manage_security'], + ui: ['es_manage_sec'], + }, + ], + }), + new ElasticsearchFeature({ + id: 'esManagementFeature', + management: { + kibana: ['esManagement'], + }, + privileges: [ + { + requiredClusterPrivileges: ['manage_security'], + ui: [], + }, + ], + }), + ]; + test(`disables ui capabilities when they don't have privileges`, async () => { + // grant some privileges const mockAuthz = createMockAuthz({ resolveCheckPrivileges: { privileges: { kibana: [ - { privilege: actions.ui.get('navLinks', 'foo'), authorized: true }, - { privilege: actions.ui.get('navLinks', 'bar'), authorized: false }, - { privilege: actions.ui.get('navLinks', 'quz'), authorized: false }, + { privilege: actions.ui.get('navLinks', 'app1'), authorized: true }, + { privilege: actions.ui.get('navLinks', 'app2'), authorized: false }, + { privilege: actions.ui.get('navLinks', 'app3'), authorized: false }, { privilege: actions.ui.get('management', 'kibana', 'indices'), authorized: true }, { privilege: actions.ui.get('management', 'kibana', 'settings'), authorized: false }, { privilege: actions.ui.get('management', 'kibana', 'esManagement'), authorized: false, }, - { privilege: actions.ui.get('fooFeature', 'foo'), authorized: true }, - { privilege: actions.ui.get('fooFeature', 'bar'), authorized: false }, - { privilege: actions.ui.get('barFeature', 'foo'), authorized: true }, - { privilege: actions.ui.get('barFeature', 'bar'), authorized: false }, + { privilege: actions.ui.get('kibanaFeature1', 'foo'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature1', 'bar'), authorized: false }, + { privilege: actions.ui.get('kibanaFeature2', 'foo'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature2', 'bar'), authorized: false }, + { privilege: actions.ui.get('optOutFeature', 'foo'), authorized: false }, + { privilege: actions.ui.get('optOutFeature', 'bar'), authorized: false }, ], elasticsearch: { cluster: [ @@ -317,58 +377,8 @@ describe('usingPrivileges', () => { const { usingPrivileges } = disableUICapabilitiesFactory( mockRequest, - [ - new KibanaFeature({ - id: 'fooFeature', - name: 'Foo KibanaFeature', - app: [], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - new KibanaFeature({ - id: 'barFeature', - name: 'Bar KibanaFeature', - app: ['bar'], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - ], - [ - new ElasticsearchFeature({ - id: 'esFeature', - privileges: [ - { - requiredClusterPrivileges: ['manage'], - ui: ['es_manage'], - }, - { - requiredClusterPrivileges: ['monitor'], - ui: ['es_monitor'], - }, - ], - }), - new ElasticsearchFeature({ - id: 'esSecurityFeature', - privileges: [ - { - requiredClusterPrivileges: ['manage_security'], - ui: ['es_manage_sec'], - }, - ], - }), - new ElasticsearchFeature({ - id: 'esManagementFeature', - management: { - kibana: ['esManagement'], - }, - privileges: [ - { - requiredClusterPrivileges: ['manage_security'], - ui: [], - }, - ], - }), - ], + [kibanaFeature1, kibanaFeature2, optOutKibanaFeature], + esFeatures, loggingSystemMock.create().get(), mockAuthz, createMockUser() @@ -377,9 +387,9 @@ describe('usingPrivileges', () => { const result = await usingPrivileges( Object.freeze({ navLinks: { - foo: true, - bar: true, - quz: true, + app1: true, + app2: true, + app3: true, }, management: { kibana: { @@ -389,11 +399,15 @@ describe('usingPrivileges', () => { }, }, catalogue: {}, - fooFeature: { + kibanaFeature1: { foo: true, bar: true, }, - barFeature: { + kibanaFeature2: { + foo: true, + bar: true, + }, + optOutFeature: { foo: true, bar: true, }, @@ -410,9 +424,9 @@ describe('usingPrivileges', () => { expect(result).toEqual({ navLinks: { - foo: true, - bar: false, - quz: true, + app1: true, + app2: false, + app3: true, }, management: { kibana: { @@ -422,14 +436,19 @@ describe('usingPrivileges', () => { }, }, catalogue: {}, - fooFeature: { + kibanaFeature1: { foo: true, bar: false, }, - barFeature: { + kibanaFeature2: { foo: true, bar: false, }, + optOutFeature: { + // these stay enabled because they opt out of Kibana security + foo: true, + bar: true, + }, esFeature: { es_manage: false, es_monitor: true, @@ -442,6 +461,7 @@ describe('usingPrivileges', () => { }); test(`doesn't re-enable disabled uiCapabilities`, async () => { + // grant all privileges const mockAuthz = createMockAuthz({ resolveCheckPrivileges: { privileges: { @@ -449,13 +469,19 @@ describe('usingPrivileges', () => { { privilege: actions.ui.get('navLinks', 'foo'), authorized: true }, { privilege: actions.ui.get('navLinks', 'bar'), authorized: true }, { privilege: actions.ui.get('management', 'kibana', 'indices'), authorized: true }, - { privilege: actions.ui.get('fooFeature', 'foo'), authorized: true }, - { privilege: actions.ui.get('fooFeature', 'bar'), authorized: true }, - { privilege: actions.ui.get('barFeature', 'foo'), authorized: true }, - { privilege: actions.ui.get('barFeature', 'bar'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature1', 'foo'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature1', 'bar'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature2', 'foo'), authorized: true }, + { privilege: actions.ui.get('kibanaFeature2', 'bar'), authorized: true }, + { privilege: actions.ui.get('optOutFeature', 'foo'), authorized: true }, + { privilege: actions.ui.get('optOutFeature', 'bar'), authorized: true }, ], elasticsearch: { - cluster: [], + cluster: [ + { privilege: 'manage', authorized: true }, + { privilege: 'monitor', authorized: true }, + { privilege: 'manage_security', authorized: true }, + ], index: {}, }, }, @@ -464,62 +490,14 @@ describe('usingPrivileges', () => { const { usingPrivileges } = disableUICapabilitiesFactory( mockRequest, - [ - new KibanaFeature({ - id: 'fooFeature', - name: 'Foo KibanaFeature', - app: [], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - new KibanaFeature({ - id: 'barFeature', - name: 'Bar KibanaFeature', - app: [], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - ], - [ - new ElasticsearchFeature({ - id: 'esFeature', - privileges: [ - { - requiredClusterPrivileges: [], - ui: [], - }, - ], - }), - ], + [kibanaFeature1, kibanaFeature2, optOutKibanaFeature], + esFeatures, loggingSystemMock.create().get(), mockAuthz, createMockUser() ); - const result = await usingPrivileges( - Object.freeze({ - navLinks: { - foo: false, - bar: false, - }, - management: { - kibana: { - indices: false, - }, - }, - catalogue: {}, - fooFeature: { - foo: false, - bar: false, - }, - barFeature: { - foo: false, - bar: false, - }, - }) - ); - - expect(result).toEqual({ + const allFalseCapabilities = Object.freeze({ navLinks: { foo: false, bar: false, @@ -530,36 +508,43 @@ describe('usingPrivileges', () => { }, }, catalogue: {}, - fooFeature: { + kibanaFeature1: { + foo: false, + bar: false, + }, + kibanaFeature2: { foo: false, bar: false, }, - barFeature: { + optOutFeature: { foo: false, bar: false, }, + esFeature: { + es_manage: false, + es_monitor: false, + }, + esSecurityFeature: { + es_manage_sec: false, + }, + esManagementFeature: {}, }); + const result = await usingPrivileges(allFalseCapabilities); + + expect(result).toEqual(allFalseCapabilities); }); }); describe('all', () => { - test(`disables uiCapabilities`, () => { + test(`disables only registered uiCapabilities that do not opt out of kibana security`, () => { const mockAuthz = createMockAuthz({ rejectCheckPrivileges: new Error(`Don't use me`) }); const { all } = disableUICapabilitiesFactory( mockRequest, - [ - new KibanaFeature({ - id: 'fooFeature', - name: 'Foo KibanaFeature', - app: ['foo'], - category: { id: 'foo', label: 'foo' }, - privileges: null, - }), - ], + [kibanaFeature1, optOutKibanaFeature], [ new ElasticsearchFeature({ - id: 'esFeature', + id: 'esFeature1', privileges: [ { requiredClusterPrivileges: [], @@ -576,8 +561,8 @@ describe('all', () => { const result = all( Object.freeze({ navLinks: { - foo: true, - bar: true, + app1: true, + app2: true, // there is no app2 registered }, management: { kibana: { @@ -585,41 +570,61 @@ describe('all', () => { }, }, catalogue: {}, - fooFeature: { + kibanaFeature1: { foo: true, bar: true, }, - barFeature: { + kibanaFeature2: { + // there is no kibanaFeature2 registered foo: true, bar: true, }, - esFeature: { + optOutFeature: { + foo: true, + bar: true, + }, + esFeature1: { + bar: true, + }, + esFeature2: { bar: true, }, }) ); expect(result).toEqual({ navLinks: { - foo: false, - bar: true, + app1: false, + app2: true, // does NOT disable because it is not a registered navlink }, management: { kibana: { - indices: false, + indices: false, // nested values are always disabled }, }, catalogue: {}, - fooFeature: { + kibanaFeature1: { + // registered kibana features with privileges get diabled foo: false, bar: false, }, - barFeature: { - foo: false, - bar: false, + kibanaFeature2: { + // does NOT disable because it is not a registered Kibana feature + foo: true, + bar: true, }, - esFeature: { + optOutFeature: { + // does NOT disable because it opts out (does not define privileges) + foo: true, + bar: true, + }, + esFeature1: { + // registered es features get diabled bar: false, }, + esFeature2: { + // does NOT disable because it is not a registered ES feature + bar: true, + }, }); }); }); diff --git a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts index 161366cb7309c..6023ea402ae56 100644 --- a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts @@ -67,20 +67,58 @@ export function disableUICapabilitiesFactory( }; }, {}); - const shouldDisableFeatureUICapability = ( - featureId: keyof UICapabilities, - uiCapability: string + const isCatalogueItemReferencedByFeatureSet = ( + catalogueEntry: string, + featureSet: Array | undefined }>> ) => { - // if the navLink isn't for a feature that we have registered, we don't wish to - // disable it based on privileges - return featureId !== 'navLinks' || featureNavLinkIds.includes(uiCapability); + return featureSet.some((feature) => (feature.catalogue ?? []).includes(catalogueEntry)); + }; + + const shouldAffectCapability = (featureId: keyof UICapabilities, uiCapability: string) => { + // This method answers: 'Should we affect a capability based on privileges?' + + // 'spaces' and 'fileUpload' feature ID's are handled independently + // The spaces and file_upload plugins have their own capabilites switchers + + // Always affect global settings + if (featureId === 'globalSettings') { + return true; + } + + // If the feature is 'catalogue', return true if it is the 'spaces' capability + // (we always want to affect that) or if we have a feature that references it + // (i.e. found in the 'catalogue' property of a registered Kibana or ES feature) + if (featureId === 'catalogue') { + return ( + uiCapability === 'spaces' || + isCatalogueItemReferencedByFeatureSet(uiCapability, features) || + isCatalogueItemReferencedByFeatureSet(uiCapability, elasticsearchFeatures) + ); + } + + // if the feature is 'navLinks', return true if the nav link was registered + // (i.e. found in the 'app' property of a registered Kibana feature) + if (featureId === 'navLinks') { + return featureNavLinkIds.includes(uiCapability); + } + + // if the feature is a Kibana feature, return true if it defines privileges + // (i.e. it adheres to the Kibana security model) + // Kibana features with no privileges opt out of the Kibana security model and + // are not subject to our control(e.g.Enterprise Search features) + const kibanaFeature = features.find((f) => f.id === featureId); + if (!!kibanaFeature) return !!kibanaFeature.privileges; + + // Lastly return true if the feature is a registered es feature (we always want to affect these), + // otherwise false(we don't know what this feature is so we don't touch it) + return !!elasticsearchFeatureMap[featureId]; }; const disableAll = (uiCapabilities: UICapabilities) => { return mapValues(uiCapabilities, (featureUICapabilities, featureId) => mapValues(featureUICapabilities, (value, uiCapability) => { if (typeof value === 'boolean') { - if (shouldDisableFeatureUICapability(featureId!, uiCapability!)) { + if (shouldAffectCapability(featureId!, uiCapability!)) { return false; } return value; @@ -175,7 +213,7 @@ export function disableUICapabilitiesFactory( ); // Catalogue and management capbility buckets can also be influenced by ES privileges, - // so the early return is not possible for these. + // so the early return is not possible for these *unless we have the required Kibana privileges. if ((!isCatalogueFeature && !isManagementFeature) || hasRequiredKibanaPrivileges) { return hasRequiredKibanaPrivileges; } @@ -230,7 +268,7 @@ export function disableUICapabilitiesFactory( featureUICapabilities, (value: boolean | Record, uiCapability) => { if (typeof value === 'boolean') { - if (!shouldDisableFeatureUICapability(featureId!, uiCapability!)) { + if (!shouldAffectCapability(featureId!, uiCapability!)) { return value; } return checkPrivilegesForCapability(value, featureId!, uiCapability!);