From 7fc6cad289d9c0557a2daefd3f96468f883a261a Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 19 Nov 2018 15:29:18 -0800 Subject: [PATCH 01/13] First attempt --- x-pack/plugins/ml/index.js | 11 +-- .../lib/authorization/check_privileges.ts | 81 ++++++++++++++----- .../check_privileges_dynamically.ts | 12 ++- .../authorization/disable_ui_capabilities.ts | 26 +++++- .../server/lib/authorization/types.ts | 5 +- .../lib/feature_registry/feature_registry.ts | 1 + 6 files changed, 97 insertions(+), 39 deletions(-) diff --git a/x-pack/plugins/ml/index.js b/x-pack/plugins/ml/index.js index bbbf9826bf5ab..d22e5f70f9b5b 100644 --- a/x-pack/plugins/ml/index.js +++ b/x-pack/plugins/ml/index.js @@ -68,16 +68,7 @@ export const ml = (kibana) => { name: 'Machine Learning', icon: 'mlApp', navLinkId: 'ml', - privileges: { - all: { - app: ['ml'], - savedObject: { - all: [], - read: ['config'] - }, - ui: [], - }, - } + clusterPrivilege: 'manage_ml' }); // Add server routes and initialize the plugin here diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges.ts b/x-pack/plugins/security/server/lib/authorization/check_privileges.ts index b7f1dd64fab02..690b03d8863e5 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.ts @@ -23,6 +23,9 @@ interface CheckPrivilegesAtResourcesResponse { [privilege: string]: boolean; }; }; + clusterPrivileges: { + [privilege: string]: boolean; + }; } export interface CheckPrivilegesAtResourceResponse { @@ -31,6 +34,9 @@ export interface CheckPrivilegesAtResourceResponse { privileges: { [privilege: string]: boolean; }; + clusterPrivileges: { + [privilege: string]: boolean; + }; } export interface CheckPrivilegesAtSpacesResponse { @@ -41,6 +47,9 @@ export interface CheckPrivilegesAtSpacesResponse { [privilege: string]: boolean; }; }; + clusterPrivileges: { + [privilege: string]: boolean; + }; } export type CheckPrivilegesWithRequest = (request: any) => CheckPrivileges; @@ -48,13 +57,18 @@ export type CheckPrivilegesWithRequest = (request: any) => CheckPrivileges; export interface CheckPrivileges { atSpace( spaceId: string, - privilegeOrPrivileges: string | string[] + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] ): Promise; atSpaces( spaceIds: string[], - privilegeOrPrivileges: string | string[] + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] ): Promise; - globally(privilegeOrPrivileges: string | string[]): Promise; + globally( + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] + ): Promise; } export function checkPrivilegesWithRequestFactory( @@ -75,7 +89,8 @@ export function checkPrivilegesWithRequestFactory( return function checkPrivilegesWithRequest(request: any): CheckPrivileges { const checkPrivilegesAtResources = async ( resources: string[], - privilegeOrPrivileges: string | string[] + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] ): Promise => { const privileges = Array.isArray(privilegeOrPrivileges) ? privilegeOrPrivileges @@ -87,6 +102,7 @@ export function checkPrivilegesWithRequestFactory( 'shield.hasPrivileges', { body: { + cluster: clusterPrivileges, applications: [ { application, @@ -120,48 +136,71 @@ export function checkPrivilegesWithRequestFactory( resourcePrivileges: transform(applicationPrivilegesResponse, (result, value, key) => { result[key!] = pick(value, privileges); }), + clusterPrivileges: hasPrivilegesResponse.cluster, }; }; const checkPrivilegesAtResource = async ( resource: string, - privilegeOrPrivileges: string | string[] - ) => { - const { hasAllRequested, username, resourcePrivileges } = await checkPrivilegesAtResources( + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] + ): Promise => { + const response = await checkPrivilegesAtResources( [resource], - privilegeOrPrivileges + privilegeOrPrivileges, + clusterPrivileges ); + return { - hasAllRequested, - username, - privileges: resourcePrivileges[resource], + hasAllRequested: response.hasAllRequested, + username: response.username, + privileges: response.resourcePrivileges[resource], + clusterPrivileges: response.clusterPrivileges, }; }; return { - async atSpace(spaceId: string, privilegeOrPrivileges: string | string[]) { + async atSpace( + spaceId: string, + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] + ) { const spaceResource = ResourceSerializer.serializeSpaceResource(spaceId); - return await checkPrivilegesAtResource(spaceResource, privilegeOrPrivileges); + return await checkPrivilegesAtResource( + spaceResource, + privilegeOrPrivileges, + clusterPrivileges + ); }, - async atSpaces(spaceIds: string[], privilegeOrPrivileges: string | string[]) { + async atSpaces( + spaceIds: string[], + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] + ) { const spaceResources = spaceIds.map(spaceId => ResourceSerializer.serializeSpaceResource(spaceId) ); - const { hasAllRequested, username, resourcePrivileges } = await checkPrivilegesAtResources( + const response = await checkPrivilegesAtResources( spaceResources, - privilegeOrPrivileges + privilegeOrPrivileges, + clusterPrivileges ); return { - hasAllRequested, - username, + hasAllRequested: response.hasAllRequested, + username: response.username, // we need to turn the resource responses back into the space ids - spacePrivileges: transform(resourcePrivileges, (result, value, key) => { + spacePrivileges: transform(response.resourcePrivileges, (result, value, key) => { result[ResourceSerializer.deserializeSpaceResource(key!)] = value; }), + clusterPrivileges: response.clusterPrivileges, }; }, - async globally(privilegeOrPrivileges: string | string[]) { - return await checkPrivilegesAtResource(GLOBAL_RESOURCE, privilegeOrPrivileges); + async globally(privilegeOrPrivileges: string | string[], clusterPrivileges?: string[]) { + return await checkPrivilegesAtResource( + GLOBAL_RESOURCE, + privilegeOrPrivileges, + clusterPrivileges + ); }, }; }; diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.ts b/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.ts index 02e25dc3e6862..f25eee151e800 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.ts +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.ts @@ -13,7 +13,8 @@ import { CheckPrivilegesAtResourceResponse, CheckPrivilegesWithRequest } from '. */ export type CheckPrivilegesDynamically = ( - privilegeOrPrivileges: string | string[] + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] ) => Promise; export type CheckPrivilegesDynamicallyWithRequest = ( @@ -26,12 +27,15 @@ export function checkPrivilegesDynamicallyWithRequestFactory( ): CheckPrivilegesDynamicallyWithRequest { return function checkPrivilegesDynamicallyWithRequest(request: Record) { const checkPrivileges = checkPrivilegesWithRequest(request); - return async function checkPrivilegesDynamically(privilegeOrPrivileges: string | string[]) { + return async function checkPrivilegesDynamically( + privilegeOrPrivileges: string | string[], + clusterPrivileges?: string[] + ) { if (spaces.isEnabled) { const spaceId = spaces.getSpaceId(request); - return await checkPrivileges.atSpace(spaceId, privilegeOrPrivileges); + return await checkPrivileges.atSpace(spaceId, privilegeOrPrivileges, clusterPrivileges); } else { - return await checkPrivileges.globally(privilegeOrPrivileges); + return await checkPrivileges.globally(privilegeOrPrivileges, clusterPrivileges); } }; }; diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts index 972c03446939c..b0074ef59b589 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts @@ -4,8 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { mapValues } from 'lodash'; +import { mapValues, uniq } from 'lodash'; import { UICapabilities } from 'ui/capabilities'; +import { Feature } from '../../../../xpack_main/types'; import { Actions } from './actions'; import { CheckPrivilegesAtResourceResponse } from './check_privileges'; import { CheckPrivilegesDynamically } from './check_privileges_dynamically'; @@ -24,6 +25,11 @@ export function disableUICapabilitesFactory( }; const usingPrivileges = async (uiCapabilities: UICapabilities) => { + const features: Feature[] = server.plugins.xpack_main.getFeatures(); + const clusterPrivileges = features.reduce( + (acc, feature) => [...acc, ...(feature.clusterPrivilege ? [feature.clusterPrivilege] : [])], + [] + ); const uiActions = Object.entries(uiCapabilities).reduce( (acc, [featureId, featureUICapabilities]) => [ ...acc, @@ -39,7 +45,7 @@ export function disableUICapabilitesFactory( const checkPrivilegesDynamically: CheckPrivilegesDynamically = authorization.checkPrivilegesDynamicallyWithRequest( request ); - checkPrivilegesResponse = await checkPrivilegesDynamically(uiActions); + checkPrivilegesResponse = await checkPrivilegesDynamically(uiActions, clusterPrivileges); } catch (err) { // if we get a 401/403, then we want to disable all uiCapabilities, as this // is generally when the user hasn't authenticated yet and we're displaying the @@ -50,6 +56,17 @@ export function disableUICapabilitesFactory( throw err; } + const clusterDisabledFeatures = features + .filter( + feature => + feature.clusterPrivilege && + feature.navLinkId && + checkPrivilegesResponse.clusterPrivileges[feature.clusterPrivilege!] === false + ) + .reduce((acc, feature) => { + return [...acc, actions.ui.get('navlink', feature.navLinkId!)]; + }, []); + return mapValues(uiCapabilities, (featureUICapabilities, featureId) => { return mapValues(featureUICapabilities, (enabled, uiCapability) => { // if the uiCapability has already been disabled, we don't want to re-enable it @@ -58,7 +75,10 @@ export function disableUICapabilitesFactory( } const action = actions.ui.get(featureId!, uiCapability!); - return checkPrivilegesResponse.privileges[action] === true; + return ( + checkPrivilegesResponse.privileges[action] === true && + !clusterDisabledFeatures.includes(action) + ); }); }); }; diff --git a/x-pack/plugins/security/server/lib/authorization/types.ts b/x-pack/plugins/security/server/lib/authorization/types.ts index 75188d1191b1a..323e49f47993e 100644 --- a/x-pack/plugins/security/server/lib/authorization/types.ts +++ b/x-pack/plugins/security/server/lib/authorization/types.ts @@ -6,13 +6,16 @@ export interface HasPrivilegesResponseApplication { [resource: string]: { - [privilegeName: string]: boolean; + [privilege: string]: boolean; }; } export interface HasPrivilegesResponse { has_all_requested: boolean; username: string; + cluster: { + [privilege: string]: boolean; + }; application: { [applicationName: string]: HasPrivilegesResponseApplication; }; diff --git a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts index 49a19a9329ef8..e8c946ad44452 100644 --- a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts +++ b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts @@ -24,6 +24,7 @@ export interface Feature { icon?: IconType; description?: string; navLinkId?: string; + clusterPrivilege?: string; privileges: { [key: string]: FeaturePrivilegeDefinition; }; From da26471aa94747c00a18e1e631a44d511adea7a1 Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 19 Nov 2018 16:06:37 -0800 Subject: [PATCH 02/13] Using either application or derived cluster privileges --- .../authorization/disable_ui_capabilities.ts | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts index b0074ef59b589..2a55503d30523 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts @@ -56,16 +56,15 @@ export function disableUICapabilitesFactory( throw err; } - const clusterDisabledFeatures = features - .filter( - feature => - feature.clusterPrivilege && - feature.navLinkId && - checkPrivilegesResponse.clusterPrivileges[feature.clusterPrivilege!] === false - ) - .reduce((acc, feature) => { - return [...acc, actions.ui.get('navlink', feature.navLinkId!)]; - }, []); + const clusterBasedFeatures = features + .filter(feature => feature.clusterPrivilege && feature.navLinkId) + .reduce>((acc, feature) => { + return { + ...acc, + [actions.ui.get('navLinks', feature.navLinkId!)]: + checkPrivilegesResponse.clusterPrivileges[feature.clusterPrivilege!] === true, + }; + }, {}); return mapValues(uiCapabilities, (featureUICapabilities, featureId) => { return mapValues(featureUICapabilities, (enabled, uiCapability) => { @@ -76,8 +75,8 @@ export function disableUICapabilitesFactory( const action = actions.ui.get(featureId!, uiCapability!); return ( - checkPrivilegesResponse.privileges[action] === true && - !clusterDisabledFeatures.includes(action) + checkPrivilegesResponse.privileges[action] === true || + clusterBasedFeatures[action] === true ); }); }); From 0307787041db9be5757f6f012e81af0e19f1e265 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 20 Nov 2018 11:13:20 -0800 Subject: [PATCH 03/13] Restructruing the manner in which cluster privileges are specified --- x-pack/plugins/apm/index.js | 16 +- x-pack/plugins/canvas/init.js | 28 ++-- x-pack/plugins/graph/index.js | 30 ++-- x-pack/plugins/ml/index.js | 4 +- x-pack/plugins/monitoring/init.js | 16 +- .../authorization/disable_ui_capabilities.ts | 25 ++- .../features_privileges_builder.ts | 24 ++- .../lib/feature_registry/feature_registry.ts | 39 +++-- .../server/lib/feature_registry/index.ts | 10 +- .../server/lib/register_oss_features.ts | 158 ++++++++++-------- x-pack/plugins/xpack_main/types.ts | 8 +- 11 files changed, 215 insertions(+), 143 deletions(-) diff --git a/x-pack/plugins/apm/index.js b/x-pack/plugins/apm/index.js index 7b4093306becd..5b9701a31b622 100644 --- a/x-pack/plugins/apm/index.js +++ b/x-pack/plugins/apm/index.js @@ -61,13 +61,15 @@ export function apm(kibana) { icon: 'apmApp', navLinkId: 'apm', privileges: { - all: { - app: ['apm'], - savedObject: { - all: [], - read: ['config'] - }, - ui: [] + kibana: { + all: { + app: ['apm'], + savedObject: { + all: [], + read: ['config'] + }, + ui: [] + } } } }); diff --git a/x-pack/plugins/canvas/init.js b/x-pack/plugins/canvas/init.js index f4b60ba4b6560..829d355bdc565 100644 --- a/x-pack/plugins/canvas/init.js +++ b/x-pack/plugins/canvas/init.js @@ -33,21 +33,23 @@ export default async function(server /*options*/) { icon: 'canvasApp', navLinkId: 'canvas', privileges: { - all: { - app: ['canvas'], - savedObject: { - all: ['canvas'], - read: ['config', 'index-pattern'], + kibana: { + all: { + app: ['canvas'], + savedObject: { + all: ['canvas'], + read: ['config', 'index-pattern'], + }, + ui: [], }, - ui: [], - }, - read: { - app: ['canvas'], - savedObject: { - all: [], - read: ['config', 'index-pattern', 'canvas'], + read: { + app: ['canvas'], + savedObject: { + all: [], + read: ['config', 'index-pattern', 'canvas'], + }, + ui: [], }, - ui: [], }, }, }); diff --git a/x-pack/plugins/graph/index.js b/x-pack/plugins/graph/index.js index 959ded232ec11..7cd3f9e9aae47 100644 --- a/x-pack/plugins/graph/index.js +++ b/x-pack/plugins/graph/index.js @@ -56,21 +56,23 @@ export function graph(kibana) { icon: 'graphApp', navLinkId: 'graph', privileges: { - all: { - app: ['graph'], - savedObject: { - all: ['graph-workspace'], - read: ['config', 'index-pattern'], + kibana: { + all: { + app: ['graph'], + savedObject: { + all: ['graph-workspace'], + read: ['config', 'index-pattern'], + }, + ui: [], }, - ui: [], - }, - read: { - app: ['graph'], - savedObject: { - all: [], - read: ['config', 'index-pattern', 'graph-workspace'], - }, - ui: [], + read: { + app: ['graph'], + savedObject: { + all: [], + read: ['config', 'index-pattern', 'graph-workspace'], + }, + ui: [], + } } } }); diff --git a/x-pack/plugins/ml/index.js b/x-pack/plugins/ml/index.js index d22e5f70f9b5b..2e7467b5ca048 100644 --- a/x-pack/plugins/ml/index.js +++ b/x-pack/plugins/ml/index.js @@ -68,7 +68,9 @@ export const ml = (kibana) => { name: 'Machine Learning', icon: 'mlApp', navLinkId: 'ml', - clusterPrivilege: 'manage_ml' + privileges: { + cluster: ['manage_ml'] + } }); // Add server routes and initialize the plugin here diff --git a/x-pack/plugins/monitoring/init.js b/x-pack/plugins/monitoring/init.js index 4027ec3e804b6..934835fc14e30 100644 --- a/x-pack/plugins/monitoring/init.js +++ b/x-pack/plugins/monitoring/init.js @@ -60,14 +60,16 @@ export const init = (monitoringPlugin, server) => { icon: 'monitoringApp', navLinkId: 'monitoring', privileges: { - all: { - app: ['monitoring'], - savedObject: { - all: [], - read: ['config'], + kibana: { + all: { + app: ['monitoring'], + savedObject: { + all: [], + read: ['config'], + }, + ui: [], }, - ui: [], - }, + } } }); diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts index 2a55503d30523..688cf34db0d9d 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { mapValues, uniq } from 'lodash'; +import { mapValues } from 'lodash'; import { UICapabilities } from 'ui/capabilities'; -import { Feature } from '../../../../xpack_main/types'; +import { Feature, isFeaturePrivilegesCluster } from '../../../../xpack_main/types'; import { Actions } from './actions'; import { CheckPrivilegesAtResourceResponse } from './check_privileges'; import { CheckPrivilegesDynamically } from './check_privileges_dynamically'; @@ -26,10 +26,11 @@ export function disableUICapabilitesFactory( const usingPrivileges = async (uiCapabilities: UICapabilities) => { const features: Feature[] = server.plugins.xpack_main.getFeatures(); - const clusterPrivileges = features.reduce( - (acc, feature) => [...acc, ...(feature.clusterPrivilege ? [feature.clusterPrivilege] : [])], - [] - ); + const clusterPrivileges = features + .map(feature => feature.privileges) + .filter(isFeaturePrivilegesCluster) + .reduce((acc, privileges) => [...acc, ...privileges.cluster], []); + const uiActions = Object.entries(uiCapabilities).reduce( (acc, [featureId, featureUICapabilities]) => [ ...acc, @@ -57,12 +58,18 @@ export function disableUICapabilitesFactory( } const clusterBasedFeatures = features - .filter(feature => feature.clusterPrivilege && feature.navLinkId) + .filter(feature => feature.navLinkId) .reduce>((acc, feature) => { + if (!isFeaturePrivilegesCluster(feature.privileges)) { + return acc; + } + + const enabled = feature.privileges.cluster.every( + clusterPrivilege => checkPrivilegesResponse.clusterPrivileges[clusterPrivilege] === true + ); return { ...acc, - [actions.ui.get('navLinks', feature.navLinkId!)]: - checkPrivilegesResponse.clusterPrivileges[feature.clusterPrivilege!] === true, + [actions.ui.get('navLinks', feature.navLinkId!)]: enabled, }; }, {}); diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts index c17ca8d9d9320..2611690084ec3 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts @@ -5,7 +5,7 @@ */ import { Dictionary, flatten, mapValues } from 'lodash'; -import { Feature } from '../../../../xpack_main/types'; +import { Feature, isFeaturePrivilegesKibana } from '../../../../xpack_main/types'; import { Actions } from './actions'; export type FeaturesPrivileges = Record>; @@ -28,11 +28,15 @@ export class FeaturesPrivilegesBuilder { return flatten( features.map(feature => { const { privileges } = feature; - if (!privileges || !privileges.read || !privileges.read.api) { + if (!privileges || !isFeaturePrivilegesKibana(privileges)) { return []; } - return feature.privileges.read.api!.map(api => this.actions.api.get(api)); + if (!privileges.kibana.read || !privileges.kibana.read.api) { + return []; + } + + return privileges.kibana.read.api!.map(api => this.actions.api.get(api)); }) ); } @@ -41,11 +45,15 @@ export class FeaturesPrivilegesBuilder { return flatten( features.map(feature => { const { privileges } = feature; - if (!privileges || !privileges.read || !privileges.read.ui) { + if (!privileges || !isFeaturePrivilegesKibana(privileges)) { return []; } - return feature.privileges.read.ui!.map(uiCapability => + if (!privileges.kibana.read || !privileges.kibana.ui) { + return []; + } + + return privileges.kibana.read.ui!.map(uiCapability => this.actions.ui.get(feature.id, uiCapability) ); }) @@ -53,7 +61,11 @@ export class FeaturesPrivilegesBuilder { } private buildFeaturePrivileges(feature: Feature): Dictionary { - return mapValues(feature.privileges, privilegeDefinition => [ + if (!isFeaturePrivilegesKibana(feature.privileges)) { + return {}; + } + + return mapValues(feature.privileges.kibana!, privilegeDefinition => [ this.actions.login, this.actions.version, ...(privilegeDefinition.api diff --git a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts index e8c946ad44452..dfbe1e46689da 100644 --- a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts +++ b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts @@ -7,14 +7,34 @@ import { IconType } from '@elastic/eui'; import _ from 'lodash'; -export interface FeaturePrivilegeDefinition { - api?: string[]; - app: string[]; - savedObject: { - all: string[]; - read: string[]; +export function isFeaturePrivilegesKibana( + privileges: FeaturePrivilegesKibana | FeaturePrivilegesCluster +): privileges is FeaturePrivilegesKibana { + return (privileges as FeaturePrivilegesKibana).kibana !== undefined; +} + +export function isFeaturePrivilegesCluster( + privileges: FeaturePrivilegesKibana | FeaturePrivilegesCluster +): privileges is FeaturePrivilegesCluster { + return (privileges as FeaturePrivilegesCluster).cluster !== undefined; +} + +export interface FeaturePrivilegesKibana { + kibana: { + [key: string]: { + api?: string[]; + app: string[]; + savedObject: { + all: string[]; + read: string[]; + }; + ui: string[]; + }; }; - ui: string[]; +} + +export interface FeaturePrivilegesCluster { + cluster: string[]; } export interface Feature { @@ -24,10 +44,7 @@ export interface Feature { icon?: IconType; description?: string; navLinkId?: string; - clusterPrivilege?: string; - privileges: { - [key: string]: FeaturePrivilegeDefinition; - }; + privileges: FeaturePrivilegesKibana | FeaturePrivilegesCluster; } const features: Record = {}; diff --git a/x-pack/plugins/xpack_main/server/lib/feature_registry/index.ts b/x-pack/plugins/xpack_main/server/lib/feature_registry/index.ts index 2456ac0516ef2..b1ef3a4d90240 100644 --- a/x-pack/plugins/xpack_main/server/lib/feature_registry/index.ts +++ b/x-pack/plugins/xpack_main/server/lib/feature_registry/index.ts @@ -4,4 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -export { Feature, registerFeature, getFeatures } from './feature_registry'; +export { + Feature, + FeaturePrivilegesCluster, + FeaturePrivilegesKibana, + registerFeature, + getFeatures, + isFeaturePrivilegesCluster, + isFeaturePrivilegesKibana, +} from './feature_registry'; diff --git a/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts b/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts index 4f7ecfbf901bf..62aeb7486bca2 100644 --- a/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts +++ b/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts @@ -13,21 +13,23 @@ const kibanaFeatures: Feature[] = [ icon: 'discoverApp', navLinkId: 'kibana:discover', privileges: { - all: { - app: ['kibana'], - savedObject: { - all: ['search'], - read: ['config', 'index-pattern'], + kibana: { + all: { + app: ['kibana'], + savedObject: { + all: ['search'], + read: ['config', 'index-pattern'], + }, + ui: ['kibana:discover'], }, - ui: ['kibana:discover'], - }, - read: { - app: ['kibana'], - savedObject: { - all: [], - read: ['config', 'index-pattern', 'search'], + read: { + app: ['kibana'], + savedObject: { + all: [], + read: ['config', 'index-pattern', 'search'], + }, + ui: [], }, - ui: [], }, }, }, @@ -37,21 +39,23 @@ const kibanaFeatures: Feature[] = [ icon: 'visualizeApp', navLinkId: 'kibana:visualize', privileges: { - all: { - app: ['kibana'], - savedObject: { - all: ['visualization'], - read: ['config', 'index-pattern', 'search'], + kibana: { + all: { + app: ['kibana'], + savedObject: { + all: ['visualization'], + read: ['config', 'index-pattern', 'search'], + }, + ui: ['kibana:visualize'], }, - ui: ['kibana:visualize'], - }, - read: { - app: ['kibana'], - savedObject: { - all: [], - read: ['config', 'index-pattern', 'search', 'visualization'], + read: { + app: ['kibana'], + savedObject: { + all: [], + read: ['config', 'index-pattern', 'search', 'visualization'], + }, + ui: [], }, - ui: [], }, }, }, @@ -61,29 +65,31 @@ const kibanaFeatures: Feature[] = [ icon: 'dashboardApp', navLinkId: 'kibana:dashboard', privileges: { - all: { - app: ['kibana'], - savedObject: { - all: ['dashboard'], - read: ['config', 'index-pattern', 'search', 'visualization', 'timelion', 'canvas'], + kibana: { + all: { + app: ['kibana'], + savedObject: { + all: ['dashboard'], + read: ['config', 'index-pattern', 'search', 'visualization', 'timelion', 'canvas'], + }, + ui: ['kibana:dashboard'], }, - ui: ['kibana:dashboard'], - }, - read: { - app: ['kibana'], - savedObject: { - all: [], - read: [ - 'config', - 'index-pattern', - 'search', - 'visualization', - 'timelion', - 'canvas', - 'dashboard', - ], + read: { + app: ['kibana'], + savedObject: { + all: [], + read: [ + 'config', + 'index-pattern', + 'search', + 'visualization', + 'timelion', + 'canvas', + 'dashboard', + ], + }, + ui: [], }, - ui: [], }, }, }, @@ -93,14 +99,16 @@ const kibanaFeatures: Feature[] = [ icon: 'devToolsApp', navLinkId: 'kibana:dev_tools', privileges: { - all: { - api: ['console/execute'], - app: ['kibana'], - savedObject: { - all: [], - read: ['config'], + kibana: { + all: { + api: ['console/execute'], + app: ['kibana'], + savedObject: { + all: [], + read: ['config'], + }, + ui: [], }, - ui: [], }, }, }, @@ -110,13 +118,15 @@ const kibanaFeatures: Feature[] = [ icon: 'managementApp', navLinkId: 'kibana:management:advancedSettings', privileges: { - all: { - app: ['kibana'], - savedObject: { - all: ['config'], - read: [], + kibana: { + all: { + app: ['kibana'], + savedObject: { + all: ['config'], + read: [], + }, + ui: [], }, - ui: [], }, }, }, @@ -129,21 +139,23 @@ const timelionFeatures: Feature[] = [ icon: 'timelionApp', navLinkId: 'timelion', privileges: { - all: { - app: ['timelion'], - savedObject: { - all: ['timelion'], - read: ['config', 'index-pattern'], + kibana: { + all: { + app: ['timelion'], + savedObject: { + all: ['timelion'], + read: ['config', 'index-pattern'], + }, + ui: [], }, - ui: [], - }, - read: { - app: ['timelion'], - savedObject: { - all: [], - read: ['config', 'index-pattern', 'timelion'], + read: { + app: ['timelion'], + savedObject: { + all: [], + read: ['config', 'index-pattern', 'timelion'], + }, + ui: [], }, - ui: [], }, }, }, diff --git a/x-pack/plugins/xpack_main/types.ts b/x-pack/plugins/xpack_main/types.ts index b6077c3d08698..5871b412f5438 100644 --- a/x-pack/plugins/xpack_main/types.ts +++ b/x-pack/plugins/xpack_main/types.ts @@ -4,4 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -export { Feature } from './server/lib/feature_registry'; +export { + Feature, + FeaturePrivilegesCluster, + FeaturePrivilegesKibana, + isFeaturePrivilegesKibana, + isFeaturePrivilegesCluster, +} from './server/lib/feature_registry'; From 036d12bf6302b7b63fa806cef7007b43d9d43d8b Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 20 Nov 2018 13:15:50 -0800 Subject: [PATCH 04/13] A bit of cleanup --- .../authorization/disable_ui_capabilities.ts | 34 ++++--------- .../features_cluster_privileges.ts | 50 +++++++++++++++++++ 2 files changed, 60 insertions(+), 24 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts index 688cf34db0d9d..a8dce1ea9b1ec 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts @@ -6,10 +6,11 @@ import { mapValues } from 'lodash'; import { UICapabilities } from 'ui/capabilities'; -import { Feature, isFeaturePrivilegesCluster } from '../../../../xpack_main/types'; +import { Feature } from '../../../../xpack_main/types'; import { Actions } from './actions'; import { CheckPrivilegesAtResourceResponse } from './check_privileges'; import { CheckPrivilegesDynamically } from './check_privileges_dynamically'; +import { FeaturesClusterPrivileges } from './features_cluster_privileges'; export function disableUICapabilitesFactory( server: Record, @@ -25,12 +26,6 @@ export function disableUICapabilitesFactory( }; const usingPrivileges = async (uiCapabilities: UICapabilities) => { - const features: Feature[] = server.plugins.xpack_main.getFeatures(); - const clusterPrivileges = features - .map(feature => feature.privileges) - .filter(isFeaturePrivilegesCluster) - .reduce((acc, privileges) => [...acc, ...privileges.cluster], []); - const uiActions = Object.entries(uiCapabilities).reduce( (acc, [featureId, featureUICapabilities]) => [ ...acc, @@ -41,6 +36,10 @@ export function disableUICapabilitesFactory( [] ); + const features: Feature[] = server.plugins.xpack_main.getFeatures(); + const featuresClusterPrivileges = new FeaturesClusterPrivileges(features, actions); + const clusterPrivileges = featuresClusterPrivileges.getAllClusterPrivileges(); + let checkPrivilegesResponse: CheckPrivilegesAtResourceResponse; try { const checkPrivilegesDynamically: CheckPrivilegesDynamically = authorization.checkPrivilegesDynamicallyWithRequest( @@ -57,22 +56,6 @@ export function disableUICapabilitesFactory( throw err; } - const clusterBasedFeatures = features - .filter(feature => feature.navLinkId) - .reduce>((acc, feature) => { - if (!isFeaturePrivilegesCluster(feature.privileges)) { - return acc; - } - - const enabled = feature.privileges.cluster.every( - clusterPrivilege => checkPrivilegesResponse.clusterPrivileges[clusterPrivilege] === true - ); - return { - ...acc, - [actions.ui.get('navLinks', feature.navLinkId!)]: enabled, - }; - }, {}); - return mapValues(uiCapabilities, (featureUICapabilities, featureId) => { return mapValues(featureUICapabilities, (enabled, uiCapability) => { // if the uiCapability has already been disabled, we don't want to re-enable it @@ -83,7 +66,10 @@ export function disableUICapabilitesFactory( const action = actions.ui.get(featureId!, uiCapability!); return ( checkPrivilegesResponse.privileges[action] === true || - clusterBasedFeatures[action] === true + featuresClusterPrivileges.isActionEnabled( + action, + checkPrivilegesResponse.clusterPrivileges + ) ); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts b/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts new file mode 100644 index 0000000000000..9d5fedac176d7 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Feature, isFeaturePrivilegesCluster } from '../../../../xpack_main/types'; +import { Actions } from './actions'; + +export class FeaturesClusterPrivileges { + private allClusterPrivileges: string[]; + private actionToClusterPrivileges: Record; + + constructor(features: Feature[], actions: Actions) { + this.allClusterPrivileges = features + .map(feature => feature.privileges) + .filter(isFeaturePrivilegesCluster) + .reduce((acc, privileges) => [...acc, ...privileges.cluster], []); + + this.actionToClusterPrivileges = features + .filter(feature => feature.navLinkId) + .reduce>((acc, feature) => { + if (!isFeaturePrivilegesCluster(feature.privileges)) { + return acc; + } + + const action = actions.ui.get('navLinks', feature.navLinkId!); + acc[action] = [...(acc[action] || []), ...feature.privileges.cluster]; + return acc; + }, {}); + } + + public getAllClusterPrivileges() { + return this.allClusterPrivileges; + } + + public isActionEnabled( + action: string, + checkPrivilegesResponseClusterPrivileges: Record + ) { + const clusterPrivileges = this.actionToClusterPrivileges[action]; + if (!clusterPrivileges || clusterPrivileges.length === 0) { + return false; + } + + return clusterPrivileges.every( + clusterPrivilege => checkPrivilegesResponseClusterPrivileges[clusterPrivilege] === true + ); + } +} From 706612acad4e402a81882c2d457b6c36e46b6868 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 20 Nov 2018 14:32:07 -0800 Subject: [PATCH 05/13] Specifying navLinks explicitly when registering features --- x-pack/plugins/apm/index.js | 4 +- x-pack/plugins/canvas/init.js | 8 +++- x-pack/plugins/graph/index.js | 8 +++- x-pack/plugins/ml/index.js | 13 +++++- x-pack/plugins/monitoring/init.js | 4 +- .../features_cluster_privileges.ts | 18 +++++++-- .../features_privileges_builder.ts | 29 ++++++++++---- .../lib/feature_registry/feature_registry.ts | 16 ++++++-- .../server/lib/register_oss_features.ts | 40 ++++++++++++++----- 9 files changed, 110 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/apm/index.js b/x-pack/plugins/apm/index.js index 5b9701a31b622..0d652612c16fc 100644 --- a/x-pack/plugins/apm/index.js +++ b/x-pack/plugins/apm/index.js @@ -68,7 +68,9 @@ export function apm(kibana) { all: [], read: ['config'] }, - ui: [] + ui: { + navLink: true + } } } } diff --git a/x-pack/plugins/canvas/init.js b/x-pack/plugins/canvas/init.js index 829d355bdc565..02519980af62c 100644 --- a/x-pack/plugins/canvas/init.js +++ b/x-pack/plugins/canvas/init.js @@ -40,7 +40,9 @@ export default async function(server /*options*/) { all: ['canvas'], read: ['config', 'index-pattern'], }, - ui: [], + ui: { + navLink: true, + }, }, read: { app: ['canvas'], @@ -48,7 +50,9 @@ export default async function(server /*options*/) { all: [], read: ['config', 'index-pattern', 'canvas'], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, diff --git a/x-pack/plugins/graph/index.js b/x-pack/plugins/graph/index.js index 7cd3f9e9aae47..f323c826d06a3 100644 --- a/x-pack/plugins/graph/index.js +++ b/x-pack/plugins/graph/index.js @@ -63,7 +63,9 @@ export function graph(kibana) { all: ['graph-workspace'], read: ['config', 'index-pattern'], }, - ui: [], + ui: { + navLink: true, + }, }, read: { app: ['graph'], @@ -71,7 +73,9 @@ export function graph(kibana) { all: [], read: ['config', 'index-pattern', 'graph-workspace'], }, - ui: [], + ui: { + navLink: true, + }, } } } diff --git a/x-pack/plugins/ml/index.js b/x-pack/plugins/ml/index.js index 2e7467b5ca048..bc419f1270e72 100644 --- a/x-pack/plugins/ml/index.js +++ b/x-pack/plugins/ml/index.js @@ -69,7 +69,18 @@ export const ml = (kibana) => { icon: 'mlApp', navLinkId: 'ml', privileges: { - cluster: ['manage_ml'] + cluster: { + manage_ml: { + ui: { + navLink: true, + }, + }, + manage_ccr: { + ui: { + navLink: true, + }, + } + } } }); diff --git a/x-pack/plugins/monitoring/init.js b/x-pack/plugins/monitoring/init.js index 934835fc14e30..a0e2a2c2f9f47 100644 --- a/x-pack/plugins/monitoring/init.js +++ b/x-pack/plugins/monitoring/init.js @@ -67,7 +67,9 @@ export const init = (monitoringPlugin, server) => { all: [], read: ['config'], }, - ui: [], + ui: { + navLink: true, + }, }, } } diff --git a/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts b/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts index 9d5fedac176d7..bf2d01b1983c0 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts @@ -15,7 +15,7 @@ export class FeaturesClusterPrivileges { this.allClusterPrivileges = features .map(feature => feature.privileges) .filter(isFeaturePrivilegesCluster) - .reduce((acc, privileges) => [...acc, ...privileges.cluster], []); + .reduce((acc, privileges) => [...acc, ...Object.keys(privileges.cluster)], []); this.actionToClusterPrivileges = features .filter(feature => feature.navLinkId) @@ -24,8 +24,20 @@ export class FeaturesClusterPrivileges { return acc; } - const action = actions.ui.get('navLinks', feature.navLinkId!); - acc[action] = [...(acc[action] || []), ...feature.privileges.cluster]; + for (const [clusterPrivilege, definition] of Object.entries(feature.privileges.cluster)) { + if (definition.ui.navLink) { + const action = actions.ui.get('navLinks', feature.navLinkId!); + acc[action] = [...(acc[action] || []), clusterPrivilege]; + } + + if (definition.ui.capability) { + for (const capability of definition.ui.capability) { + const action = actions.ui.get(feature.id, capability); + acc[action] = [...(acc[action] || []), clusterPrivilege]; + } + } + } + return acc; }, {}); } diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts index 2611690084ec3..7fcc1ac15d885 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts @@ -36,7 +36,9 @@ export class FeaturesPrivilegesBuilder { return []; } - return privileges.kibana.read.api!.map(api => this.actions.api.get(api)); + const { api } = privileges.kibana.read; + + return api!.map(operation => this.actions.api.get(operation)); }) ); } @@ -49,13 +51,20 @@ export class FeaturesPrivilegesBuilder { return []; } - if (!privileges.kibana.read || !privileges.kibana.ui) { + if (!privileges.kibana.read || !privileges.kibana.read.ui) { return []; } - return privileges.kibana.read.ui!.map(uiCapability => - this.actions.ui.get(feature.id, uiCapability) - ); + const { ui } = privileges.kibana.read; + + return [ + ...(ui.capability + ? ui.capability.map(uiCapability => this.actions.ui.get(feature.id, uiCapability)) + : []), + ...(feature.navLinkId && ui.navLink + ? [this.actions.ui.get('navLinks', feature.navLinkId)] + : []), + ]; }) ); } @@ -82,8 +91,14 @@ export class FeaturesPrivilegesBuilder { this.actions.savedObject.readOperations(types) ) ), - ...privilegeDefinition.ui.map(ui => this.actions.ui.get(feature.id, ui)), - ...(feature.navLinkId ? [this.actions.ui.get('navLinks', feature.navLinkId)] : []), + ...(privilegeDefinition.ui.capability + ? privilegeDefinition.ui.capability.map(uiCapability => + this.actions.ui.get(feature.id, uiCapability) + ) + : []), + ...(feature.navLinkId && privilegeDefinition.ui.navLink + ? [this.actions.ui.get('navLinks', feature.navLinkId)] + : []), ]); } } diff --git a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts index dfbe1e46689da..af697abd8640a 100644 --- a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts +++ b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts @@ -21,20 +21,30 @@ export function isFeaturePrivilegesCluster( export interface FeaturePrivilegesKibana { kibana: { - [key: string]: { + [feature: string]: { api?: string[]; app: string[]; savedObject: { all: string[]; read: string[]; }; - ui: string[]; + ui: { + navLink?: boolean; + capability?: string[]; + }; }; }; } export interface FeaturePrivilegesCluster { - cluster: string[]; + cluster: { + [clusterPrivilege: string]: { + ui: { + navLink?: boolean; + capability: string[]; + }; + }; + }; } export interface Feature { diff --git a/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts b/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts index 62aeb7486bca2..f12862b15e88b 100644 --- a/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts +++ b/x-pack/plugins/xpack_main/server/lib/register_oss_features.ts @@ -20,7 +20,9 @@ const kibanaFeatures: Feature[] = [ all: ['search'], read: ['config', 'index-pattern'], }, - ui: ['kibana:discover'], + ui: { + navLink: true, + }, }, read: { app: ['kibana'], @@ -28,7 +30,9 @@ const kibanaFeatures: Feature[] = [ all: [], read: ['config', 'index-pattern', 'search'], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, @@ -46,7 +50,9 @@ const kibanaFeatures: Feature[] = [ all: ['visualization'], read: ['config', 'index-pattern', 'search'], }, - ui: ['kibana:visualize'], + ui: { + navLink: true, + }, }, read: { app: ['kibana'], @@ -54,7 +60,9 @@ const kibanaFeatures: Feature[] = [ all: [], read: ['config', 'index-pattern', 'search', 'visualization'], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, @@ -72,7 +80,9 @@ const kibanaFeatures: Feature[] = [ all: ['dashboard'], read: ['config', 'index-pattern', 'search', 'visualization', 'timelion', 'canvas'], }, - ui: ['kibana:dashboard'], + ui: { + navLink: true, + }, }, read: { app: ['kibana'], @@ -88,7 +98,9 @@ const kibanaFeatures: Feature[] = [ 'dashboard', ], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, @@ -107,7 +119,9 @@ const kibanaFeatures: Feature[] = [ all: [], read: ['config'], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, @@ -125,7 +139,9 @@ const kibanaFeatures: Feature[] = [ all: ['config'], read: [], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, @@ -146,7 +162,9 @@ const timelionFeatures: Feature[] = [ all: ['timelion'], read: ['config', 'index-pattern'], }, - ui: [], + ui: { + navLink: true, + }, }, read: { app: ['timelion'], @@ -154,7 +172,9 @@ const timelionFeatures: Feature[] = [ all: [], read: ['config', 'index-pattern', 'timelion'], }, - ui: [], + ui: { + navLink: true, + }, }, }, }, From 4408bc6b846d867bf7095d1e20a633392ffc8549 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 20 Nov 2018 16:08:05 -0800 Subject: [PATCH 06/13] Fixing some checkPrivileges related unit tests --- .../check_privileges.test.ts.snap | 18 +- .../validate_es_response.test.ts.snap | 6 + .../authorization/check_privileges.test.ts | 695 ++++++++---------- .../lib/authorization/check_privileges.ts | 5 +- .../check_privileges_dynamically.test.ts | 17 +- .../validate_es_response.test.ts | 116 ++- .../lib/authorization/validate_es_response.ts | 27 +- 7 files changed, 484 insertions(+), 400 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.ts.snap index 1212c2cd6a5cb..3125b296d5cf5 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.ts.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/check_privileges.test.ts.snap @@ -2,11 +2,13 @@ exports[`#atSpace throws error when checking for login and user has login but doesn't have version 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`; -exports[`#atSpace with a malformed Elasticsearch response throws a validation error when an extra privilege is present in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because ["saved_object:bar-type/get" is not allowed]]]]`; +exports[`#atSpace with a malformed Elasticsearch response throws a validation error when a cluster privilege is missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because [child "monitor_bar" fails because ["monitor_bar" is required]]]`; + +exports[`#atSpace with a malformed Elasticsearch response throws a validation error when an extra cluster privilege is in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because ["manage_bar" is not allowed]]`; -exports[`#atSpace with a malformed Elasticsearch response throws a validation error when privileges are missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because [child "saved_object:foo-type/get" fails because ["saved_object:foo-type/get" is required]]]]]`; +exports[`#atSpace with a malformed Elasticsearch response throws a validation error when an extra privilege is present in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because ["saved_object:bar-type/get" is not allowed]]]]`; -exports[`#atSpaces throws error when Elasticsearch returns malformed response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because [child "mock-action:version" fails because ["mock-action:version" is required]]]]]`; +exports[`#atSpace with a malformed Elasticsearch response throws a validation error when application privileges are missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because [child "saved_object:foo-type/get" fails because ["saved_object:foo-type/get" is required]]]]]`; exports[`#atSpaces throws error when checking for login and user has login but doesn't have version 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`; @@ -16,12 +18,20 @@ exports[`#atSpaces with a malformed Elasticsearch response throws a validation e exports[`#atSpaces with a malformed Elasticsearch response throws a validation error when an extra space is present in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because ["space:space_3" is not allowed]]]`; +exports[`#atSpaces with a malformed Elasticsearch response throws a validation error when missing a cluster privilege in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because [child "monitor_bar" fails because ["monitor_bar" is required]]]`; + exports[`#atSpaces with a malformed Elasticsearch response throws a validation error when privileges are missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_2" fails because ["space:space_2" is required]]]]`; -exports[`#globally throws error when Elasticsearch returns malformed response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "*" fails because [child "mock-action:version" fails because ["mock-action:version" is required]]]]]`; +exports[`#atSpaces with a malformed Elasticsearch response throws a validation error when there's an extra cluster privilege in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because ["manage_bar" is not allowed]]`; + +exports[`#atSpaces with a malformed Elasticsearch response throws error when Elasticsearch returns malformed response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "space:space_1" fails because [child "mock-action:version" fails because ["mock-action:version" is required]]]]]`; exports[`#globally throws error when checking for login and user has login but doesn't have version 1`] = `[Error: Multiple versions of Kibana are running against the same Elasticsearch cluster, unable to authorize user.]`; +exports[`#globally with a malformed Elasticsearch response throws a validation error when an extra cluster privilege is in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because ["manage_bar" is not allowed]]`; + exports[`#globally with a malformed Elasticsearch response throws a validation error when an extra privilege is present in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "*" fails because ["saved_object:bar-type/get" is not allowed]]]]`; +exports[`#globally with a malformed Elasticsearch response throws a validation error when cluster privilege is missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "cluster" fails because [child "monitor_bar" fails because ["monitor_bar" is required]]]`; + exports[`#globally with a malformed Elasticsearch response throws a validation error when privileges are missing in the response 1`] = `[Error: Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child "application" fails because [child "kibana-our_application" fails because [child "*" fails because [child "saved_object:foo-type/get" fails because ["saved_object:foo-type/get" is required]]]]]`; diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/validate_es_response.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/validate_es_response.test.ts.snap index 226002545a378..616bc88a7b422 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/validate_es_response.test.ts.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/validate_es_response.test.ts.snap @@ -1,5 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`validateEsPrivilegeResponse fails validation when a cluster privilege is malformed in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"cluster\\" fails because [child \\"clusterPrivilege2\\" fails because [\\"clusterPrivilege2\\" must be a boolean]]"`; + +exports[`validateEsPrivilegeResponse fails validation when a cluster privilege is missing in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"cluster\\" fails because [child \\"clusterPrivilege2\\" fails because [\\"clusterPrivilege2\\" is required]]"`; + exports[`validateEsPrivilegeResponse fails validation when an action is malformed in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"application\\" fails because [child \\"foo-application\\" fails because [child \\"foo-resource\\" fails because [child \\"action3\\" fails because [\\"action3\\" must be a boolean]]]]"`; exports[`validateEsPrivilegeResponse fails validation when an action is missing in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"application\\" fails because [child \\"foo-application\\" fails because [child \\"foo-resource\\" fails because [child \\"action2\\" fails because [\\"action2\\" is required]]]]"`; @@ -19,3 +23,5 @@ exports[`validateEsPrivilegeResponse fails validation when the requested applica exports[`validateEsPrivilegeResponse fails validation when the resource propertry is malformed in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"application\\" fails because [child \\"foo-application\\" fails because [child \\"foo-resource\\" fails because [\\"foo-resource\\" must be an object]]]"`; exports[`validateEsPrivilegeResponse fails validation when there are no resource properties in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"application\\" fails because [child \\"foo-application\\" fails because [child \\"foo-resource\\" fails because [\\"foo-resource\\" is required]]]"`; + +exports[`validateEsPrivilegeResponse fails validation when there is an extra cluster privilege in the response 1`] = `"Invalid response received from Elasticsearch has_privilege endpoint. ValidationError: child \\"cluster\\" fails because [\\"clusterPrivilege3\\" is not allowed]"`; diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges.test.ts b/x-pack/plugins/security/server/lib/authorization/check_privileges.test.ts index 22d2fc3116b23..c3123fa827682 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.test.ts @@ -34,6 +34,7 @@ describe('#atSpace', () => { options: { spaceId: string; privilegeOrPrivileges: string | string[]; + clusterPrivileges: string[]; esHasPrivilegesResponse: HasPrivilegesResponse; expectedResult?: any; expectErrorThrown?: any; @@ -54,7 +55,8 @@ describe('#atSpace', () => { try { actualResult = await checkPrivileges.atSpace( options.spaceId, - options.privilegeOrPrivileges + options.privilegeOrPrivileges, + options.clusterPrivileges ); } catch (err) { errorThrown = err; @@ -65,6 +67,7 @@ describe('#atSpace', () => { 'shield.hasPrivileges', { body: { + cluster: options.clusterPrivileges, applications: [ { application, @@ -88,17 +91,20 @@ describe('#atSpace', () => { } if (options.expectErrorThrown) { + expect(errorThrown).not.toBeNull(); expect(errorThrown).toMatchSnapshot(); } }); }; - checkPrivilegesAtSpaceTest('successful when checking for login and user has login', { + checkPrivilegesAtSpaceTest('returns login privilege when checking for login', { spaceId: 'space_1', privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: true, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -114,30 +120,7 @@ describe('#atSpace', () => { privileges: { [mockActions.login]: true, }, - }, - }); - - checkPrivilegesAtSpaceTest(`failure when checking for login and user doesn't have login`, { - spaceId: 'space_1', - privilegeOrPrivileges: mockActions.login, - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: false, - [mockActions.version]: true, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - privileges: { - [mockActions.login]: false, - }, + clusterPrivileges: {}, }, }); @@ -146,9 +129,11 @@ describe('#atSpace', () => { { spaceId: 'space_1', privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -162,62 +147,41 @@ describe('#atSpace', () => { } ); - checkPrivilegesAtSpaceTest(`successful when checking for two actions and the user has both`, { + checkPrivilegesAtSpaceTest(`successful when checking two application and cluster privileges`, { spaceId: 'space_1', privilegeOrPrivileges: [ `saved_object:${savedObjectTypes[0]}/get`, `saved_object:${savedObjectTypes[1]}/get`, ], + clusterPrivileges: ['manage_foo', 'monitor_bar'], esHasPrivilegesResponse: { - has_all_requested: true, + has_all_requested: false, username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: false, + }, application: { [application]: { 'space:space_1': { [mockActions.login]: true, [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, }, }, }, }, expectedResult: { - hasAllRequested: true, + hasAllRequested: false, username: 'foo-username', privileges: { [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, }, - }, - }); - - checkPrivilegesAtSpaceTest(`failure when checking for two actions and the user has only one`, { - spaceId: 'space_1', - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - privileges: { - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + clusterPrivileges: { + manage_foo: true, + monitor_bar: false, }, }, }); @@ -228,9 +192,11 @@ describe('#atSpace', () => { { spaceId: 'space_1', privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -247,13 +213,67 @@ describe('#atSpace', () => { ); checkPrivilegesAtSpaceTest( - `throws a validation error when privileges are missing in the response`, + `throws a validation error when application privileges are missing in the response`, { spaceId: 'space_1', privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: {}, + application: { + [application]: { + 'space:space_1': { + [mockActions.login]: true, + [mockActions.version]: true, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesAtSpaceTest( + `throws a validation error when a cluster privilege is missing in the response`, + { + spaceId: 'space_1', + privilegeOrPrivileges: [], + clusterPrivileges: ['manage_foo', 'monitor_bar'], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: { + manage_foo: true, + }, + application: { + [application]: { + 'space:space_1': { + [mockActions.login]: true, + [mockActions.version]: true, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesAtSpaceTest( + `throws a validation error when an extra cluster privilege is in the response`, + { + spaceId: 'space_1', + privilegeOrPrivileges: [], + clusterPrivileges: ['manage_foo', 'monitor_bar'], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: true, + manage_bar: false, + }, application: { [application]: { 'space:space_1': { @@ -275,6 +295,7 @@ describe('#atSpaces', () => { options: { spaceIds: string[]; privilegeOrPrivileges: string | string[]; + clusterPrivileges: string[]; esHasPrivilegesResponse: HasPrivilegesResponse; expectedResult?: any; expectErrorThrown?: any; @@ -295,7 +316,8 @@ describe('#atSpaces', () => { try { actualResult = await checkPrivileges.atSpaces( options.spaceIds, - options.privilegeOrPrivileges + options.privilegeOrPrivileges, + options.clusterPrivileges ); } catch (err) { errorThrown = err; @@ -306,6 +328,7 @@ describe('#atSpaces', () => { 'shield.hasPrivileges', { body: { + cluster: options.clusterPrivileges, applications: [ { application, @@ -329,247 +352,104 @@ describe('#atSpaces', () => { } if (options.expectErrorThrown) { + expect(errorThrown).not.toBeNull(); expect(errorThrown).toMatchSnapshot(); } }); }; - checkPrivilegesAtSpacesTest( - 'successful when checking for login and user has login at both spaces', - { - spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: mockActions.login, - esHasPrivilegesResponse: { - has_all_requested: true, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, - }, - 'space:space_2': { - [mockActions.login]: true, - [mockActions.version]: true, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: true, - username: 'foo-username', - spacePrivileges: { - space_1: { - [mockActions.login]: true, - }, - space_2: { - [mockActions.login]: true, - }, - }, - }, - } - ); - - checkPrivilegesAtSpacesTest( - 'failure when checking for login and user has login at only one space', - { - spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: mockActions.login, - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, - }, - 'space:space_2': { - [mockActions.login]: false, - [mockActions.version]: true, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - spacePrivileges: { - space_1: { - [mockActions.login]: true, - }, - space_2: { - [mockActions.login]: false, - }, - }, - }, - } - ); - - checkPrivilegesAtSpacesTest( - `throws error when checking for login and user has login but doesn't have version`, - { - spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: mockActions.login, - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: false, - }, - 'space:space_2': { - [mockActions.login]: true, - [mockActions.version]: false, - }, - }, - }, - }, - expectErrorThrown: true, - } - ); - - checkPrivilegesAtSpacesTest(`throws error when Elasticsearch returns malformed response`, { + checkPrivilegesAtSpacesTest('returns login privileges when checking for login', { spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], + privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: true, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [mockActions.login]: true, + [mockActions.version]: true, }, 'space:space_2': { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [mockActions.login]: true, + [mockActions.version]: true, }, }, }, }, - expectErrorThrown: true, - }); - - checkPrivilegesAtSpacesTest( - `successful when checking for two actions at two spaces and user has it all`, - { - spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: true, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - 'space:space_2': { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - }, + expectedResult: { + hasAllRequested: true, + username: 'foo-username', + spacePrivileges: { + space_1: { + [mockActions.login]: true, }, - }, - expectedResult: { - hasAllRequested: true, - username: 'foo-username', - spacePrivileges: { - space_1: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - space_2: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, + space_2: { + [mockActions.login]: true, }, }, - } - ); + clusterPrivileges: {}, + }, + }); checkPrivilegesAtSpacesTest( - `failure when checking for two actions at two spaces and user has one action at one space`, + `throws error when checking for login and user has login but doesn't have version`, { spaceIds: ['space_1', 'space_2'], - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: false, - }, - 'space:space_2': { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: false, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - spacePrivileges: { - space_1: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: false, - }, - space_2: { - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: false, + privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: {}, + application: { + [application]: { + 'space:space_1': { + [mockActions.login]: true, + [mockActions.version]: false, + }, + 'space:space_2': { + [mockActions.login]: true, + [mockActions.version]: false, + }, }, }, }, + expectErrorThrown: true, } ); checkPrivilegesAtSpacesTest( - `failure when checking for two actions at two spaces and user has two actions at one space`, + `successful when checking two application and cluster privileges at two spaces`, { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [ `saved_object:${savedObjectTypes[0]}/get`, `saved_object:${savedObjectTypes[1]}/get`, ], + clusterPrivileges: ['manage_foo', 'monitor_bar'], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: false, + }, application: { [application]: { 'space:space_1': { [mockActions.login]: true, [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, }, 'space:space_2': { [mockActions.login]: true, [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: false, + [`saved_object:${savedObjectTypes[1]}/get`]: true, }, }, }, @@ -580,71 +460,59 @@ describe('#atSpaces', () => { spacePrivileges: { space_1: { [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, }, space_2: { [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: false, + [`saved_object:${savedObjectTypes[1]}/get`]: true, }, }, + clusterPrivileges: { + manage_foo: true, + monitor_bar: false, + }, }, } ); - checkPrivilegesAtSpacesTest( - `failure when checking for two actions at two spaces and user has two actions at one space & one action at the other`, - { + describe('with a malformed Elasticsearch response', () => { + checkPrivilegesAtSpacesTest(`throws error when Elasticsearch returns malformed response`, { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [ `saved_object:${savedObjectTypes[0]}/get`, `saved_object:${savedObjectTypes[1]}/get`, ], + clusterPrivileges: [], esHasPrivilegesResponse: { - has_all_requested: false, + has_all_requested: true, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { - [mockActions.login]: true, - [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: true, [`saved_object:${savedObjectTypes[1]}/get`]: true, }, 'space:space_2': { - [mockActions.login]: true, - [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: false, + [`saved_object:${savedObjectTypes[1]}/get`]: true, }, }, }, }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - spacePrivileges: { - space_1: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - space_2: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: false, - }, - }, - }, - } - ); + expectErrorThrown: true, + }); - describe('with a malformed Elasticsearch response', () => { checkPrivilegesAtSpacesTest( `throws a validation error when an extra privilege is present in the response`, { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -671,9 +539,11 @@ describe('#atSpaces', () => { { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -698,9 +568,11 @@ describe('#atSpaces', () => { { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { 'space:space_1': { @@ -730,9 +602,70 @@ describe('#atSpaces', () => { { spaceIds: ['space_1', 'space_2'], privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: {}, + application: { + [application]: { + 'space:space_1': { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: false, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesAtSpacesTest( + `throws a validation error when missing a cluster privilege in the response`, + { + spaceIds: ['space_1', 'space_2'], + privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: ['manage_foo', 'monitor_bar'], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: { + manage_foo: true, + }, + application: { + [application]: { + 'space:space_1': { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: false, + }, + 'space:space_2': { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: false, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesAtSpacesTest( + `throws a validation error when there's an extra cluster privilege in the response`, + { + spaceIds: ['space_1', 'space_2'], + privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: ['manage_foo', 'monitor_bar'], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: true, + manage_bar: false, + }, application: { [application]: { 'space:space_1': { @@ -740,6 +673,11 @@ describe('#atSpaces', () => { [mockActions.version]: true, [`saved_object:${savedObjectTypes[0]}/get`]: false, }, + 'space:space_2': { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: false, + }, }, }, }, @@ -754,6 +692,7 @@ describe('#globally', () => { description: string, options: { privilegeOrPrivileges: string | string[]; + clusterPrivileges: string[]; esHasPrivilegesResponse: HasPrivilegesResponse; expectedResult?: any; expectErrorThrown?: any; @@ -772,7 +711,10 @@ describe('#globally', () => { let actualResult; let errorThrown = null; try { - actualResult = await checkPrivileges.globally(options.privilegeOrPrivileges); + actualResult = await checkPrivileges.globally( + options.privilegeOrPrivileges, + options.clusterPrivileges + ); } catch (err) { errorThrown = err; } @@ -782,6 +724,7 @@ describe('#globally', () => { 'shield.hasPrivileges', { body: { + cluster: options.clusterPrivileges, applications: [ { application, @@ -805,16 +748,19 @@ describe('#globally', () => { } if (options.expectErrorThrown) { + expect(errorThrown).not.toBeNull(); expect(errorThrown).toMatchSnapshot(); } }); }; - checkPrivilegesGloballyTest('successful when checking for login and user has login', { + checkPrivilegesGloballyTest('returns login privilege when checking for login', { privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: true, username: 'foo-username', + cluster: {}, application: { [application]: { [GLOBAL_RESOURCE]: { @@ -830,29 +776,7 @@ describe('#globally', () => { privileges: { [mockActions.login]: true, }, - }, - }); - - checkPrivilegesGloballyTest(`failure when checking for login and user doesn't have login`, { - privilegeOrPrivileges: mockActions.login, - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - [GLOBAL_RESOURCE]: { - [mockActions.login]: false, - [mockActions.version]: true, - }, - }, - }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - privileges: { - [mockActions.login]: false, - }, + clusterPrivileges: {}, }, }); @@ -860,9 +784,11 @@ describe('#globally', () => { `throws error when checking for login and user has login but doesn't have version`, { privilegeOrPrivileges: mockActions.login, + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { [GLOBAL_RESOURCE]: { @@ -876,92 +802,57 @@ describe('#globally', () => { } ); - checkPrivilegesGloballyTest(`throws error when Elasticsearch returns malformed response`, { - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - [GLOBAL_RESOURCE]: { - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, + checkPrivilegesGloballyTest( + `successful when checking for two application and cluster privileges`, + { + privilegeOrPrivileges: [ + `saved_object:${savedObjectTypes[0]}/get`, + `saved_object:${savedObjectTypes[1]}/get`, + ], + clusterPrivileges: ['manage_foo', 'monitor_bar'], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: false, }, - }, - }, - expectErrorThrown: true, - }); - - checkPrivilegesGloballyTest(`successful when checking for two actions and the user has both`, { - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: true, - username: 'foo-username', - application: { - [application]: { - [GLOBAL_RESOURCE]: { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, + application: { + [application]: { + [GLOBAL_RESOURCE]: { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, + }, }, }, }, - }, - expectedResult: { - hasAllRequested: true, - username: 'foo-username', - privileges: { - [`saved_object:${savedObjectTypes[0]}/get`]: true, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - }, - }); - - checkPrivilegesGloballyTest(`failure when checking for two actions and the user has only one`, { - privilegeOrPrivileges: [ - `saved_object:${savedObjectTypes[0]}/get`, - `saved_object:${savedObjectTypes[1]}/get`, - ], - esHasPrivilegesResponse: { - has_all_requested: false, - username: 'foo-username', - application: { - [application]: { - [GLOBAL_RESOURCE]: { - [mockActions.login]: true, - [mockActions.version]: true, - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, + expectedResult: { + hasAllRequested: false, + username: 'foo-username', + privileges: { + [`saved_object:${savedObjectTypes[0]}/get`]: true, + [`saved_object:${savedObjectTypes[1]}/get`]: false, + }, + clusterPrivileges: { + manage_foo: true, + monitor_bar: false, }, }, - }, - expectedResult: { - hasAllRequested: false, - username: 'foo-username', - privileges: { - [`saved_object:${savedObjectTypes[0]}/get`]: false, - [`saved_object:${savedObjectTypes[1]}/get`]: true, - }, - }, - }); + } + ); describe('with a malformed Elasticsearch response', () => { checkPrivilegesGloballyTest( `throws a validation error when an extra privilege is present in the response`, { privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: {}, application: { [application]: { [GLOBAL_RESOURCE]: { @@ -981,14 +872,68 @@ describe('#globally', () => { `throws a validation error when privileges are missing in the response`, { privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: [], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: {}, + application: { + [application]: { + [GLOBAL_RESOURCE]: { + [mockActions.login]: true, + [mockActions.version]: true, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesGloballyTest( + `throws a validation error when cluster privilege is missing in the response`, + { + privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: ['manage_foo', 'monitor_bar'], + esHasPrivilegesResponse: { + has_all_requested: false, + username: 'foo-username', + cluster: { + manage_foo: true, + }, + application: { + [application]: { + [GLOBAL_RESOURCE]: { + [mockActions.login]: true, + [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: true, + }, + }, + }, + }, + expectErrorThrown: true, + } + ); + + checkPrivilegesGloballyTest( + `throws a validation error when an extra cluster privilege is in the response`, + { + privilegeOrPrivileges: [`saved_object:${savedObjectTypes[0]}/get`], + clusterPrivileges: ['manage_foo', 'monitor_bar'], esHasPrivilegesResponse: { has_all_requested: false, username: 'foo-username', + cluster: { + manage_foo: true, + monitor_bar: true, + manage_bar: false, + }, application: { [application]: { [GLOBAL_RESOURCE]: { [mockActions.login]: true, [mockActions.version]: true, + [`saved_object:${savedObjectTypes[0]}/get`]: true, }, }, }, diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges.ts b/x-pack/plugins/security/server/lib/authorization/check_privileges.ts index 690b03d8863e5..4bc874eb36d14 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges.ts @@ -90,7 +90,7 @@ export function checkPrivilegesWithRequestFactory( const checkPrivilegesAtResources = async ( resources: string[], privilegeOrPrivileges: string | string[], - clusterPrivileges?: string[] + clusterPrivileges: string[] = [] ): Promise => { const privileges = Array.isArray(privilegeOrPrivileges) ? privilegeOrPrivileges @@ -118,7 +118,8 @@ export function checkPrivilegesWithRequestFactory( hasPrivilegesResponse, application, allApplicationPrivileges, - resources + resources, + clusterPrivileges ); const applicationPrivilegesResponse = hasPrivilegesResponse.application[application]; diff --git a/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts b/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts index 9b76f554b7ee8..cabcde1710211 100644 --- a/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts @@ -19,15 +19,20 @@ test(`checkPrivileges.atSpace when spaces is enabled`, async () => { }; const request = Symbol(); const privilegeOrPrivileges = ['foo', 'bar']; + const clusterPrivileges = ['cluster_foo', 'cluster_bar']; const checkPrivilegesDynamically = checkPrivilegesDynamicallyWithRequestFactory( mockCheckPrivilegesWithRequest, mockSpaces )(request as any); - const result = await checkPrivilegesDynamically(privilegeOrPrivileges); + const result = await checkPrivilegesDynamically(privilegeOrPrivileges, clusterPrivileges); expect(result).toBe(expectedResult); expect(mockCheckPrivilegesWithRequest).toHaveBeenCalledWith(request); - expect(mockCheckPrivileges.atSpace).toHaveBeenCalledWith(spaceId, privilegeOrPrivileges); + expect(mockCheckPrivileges.atSpace).toHaveBeenCalledWith( + spaceId, + privilegeOrPrivileges, + clusterPrivileges + ); }); test(`checkPrivileges.globally when spaces is disabled`, async () => { @@ -41,13 +46,17 @@ test(`checkPrivileges.globally when spaces is disabled`, async () => { }; const request = Symbol(); const privilegeOrPrivileges = ['foo', 'bar']; + const clusterPrivileges = ['cluster_foo', 'cluster_bar']; const checkPrivilegesDynamically = checkPrivilegesDynamicallyWithRequestFactory( mockCheckPrivilegesWithRequest, mockSpaces )(request as any); - const result = await checkPrivilegesDynamically(privilegeOrPrivileges); + const result = await checkPrivilegesDynamically(privilegeOrPrivileges, clusterPrivileges); expect(result).toBe(expectedResult); expect(mockCheckPrivilegesWithRequest).toHaveBeenCalledWith(request); - expect(mockCheckPrivileges.globally).toHaveBeenCalledWith(privilegeOrPrivileges); + expect(mockCheckPrivileges.globally).toHaveBeenCalledWith( + privilegeOrPrivileges, + clusterPrivileges + ); }); diff --git a/x-pack/plugins/security/server/lib/authorization/validate_es_response.test.ts b/x-pack/plugins/security/server/lib/authorization/validate_es_response.test.ts index 2e0d1b8501e65..997d9bc99a325 100644 --- a/x-pack/plugins/security/server/lib/authorization/validate_es_response.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/validate_es_response.test.ts @@ -19,6 +19,10 @@ describe('validateEsPrivilegeResponse', () => { it('should validate a proper response', () => { const response = { ...commonResponse, + cluster: { + clusterPrivilege1: true, + clusterPrivilege2: true, + }, application: { [application]: { [resource1]: { @@ -39,7 +43,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + ['clusterPrivilege1', 'clusterPrivilege2'] ); expect(result).toEqual(response); }); @@ -47,6 +52,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an action is missing in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -67,7 +73,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -75,6 +82,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an extra action is present in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -97,7 +105,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -105,6 +114,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an action is malformed in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -126,7 +136,8 @@ describe('validateEsPrivilegeResponse', () => { response as any, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -134,6 +145,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an extra application is present in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -167,7 +179,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -175,6 +188,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when the requested application is missing from the response', () => { const response = { ...commonResponse, + cluster: {}, application: {}, }; @@ -183,7 +197,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -191,6 +206,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when the "application" property is missing from the response', () => { const response = { ...commonResponse, + cluster: {}, index: {}, }; @@ -199,7 +215,8 @@ describe('validateEsPrivilegeResponse', () => { response as any, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -207,6 +224,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an expected resource property is missing from the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -223,7 +241,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -231,6 +250,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when there are no resource properties in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: {}, }, @@ -241,7 +261,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -249,6 +270,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when an unexpected resource property is present in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: { @@ -270,7 +292,8 @@ describe('validateEsPrivilegeResponse', () => { response, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] ) ).toThrowErrorMatchingSnapshot(); }); @@ -278,6 +301,7 @@ describe('validateEsPrivilegeResponse', () => { it('fails validation when the resource propertry is malformed in the response', () => { const response = { ...commonResponse, + cluster: {}, application: { [application]: { [resource1]: 'not-an-object', @@ -295,7 +319,77 @@ describe('validateEsPrivilegeResponse', () => { response as any, application, ['action1', 'action2', 'action3'], - [resource1, resource2] + [resource1, resource2], + [] + ) + ).toThrowErrorMatchingSnapshot(); + }); + + it('fails validation when a cluster privilege is missing in the response', () => { + const response = { + ...commonResponse, + cluster: { + clusterPrivilege1: true, + }, + application: { + [application]: {}, + }, + }; + + expect(() => + validateEsPrivilegeResponse( + response as any, + application, + [], + [], + ['clusterPrivilege1', 'clusterPrivilege2'] + ) + ).toThrowErrorMatchingSnapshot(); + }); + + it('fails validation when there is an extra cluster privilege in the response', () => { + const response = { + ...commonResponse, + cluster: { + clusterPrivilege1: true, + clusterPrivilege2: true, + clusterPrivilege3: false, + }, + application: { + [application]: {}, + }, + }; + + expect(() => + validateEsPrivilegeResponse( + response as any, + application, + [], + [], + ['clusterPrivilege1', 'clusterPrivilege2'] + ) + ).toThrowErrorMatchingSnapshot(); + }); + + it('fails validation when a cluster privilege is malformed in the response', () => { + const response = { + ...commonResponse, + cluster: { + clusterPrivilege1: true, + clusterPrivilege2: 'this should be a boolean', + }, + application: { + [application]: {}, + }, + }; + + expect(() => + validateEsPrivilegeResponse( + response as any, + application, + [], + [], + ['clusterPrivilege1', 'clusterPrivilege2'] ) ).toThrowErrorMatchingSnapshot(); }); diff --git a/x-pack/plugins/security/server/lib/authorization/validate_es_response.ts b/x-pack/plugins/security/server/lib/authorization/validate_es_response.ts index 8d8e3125266a2..43279abf40734 100644 --- a/x-pack/plugins/security/server/lib/authorization/validate_es_response.ts +++ b/x-pack/plugins/security/server/lib/authorization/validate_es_response.ts @@ -11,9 +11,10 @@ export function validateEsPrivilegeResponse( response: HasPrivilegesResponse, application: string, actions: string[], - resources: string[] + resources: string[], + clusterPrivileges: string[] ) { - const schema = buildValidationSchema(application, actions, resources); + const schema = buildValidationSchema(application, actions, resources, clusterPrivileges); const { error, value } = schema.validate(response); if (error) { @@ -36,7 +37,23 @@ function buildActionsValidationSchema(actions: string[]) { }).required(); } -function buildValidationSchema(application: string, actions: string[], resources: string[]) { +function buildClusterValidationSchema(clusterPrivileges: string[]) { + return Joi.object({ + ...clusterPrivileges.reduce>((acc, clusterPrivilege) => { + return { + ...acc, + [clusterPrivilege]: Joi.bool().required(), + }; + }, {}), + }).required(); +} + +function buildValidationSchema( + application: string, + actions: string[], + resources: string[], + clusterPrivileges: string[] +) { const actionValidationSchema = buildActionsValidationSchema(actions); const resourceValidationSchema = Joi.object({ @@ -48,10 +65,12 @@ function buildValidationSchema(application: string, actions: string[], resources }, {}), }).required(); + const clusterValidationSchema = buildClusterValidationSchema(clusterPrivileges); + return Joi.object({ username: Joi.string().required(), has_all_requested: Joi.bool(), - cluster: Joi.object(), + cluster: clusterValidationSchema, application: Joi.object({ [application]: resourceValidationSchema, }).required(), From ec08461e6f2f8bcf07caae0ab82e8ca72b0e410f Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 26 Nov 2018 17:25:45 -0800 Subject: [PATCH 07/13] Fixing how we disable cluster only driven capabilities --- x-pack/plugins/ml/index.js | 5 - x-pack/plugins/security/index.js | 8 +- .../server/lib/authorization/index.ts | 2 +- .../disable_ui_capabilities.test.ts.snap | 0 .../features_cluster_privileges.ts | 45 ++--- .../authorization/ui_capabilities/index.ts | 7 + .../ui_capabilities.test.ts} | 180 +++++++++++++++--- .../ui_capabilities.ts} | 44 +++-- 8 files changed, 208 insertions(+), 83 deletions(-) rename x-pack/plugins/security/server/lib/authorization/{ => ui_capabilities}/__snapshots__/disable_ui_capabilities.test.ts.snap (100%) rename x-pack/plugins/security/server/lib/authorization/{ => ui_capabilities}/features_cluster_privileges.ts (55%) create mode 100644 x-pack/plugins/security/server/lib/authorization/ui_capabilities/index.ts rename x-pack/plugins/security/server/lib/authorization/{disable_ui_capabilities.test.ts => ui_capabilities/ui_capabilities.test.ts} (59%) rename x-pack/plugins/security/server/lib/authorization/{disable_ui_capabilities.ts => ui_capabilities/ui_capabilities.ts} (65%) diff --git a/x-pack/plugins/ml/index.js b/x-pack/plugins/ml/index.js index bc419f1270e72..d10364482156b 100644 --- a/x-pack/plugins/ml/index.js +++ b/x-pack/plugins/ml/index.js @@ -74,11 +74,6 @@ export const ml = (kibana) => { ui: { navLink: true, }, - }, - manage_ccr: { - ui: { - navLink: true, - }, } } } diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index 89f1c8ecc844a..e6a70a8238713 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -20,7 +20,7 @@ import { checkLicense } from './server/lib/check_license'; import { initAuthenticator } from './server/lib/authentication/authenticator'; import { SecurityAuditLogger } from './server/lib/audit_logger'; import { AuditLogger } from '../../server/lib/audit_logger'; -import { createAuthorizationService, disableUICapabilitesFactory, registerPrivilegesWithCluster } from './server/lib/authorization'; +import { createAuthorizationService, registerPrivilegesWithCluster, uiCapabilitesFactory } from './server/lib/authorization'; import { watchStatusAndLicenseToInitialize } from '../../server/lib/watch_status_and_license_to_initialize'; import { SecureSavedObjectsClientWrapper } from './server/lib/saved_objects_client/secure_saved_objects_client_wrapper'; import { deepFreeze } from './server/lib/deep_freeze'; @@ -91,18 +91,18 @@ export const security = (kibana) => new kibana.Plugin({ }; }, replaceInjectedVars: async function (originalInjectedVars, request, server) { - const disableUICapabilites = disableUICapabilitesFactory(server, request); + const uiCapabilities = uiCapabilitesFactory(server, request); // if we're an anonymous route, we disable all ui capabilities if (request.route.settings.auth === false) { return { ...originalInjectedVars, - uiCapabilities: disableUICapabilites.all(originalInjectedVars.uiCapabilities) + uiCapabilities: uiCapabilities.disableAll(originalInjectedVars.uiCapabilities) }; } return { ...originalInjectedVars, - uiCapabilities: await disableUICapabilites.usingPrivileges(originalInjectedVars.uiCapabilities) + uiCapabilities: await uiCapabilities.disableUsingPrivileges(originalInjectedVars.uiCapabilities) }; } }, diff --git a/x-pack/plugins/security/server/lib/authorization/index.ts b/x-pack/plugins/security/server/lib/authorization/index.ts index 69e31b8d48445..814119021ac6f 100644 --- a/x-pack/plugins/security/server/lib/authorization/index.ts +++ b/x-pack/plugins/security/server/lib/authorization/index.ts @@ -7,7 +7,7 @@ export { Actions } from './actions'; // @ts-ignore export { createAuthorizationService } from './service'; -export { disableUICapabilitesFactory } from './disable_ui_capabilities'; +export { uiCapabilitesFactory } from './ui_capabilities'; export { PrivilegeSerializer } from './privilege_serializer'; // @ts-ignore export { registerPrivilegesWithCluster } from './register_privileges_with_cluster'; diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/disable_ui_capabilities.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap similarity index 100% rename from x-pack/plugins/security/server/lib/authorization/__snapshots__/disable_ui_capabilities.test.ts.snap rename to x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap diff --git a/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/features_cluster_privileges.ts similarity index 55% rename from x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts rename to x-pack/plugins/security/server/lib/authorization/ui_capabilities/features_cluster_privileges.ts index bf2d01b1983c0..b843e7e9ee829 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_cluster_privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/features_cluster_privileges.ts @@ -4,9 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Feature, isFeaturePrivilegesCluster } from '../../../../xpack_main/types'; -import { Actions } from './actions'; +import { Feature, isFeaturePrivilegesCluster } from '../../../../../xpack_main/types'; +import { Actions } from '../actions'; +// this is "under test" by the disable_ui_capabilities, as we don't have other usages at this point. +// this should be treated as an implementation detail, and if we want it exposed publicly, we'll need +// to add more tests export class FeaturesClusterPrivileges { private allClusterPrivileges: string[]; private actionToClusterPrivileges: Record; @@ -17,29 +20,27 @@ export class FeaturesClusterPrivileges { .filter(isFeaturePrivilegesCluster) .reduce((acc, privileges) => [...acc, ...Object.keys(privileges.cluster)], []); - this.actionToClusterPrivileges = features - .filter(feature => feature.navLinkId) - .reduce>((acc, feature) => { - if (!isFeaturePrivilegesCluster(feature.privileges)) { - return acc; + this.actionToClusterPrivileges = features.reduce>((acc, feature) => { + if (!isFeaturePrivilegesCluster(feature.privileges)) { + return acc; + } + + for (const [clusterPrivilege, definition] of Object.entries(feature.privileges.cluster)) { + if (feature.navLinkId && definition.ui.navLink) { + const action = actions.ui.get('navLinks', feature.navLinkId!); + acc[action] = [...(acc[action] || []), clusterPrivilege]; } - for (const [clusterPrivilege, definition] of Object.entries(feature.privileges.cluster)) { - if (definition.ui.navLink) { - const action = actions.ui.get('navLinks', feature.navLinkId!); + if (definition.ui.capability) { + for (const capability of definition.ui.capability) { + const action = actions.ui.get(feature.id, capability); acc[action] = [...(acc[action] || []), clusterPrivilege]; } - - if (definition.ui.capability) { - for (const capability of definition.ui.capability) { - const action = actions.ui.get(feature.id, capability); - acc[action] = [...(acc[action] || []), clusterPrivilege]; - } - } } + } - return acc; - }, {}); + return acc; + }, {}); } public getAllClusterPrivileges() { @@ -48,11 +49,11 @@ export class FeaturesClusterPrivileges { public isActionEnabled( action: string, - checkPrivilegesResponseClusterPrivileges: Record + checkPrivilegesResponseClusterPrivileges: Record ) { const clusterPrivileges = this.actionToClusterPrivileges[action]; - if (!clusterPrivileges || clusterPrivileges.length === 0) { - return false; + if (!clusterPrivileges) { + return null; } return clusterPrivileges.every( diff --git a/x-pack/plugins/security/server/lib/authorization/ui_capabilities/index.ts b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/index.ts new file mode 100644 index 0000000000000..c0b4ec47e29e6 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { uiCapabilitesFactory } from './ui_capabilities'; diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.test.ts b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.test.ts similarity index 59% rename from x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.test.ts rename to x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.test.ts index 5677a9ec30c51..6f7d930fc6d10 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.test.ts @@ -4,22 +4,22 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Actions } from '.'; -import { disableUICapabilitesFactory } from './disable_ui_capabilities'; - -interface MockServerOptions { - checkPrivileges: { - reject?: any; - resolve?: any; - }; -} +import { Actions } from '../'; +import { Feature } from '../../../../../xpack_main/types'; +import { uiCapabilitesFactory } from './ui_capabilities'; const actions = new Actions('1.0.0-zeta1'); const mockRequest = { foo: Symbol(), }; -const createMockServer = (options: MockServerOptions) => { +const createMockServer = (options: { + checkPrivileges: { + reject?: any; + resolve?: any; + }; + features: Feature[]; +}) => { const mockSpacesPlugin = { getSpaceId: () => 'foo', }; @@ -44,8 +44,13 @@ const createMockServer = (options: MockServerOptions) => { }, }; + const mockXPackMainPlugin = { + getFeatures: jest.fn().mockReturnValue(options.features), + }; + return { plugins: { + xpack_main: mockXPackMainPlugin, spaces: mockSpacesPlugin, security: { authorization: mockAuthorizationService, @@ -54,7 +59,7 @@ const createMockServer = (options: MockServerOptions) => { }; }; -describe('usingPrivileges', () => { +describe('disableUsingPrivileges', () => { describe('checkPrivileges errors', () => { test(`disables all uiCapabilities when a 401 is thrown`, async () => { const mockServer = createMockServer({ @@ -63,9 +68,10 @@ describe('usingPrivileges', () => { statusCode: 401, }, }, + features: [], }); - const { usingPrivileges } = disableUICapabilitesFactory(mockServer, mockRequest); - const result = await usingPrivileges( + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( Object.freeze({ navLinks: { foo: true, @@ -105,9 +111,10 @@ describe('usingPrivileges', () => { statusCode: 403, }, }, + features: [], }); - const { usingPrivileges } = disableUICapabilitesFactory(mockServer, mockRequest); - const result = await usingPrivileges( + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( Object.freeze({ navLinks: { foo: true, @@ -145,10 +152,11 @@ describe('usingPrivileges', () => { checkPrivileges: { reject: new Error('something else entirely'), }, + features: [], }); - const { usingPrivileges } = disableUICapabilitesFactory(mockServer, mockRequest); + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); await expect( - usingPrivileges({ + disableUsingPrivileges({ navLinks: { foo: true, bar: false, @@ -158,7 +166,7 @@ describe('usingPrivileges', () => { }); }); - test(`disables ui capabilities when they don't have privileges`, async () => { + test(`disables ui capabilities when they don't have the application privileges`, async () => { const mockServer = createMockServer({ checkPrivileges: { resolve: { @@ -167,14 +175,13 @@ describe('usingPrivileges', () => { [actions.ui.get('navLinks', 'bar')]: false, [actions.ui.get('fooFeature', 'foo')]: true, [actions.ui.get('fooFeature', 'bar')]: false, - [actions.ui.get('barFeature', 'foo')]: true, - [actions.ui.get('barFeature', 'bar')]: false, }, }, }, + features: [], }); - const { usingPrivileges } = disableUICapabilitesFactory(mockServer, mockRequest); - const result = await usingPrivileges( + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( Object.freeze({ navLinks: { foo: true, @@ -184,7 +191,61 @@ describe('usingPrivileges', () => { foo: true, bar: true, }, - barFeature: { + }) + ); + + expect(result).toEqual({ + navLinks: { + foo: true, + bar: false, + }, + fooFeature: { + foo: true, + bar: false, + }, + }); + }); + + test(`disables ui capabilities when they don't have the cluster privilege`, async () => { + const mockServer = createMockServer({ + checkPrivileges: { + resolve: { + privileges: { + [actions.ui.get('navLinks', 'foo')]: false, + [actions.ui.get('navLinks', 'bar')]: true, + [actions.ui.get('fooFeature', 'foo')]: false, + [actions.ui.get('fooFeature', 'bar')]: true, + }, + clusterPrivileges: { + cluster_foo: false, + }, + }, + }, + features: [ + { + id: 'fooFeature', + name: 'Foo', + privileges: { + cluster: { + cluster_foo: { + ui: { + navLink: true, + capability: ['foo'], + }, + }, + }, + }, + }, + ], + }); + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( + Object.freeze({ + navLinks: { + foo: true, + bar: true, + }, + fooFeature: { foo: true, bar: true, }, @@ -193,14 +254,69 @@ describe('usingPrivileges', () => { expect(result).toEqual({ navLinks: { - foo: true, - bar: false, + foo: false, + bar: true, }, fooFeature: { + foo: false, + bar: true, + }, + }); + }); + + test(`doesn't disable ui capabilities when they only have the cluster privilege`, async () => { + const mockServer = createMockServer({ + checkPrivileges: { + resolve: { + privileges: { + [actions.ui.get('navLinks', 'foo')]: false, + [actions.ui.get('navLinks', 'bar')]: false, + [actions.ui.get('fooFeature', 'foo')]: false, + [actions.ui.get('fooFeature', 'bar')]: false, + }, + clusterPrivileges: { + cluster_foo: true, + }, + }, + }, + features: [ + { + id: 'fooFeature', + name: 'Foo', + navLinkId: 'foo', + privileges: { + cluster: { + cluster_foo: { + ui: { + navLink: true, + capability: ['foo'], + }, + }, + }, + }, + }, + ], + }); + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( + Object.freeze({ + navLinks: { + foo: true, + bar: true, + }, + fooFeature: { + foo: true, + bar: true, + }, + }) + ); + + expect(result).toEqual({ + navLinks: { foo: true, bar: false, }, - barFeature: { + fooFeature: { foo: true, bar: false, }, @@ -221,9 +337,10 @@ describe('usingPrivileges', () => { }, }, }, + features: [], }); - const { usingPrivileges } = disableUICapabilitesFactory(mockServer, mockRequest); - const result = await usingPrivileges( + const { disableUsingPrivileges } = uiCapabilitesFactory(mockServer, mockRequest); + const result = await disableUsingPrivileges( Object.freeze({ navLinks: { foo: false, @@ -257,15 +374,16 @@ describe('usingPrivileges', () => { }); }); -describe('all', () => { +describe('disableAll', () => { test(`disables all uiCapabilities`, () => { const mockServer = createMockServer({ checkPrivileges: { reject: new Error(`Don't use me`), }, + features: [], }); - const { all } = disableUICapabilitesFactory(mockServer, mockRequest); - const result = all( + const { disableAll } = uiCapabilitesFactory(mockServer, mockRequest); + const result = disableAll( Object.freeze({ navLinks: { foo: true, diff --git a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.ts similarity index 65% rename from x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts rename to x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.ts index a8dce1ea9b1ec..1beac3199e7ab 100644 --- a/x-pack/plugins/security/server/lib/authorization/disable_ui_capabilities.ts +++ b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/ui_capabilities.ts @@ -4,19 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -import { mapValues } from 'lodash'; +import { isBoolean, mapValues } from 'lodash'; import { UICapabilities } from 'ui/capabilities'; -import { Feature } from '../../../../xpack_main/types'; -import { Actions } from './actions'; -import { CheckPrivilegesAtResourceResponse } from './check_privileges'; -import { CheckPrivilegesDynamically } from './check_privileges_dynamically'; +import { Feature } from '../../../../../xpack_main/types'; +import { Actions } from '../actions'; +import { CheckPrivilegesAtResourceResponse } from '../check_privileges'; +import { CheckPrivilegesDynamically } from '../check_privileges_dynamically'; import { FeaturesClusterPrivileges } from './features_cluster_privileges'; -export function disableUICapabilitesFactory( - server: Record, - request: Record -) { - const { authorization } = server.plugins.security; +export function uiCapabilitesFactory(server: Record, request: Record) { + const { + xpack_main: xpackMainPlugin, + security: { authorization }, + } = server.plugins; const actions: Actions = authorization.actions; const disableAll = (uiCapabilities: UICapabilities) => { @@ -25,7 +25,7 @@ export function disableUICapabilitesFactory( ); }; - const usingPrivileges = async (uiCapabilities: UICapabilities) => { + const disableUsingPrivileges = async (uiCapabilities: UICapabilities) => { const uiActions = Object.entries(uiCapabilities).reduce( (acc, [featureId, featureUICapabilities]) => [ ...acc, @@ -36,7 +36,7 @@ export function disableUICapabilitesFactory( [] ); - const features: Feature[] = server.plugins.xpack_main.getFeatures(); + const features: Feature[] = xpackMainPlugin.getFeatures(); const featuresClusterPrivileges = new FeaturesClusterPrivileges(features, actions); const clusterPrivileges = featuresClusterPrivileges.getAllClusterPrivileges(); @@ -64,19 +64,23 @@ export function disableUICapabilitesFactory( } const action = actions.ui.get(featureId!, uiCapability!); - return ( - checkPrivilegesResponse.privileges[action] === true || - featuresClusterPrivileges.isActionEnabled( - action, - checkPrivilegesResponse.clusterPrivileges - ) + const isEnabledByClusterPrivileges = featuresClusterPrivileges.isActionEnabled( + action, + checkPrivilegesResponse.clusterPrivileges ); + // we need the explicit isBoolean() check here as this can be null + // if the action isn't specified by a feature's cluster privileges definition + if (isBoolean(isEnabledByClusterPrivileges)) { + return isEnabledByClusterPrivileges; + } + + return checkPrivilegesResponse.privileges[action] === true; }); }); }; return { - all: disableAll, - usingPrivileges, + disableAll, + disableUsingPrivileges, }; } From e556b0780e75f5d9cd093d34fd5e708f9695b706 Mon Sep 17 00:00:00 2001 From: kobelb Date: Mon, 26 Nov 2018 17:46:10 -0800 Subject: [PATCH 08/13] Fixing FeaturesPrivilegesBuilder tests --- ...ts => features_privileges_builder.test.ts} | 286 ++++++++++++------ 1 file changed, 193 insertions(+), 93 deletions(-) rename x-pack/plugins/security/server/lib/authorization/{features_privileges_builders.test.ts => features_privileges_builder.test.ts} (55%) diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builders.test.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts similarity index 55% rename from x-pack/plugins/security/server/lib/authorization/features_privileges_builders.test.ts rename to x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts index 7226e41a7866b..e94bdd12b6f5b 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builders.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts @@ -17,12 +17,16 @@ describe('#buildFeaturesPrivileges', () => { { id: 'foo', name: '', - privileges: {}, + privileges: { + kibana: {}, + }, }, { id: 'bar', name: '', - privileges: {}, + privileges: { + kibana: {}, + }, }, ]; const result = builder.buildFeaturesPrivileges(features); @@ -40,13 +44,15 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - app: [], - savedObject: { - all: [], - read: [], + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -67,14 +73,16 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - api: ['foo/operation', 'bar/operation'], - app: [], - savedObject: { - all: [], - read: [], + kibana: { + bar: { + api: ['foo/operation', 'bar/operation'], + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -100,13 +108,15 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - app: ['foo-app', 'bar-app'], - savedObject: { - all: [], - read: [], + kibana: { + bar: { + app: ['foo-app', 'bar-app'], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -132,13 +142,15 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - app: [], - savedObject: { - all: ['foo-type', 'bar-type'], - read: [], + kibana: { + bar: { + app: [], + savedObject: { + all: ['foo-type', 'bar-type'], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -163,13 +175,15 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - app: [], - savedObject: { - all: [], - read: ['foo-type', 'bar-type'], + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: ['foo-type', 'bar-type'], + }, + ui: {}, }, - ui: [], }, }, }, @@ -194,13 +208,17 @@ describe('#buildFeaturesPrivileges', () => { id: 'foo', name: '', privileges: { - bar: { - app: [], - savedObject: { - all: [], - read: [], + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: { + capability: ['foo-ui-capability', 'bar-ui-capability'], + }, }, - ui: ['foo-ui-capability', 'bar-ui-capability'], }, }, }, @@ -218,7 +236,7 @@ describe('#buildFeaturesPrivileges', () => { }); }); - test('includes navlink ui capability action when specified', () => { + test('includes navlink ui capability action when navLinkId is specified and ui.navLink is true', () => { const actions = new Actions(versionNumber); const builder = new FeaturesPrivilegesBuilder(actions); const features = [ @@ -227,13 +245,17 @@ describe('#buildFeaturesPrivileges', () => { name: '', navLinkId: 'foo-navlink', privileges: { - bar: { - app: [], - savedObject: { - all: [], - read: [], + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: { + navLink: true, + }, }, - ui: [], }, }, }, @@ -245,6 +267,68 @@ describe('#buildFeaturesPrivileges', () => { }, }); }); + + test(`doesn't include navlink ui capability action when navLinkId is specified and ui.navLink is false`, () => { + const actions = new Actions(versionNumber); + const builder = new FeaturesPrivilegesBuilder(actions); + const features = [ + { + id: 'foo', + name: '', + navLinkId: 'foo-navlink', + privileges: { + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: { + navLink: false, + }, + }, + }, + }, + }, + ]; + const result = builder.buildFeaturesPrivileges(features); + expect(result).toEqual({ + foo: { + bar: [actions.login, actions.version], + }, + }); + }); + + test(`doesn't include navlink ui capability action when navLinkId is specified and ui.navLink isn't specified`, () => { + const actions = new Actions(versionNumber); + const builder = new FeaturesPrivilegesBuilder(actions); + const features = [ + { + id: 'foo', + name: '', + navLinkId: 'foo-navlink', + privileges: { + kibana: { + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, + }, + }, + }, + }, + ]; + const result = builder.buildFeaturesPrivileges(features); + expect(result).toEqual({ + foo: { + bar: [actions.login, actions.version], + }, + }); + }); }); describe('#getApiReadActions', () => { @@ -256,24 +340,26 @@ describe('#getApiReadActions', () => { id: 'foo', name: '', privileges: { - // wrong privilege name - bar: { - app: [], - api: ['foo/api'], - savedObject: { - all: [], - read: [], + kibana: { + // wrong privilege name + bar: { + app: [], + api: ['foo/api'], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], - }, - // no api read privileges - read: { - app: [], - savedObject: { - all: [], - read: [], + // no api read privileges + read: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -281,15 +367,17 @@ describe('#getApiReadActions', () => { id: 'bar', name: '', privileges: { - // this one should show up in the results - read: { - app: [], - api: ['foo/api'], - savedObject: { - all: [], - read: [], + kibana: { + // this one should show up in the results + read: { + app: [], + api: ['foo/api'], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, @@ -307,44 +395,56 @@ describe('#getUIReadActions', () => { { id: 'foo', name: '', + navLinkId: 'foo', privileges: { - // wrong privilege name - bar: { - app: [], - savedObject: { - all: [], - read: [], + kibana: { + // wrong privilege name + bar: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], - }, - // no ui read privileges - read: { - app: [], - savedObject: { - all: [], - read: [], + // no ui read privileges + read: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: {}, }, - ui: [], }, }, }, { id: 'bar', name: '', + navLinkId: 'bar-nav-link', privileges: { - // this ui capability should show up in the results - read: { - app: [], - savedObject: { - all: [], - read: [], + kibana: { + // this ui capability should show up in the results + read: { + app: [], + savedObject: { + all: [], + read: [], + }, + ui: { + navLink: true, + capability: ['bar-ui-capability'], + }, }, - ui: ['bar-ui-capability'], }, }, }, ]; const result = builder.getUIReadActions(features); - expect(result).toEqual([actions.ui.get('bar', 'bar-ui-capability')]); + expect(result).toEqual([ + actions.ui.get('bar', 'bar-ui-capability'), + actions.ui.get('navLinks', 'bar-nav-link'), + ]); }); }); From 0ce8bd6c721e02003fe2bc907ac22759e0012d1b Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 27 Nov 2018 09:15:08 -0800 Subject: [PATCH 09/13] Removing snapshot --- .../__snapshots__/disable_ui_capabilities.test.ts.snap | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap diff --git a/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap deleted file mode 100644 index 53c567726332c..0000000000000 --- a/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/disable_ui_capabilities.test.ts.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`usingPrivileges checkPrivileges errors otherwise it throws the error 1`] = `"something else entirely"`; From 9e86a861c4955fdf0a1fd9013e2eb95500f0867d Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 27 Nov 2018 10:01:21 -0800 Subject: [PATCH 10/13] Fixing some privilege tests --- .../features_privileges_builder.test.ts | 5 +- .../features_privileges_builder.ts | 3 - .../lib/authorization/privileges.test.ts | 73 +++++++++++-------- .../ui_capabilities.test.ts.snap | 3 + 4 files changed, 48 insertions(+), 36 deletions(-) create mode 100644 x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/ui_capabilities.test.ts.snap diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts index e94bdd12b6f5b..04d21458e610f 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts @@ -442,9 +442,6 @@ describe('#getUIReadActions', () => { }, ]; const result = builder.getUIReadActions(features); - expect(result).toEqual([ - actions.ui.get('bar', 'bar-ui-capability'), - actions.ui.get('navLinks', 'bar-nav-link'), - ]); + expect(result).toEqual([actions.ui.get('bar', 'bar-ui-capability')]); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts index 7fcc1ac15d885..066914ac8440d 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts @@ -61,9 +61,6 @@ export class FeaturesPrivilegesBuilder { ...(ui.capability ? ui.capability.map(uiCapability => this.actions.ui.get(feature.id, uiCapability)) : []), - ...(feature.navLinkId && ui.navLink - ? [this.actions.ui.get('navLinks', feature.navLinkId)] - : []), ]; }) ); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.test.ts b/x-pack/plugins/security/server/lib/authorization/privileges.test.ts index 7775b63fff060..174b70a7b3050 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges.test.ts @@ -22,22 +22,29 @@ test(`builds privileges correctly`, () => { icon: 'arrowDown', navLinkId: 'kibana:foo-feature', privileges: { - all: { - app: ['foo-app'], - savedObject: { - all: ['foo-saved-object-type'], - read: ['bad-saved-object-type'], + kibana: { + all: { + app: ['foo-app'], + savedObject: { + all: ['foo-saved-object-type'], + read: ['bad-saved-object-type'], + }, + ui: { + navLink: true, + capability: ['show', 'showSaveButton', 'showCreateButton'], + }, }, - ui: ['show', 'showSaveButton', 'showCreateButton'], - }, - read: { - app: ['foo-app'], - api: ['foo/read/api'], - savedObject: { - all: [], - read: ['foo-saved-object-type', 'bar-saved-object-type'], + read: { + app: ['foo-app'], + api: ['foo/read/api'], + savedObject: { + all: [], + read: ['foo-saved-object-type', 'bar-saved-object-type'], + }, + ui: { + capability: ['show'], + }, }, - ui: ['show'], }, }, }, @@ -45,23 +52,31 @@ test(`builds privileges correctly`, () => { id: 'bar-feature', name: 'Bar Feature', icon: 'arrowUp', + navLinkId: 'kibana:bar-feature', privileges: { - all: { - app: ['bar-app'], - savedObject: { - all: ['bar-saved-object-type'], - read: ['foo-saved-object-type'], + kibana: { + all: { + app: ['bar-app'], + savedObject: { + all: ['bar-saved-object-type'], + read: ['foo-saved-object-type'], + }, + ui: { + capability: ['show', 'showSaveButton', 'showCreateButton'], + }, }, - ui: ['show', 'showSaveButton', 'showCreateButton'], - }, - read: { - app: ['bar-app'], - api: ['bar/read/api'], - savedObject: { - all: [], - read: ['foo-saved-object-type', 'bar-saved-object-type'], + read: { + app: ['bar-app'], + api: ['bar/read/api'], + savedObject: { + all: [], + read: ['foo-saved-object-type', 'bar-saved-object-type'], + }, + ui: { + navLink: true, + capability: ['show'], + }, }, - ui: ['show'], }, }, }, @@ -109,6 +124,7 @@ test(`builds privileges correctly`, () => { 'saved_object:bar-saved-object-type/get', 'saved_object:bar-saved-object-type/find', 'ui:bar-feature/show', + 'ui:navLinks/kibana:bar-feature', ], }, 'foo-feature': { @@ -143,7 +159,6 @@ test(`builds privileges correctly`, () => { 'saved_object:bar-saved-object-type/get', 'saved_object:bar-saved-object-type/find', 'ui:foo-feature/show', - 'ui:navLinks/kibana:foo-feature', ], }, }, diff --git a/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/ui_capabilities.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/ui_capabilities.test.ts.snap new file mode 100644 index 0000000000000..babfd3b95a231 --- /dev/null +++ b/x-pack/plugins/security/server/lib/authorization/ui_capabilities/__snapshots__/ui_capabilities.test.ts.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`disableUsingPrivileges checkPrivileges errors otherwise it throws the error 1`] = `"something else entirely"`; From 114c35c88a44a60c1f7f35eb5d3dc0a943e759d8 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 27 Nov 2018 10:11:25 -0800 Subject: [PATCH 11/13] Only features with read privileges appear in read mode --- .../lib/authorization/features_privileges_builder.test.ts | 5 ++++- .../server/lib/authorization/features_privileges_builder.ts | 3 +++ .../security/server/lib/authorization/privileges.test.ts | 4 ++-- .../plugins/security/server/lib/authorization/privileges.ts | 2 -- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts index 04d21458e610f..e94bdd12b6f5b 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.test.ts @@ -442,6 +442,9 @@ describe('#getUIReadActions', () => { }, ]; const result = builder.getUIReadActions(features); - expect(result).toEqual([actions.ui.get('bar', 'bar-ui-capability')]); + expect(result).toEqual([ + actions.ui.get('bar', 'bar-ui-capability'), + actions.ui.get('navLinks', 'bar-nav-link'), + ]); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts index 066914ac8440d..7fcc1ac15d885 100644 --- a/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts +++ b/x-pack/plugins/security/server/lib/authorization/features_privileges_builder.ts @@ -61,6 +61,9 @@ export class FeaturesPrivilegesBuilder { ...(ui.capability ? ui.capability.map(uiCapability => this.actions.ui.get(feature.id, uiCapability)) : []), + ...(feature.navLinkId && ui.navLink + ? [this.actions.ui.get('navLinks', feature.navLinkId)] + : []), ]; }) ); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.test.ts b/x-pack/plugins/security/server/lib/authorization/privileges.test.ts index 174b70a7b3050..51a56af00f2fc 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges.test.ts @@ -186,7 +186,7 @@ test(`builds privileges correctly`, () => { 'saved_object:bar-saved-object-type/find', 'ui:foo-feature/show', 'ui:bar-feature/show', - 'ui:navLinks/*', + 'ui:navLinks/kibana:bar-feature', ], }, space: { @@ -225,7 +225,7 @@ test(`builds privileges correctly`, () => { 'saved_object:bar-saved-object-type/find', 'ui:foo-feature/show', 'ui:bar-feature/show', - 'ui:navLinks/*', + 'ui:navLinks/kibana:bar-feature', ], }, }); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges.ts b/x-pack/plugins/security/server/lib/authorization/privileges.ts index f9a27faddf65d..18fc922d1870f 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges.ts @@ -57,7 +57,6 @@ export function privilegesFactory( actions.app.all, ...actions.savedObject.readOperations(validSavedObjectTypes), ...featuresPrivilegesBuilder.getUIReadActions(features), - actions.ui.allNavLinks, ], }, space: { @@ -76,7 +75,6 @@ export function privilegesFactory( actions.app.all, ...actions.savedObject.readOperations(validSavedObjectTypes), ...featuresPrivilegesBuilder.getUIReadActions(features), - actions.ui.allNavLinks, ], }, }; From 46a0943c9b17625ba4142643953cdb8755d8e10f Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 27 Nov 2018 11:58:38 -0800 Subject: [PATCH 12/13] Preventing duplicate navLinkIds from being registered --- .../server/lib/feature_registry/feature_registry.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts index af697abd8640a..c42ea34e395b3 100644 --- a/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts +++ b/x-pack/plugins/xpack_main/server/lib/feature_registry/feature_registry.ts @@ -58,12 +58,21 @@ export interface Feature { } const features: Record = {}; +const navLinkIds: Set = new Set(); export function registerFeature(feature: Feature) { if (feature.id in features) { throw new Error(`Feature with id ${feature.id} is already registered.`); } + if (feature.navLinkId) { + if (navLinkIds.has(feature.navLinkId)) { + throw new Error(`Feature with navLinkId of ${feature.navLinkId} is already registered`); + } + + navLinkIds.add(feature.navLinkId); + } + features[feature.id] = feature; } From fc600ee76db818caf92a01a3a8752180d6c8a370 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 27 Nov 2018 16:44:11 -0800 Subject: [PATCH 13/13] Removing app filtering --- x-pack/plugins/ml/index.js | 2 +- x-pack/plugins/security/index.js | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/x-pack/plugins/ml/index.js b/x-pack/plugins/ml/index.js index d10364482156b..324bb50ebcedb 100644 --- a/x-pack/plugins/ml/index.js +++ b/x-pack/plugins/ml/index.js @@ -70,7 +70,7 @@ export const ml = (kibana) => { navLinkId: 'ml', privileges: { cluster: { - manage_ml: { + monitor_ml: { ui: { navLink: true, }, diff --git a/x-pack/plugins/security/index.js b/x-pack/plugins/security/index.js index e6a70a8238713..db8d921341ce2 100644 --- a/x-pack/plugins/security/index.js +++ b/x-pack/plugins/security/index.js @@ -212,17 +212,6 @@ export const security = (kibana) => new kibana.Plugin({ const { actions, checkPrivilegesDynamicallyWithRequest } = server.plugins.security.authorization; const checkPrivileges = checkPrivilegesDynamicallyWithRequest(req); - // Enforce app restrictions - if (path.startsWith('/app/')) { - const appId = path.split('/', 3)[2]; - const appAction = actions.app.get(appId); - - const checkPrivilegesResponse = await checkPrivileges(appAction); - if (!checkPrivilegesResponse.hasAllRequested) { - return Boom.notFound(); - } - } - // Enforce API restrictions for associated applications if (path.startsWith('/api/')) { const { tags = [] } = req.route.settings;