From a3fcb51230b5fcd4c5ddb9f738cdf691eb80701b Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 4 Dec 2018 08:13:21 -0800 Subject: [PATCH 01/18] Updated the role api PUT structure --- .../server/routes/api/public/roles/put.js | 69 ++++++++++++------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/security/server/routes/api/public/roles/put.js b/x-pack/plugins/security/server/routes/api/public/roles/put.js index 8f10a1773527c..b85925b4c1b33 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/put.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/put.js @@ -20,20 +20,38 @@ export function initPutRolesApi( const transformKibanaPrivilegesToEs = (kibanaPrivileges) => { const kibanaApplicationPrivileges = []; - if (kibanaPrivileges.global && kibanaPrivileges.global.length) { + if (kibanaPrivileges.global) { kibanaApplicationPrivileges.push({ - privileges: kibanaPrivileges.global.map(privilege => PrivilegeSerializer.serializePrivilegeAssignedGlobally(privilege)), + privileges: [ + ...kibanaPrivileges.global.minimum ? [PrivilegeSerializer.serializeGlobalReservedPrivilege(kibanaPrivileges.global.minimum)] : [], + ...kibanaPrivileges.global.feature ? flatten( + Object.entries(kibanaPrivileges.global.feature).map( + ([featureName, privileges])=> privileges.map( + privilege => PrivilegeSerializer.serializeFeaturePrivilege(featureName, privilege) + ) + ) + ) : [], + ], application, resources: [GLOBAL_RESOURCE], }); } if (kibanaPrivileges.space) { - for(const [spaceId, privileges] of Object.entries(kibanaPrivileges.space)) { + for(const [spaceId, spacePrivileges] of Object.entries(kibanaPrivileges.space)) { kibanaApplicationPrivileges.push({ - privileges: privileges.map(privilege => PrivilegeSerializer.serializePrivilegeAssignedAtSpace(privilege)), + privileges: [ + ...spacePrivileges.minimum ? [PrivilegeSerializer.serializeGlobalReservedPrivilege(spacePrivileges.minimum)] : [], + ...spacePrivileges.feature ? flatten( + Object.entries(spacePrivileges.feature).map( + ([featureName, privileges])=> privileges.map( + privilege => PrivilegeSerializer.serializeFeaturePrivilege(featureName, privilege) + ) + ) + ) : [] + ], application, - resources: [ResourceSerializer.serializeSpaceResource(spaceId)] + resources: [ResourceSerializer.serializeSpaceResource(spaceId)], }); } } @@ -62,29 +80,28 @@ export function initPutRolesApi( }, identity); }; - // this should be short-lived once we refactor the way that these APIs work, hence the ugly string concatenation - // if you see this code in master, please yell at Brandon - const getFeaturePrivileges = (features) => { - return flatten(Object.entries(features).map( - ([featureName, featurePrivileges]) => { - return Object.keys(featurePrivileges).map( - (privilegeName) => { - return `${featureName}_${privilegeName}`; - }); - }) - ); - }; - - const getGlobalItemsSchema = () => { + const getGlobalSchema = () => { const privileges = authorization.privileges.get(); - const validPrivileges = [...Object.keys(privileges.global), ...getFeaturePrivileges(privileges.features)]; - return Joi.string().valid(validPrivileges); + const featureObject = Object.entries(privileges.features).reduce((acc, [feature, featurePrivileges]) => ({ + ...acc, + [feature]: Joi.array().items(Joi.string().valid(Object.keys(featurePrivileges))) + }), {}); + const featureSchema = Joi.object(featureObject); + return Joi.object({ + minimum: Joi.string().valid(Object.keys(privileges.global)), + feature: featureSchema, + }); }; - const getSpaceItemsSchema = () => { + const getSpaceSchema = () => { const privileges = authorization.privileges.get(); - const validPrivileges = [...Object.keys(privileges.space), ...getFeaturePrivileges(privileges.features)]; - return Joi.string().valid(validPrivileges); + return Joi.object().pattern(/^[a-z0-9_-]+$/, Joi.object({ + minimum: Joi.string().valid(Object.keys(privileges.space)), + feature: Joi.object(Object.entries(privileges.features).reduce((acc, [feature, featurePrivileges]) => ({ + ...acc, + [feature]: Joi.array().items(Joi.string().valid(Object.keys(featurePrivileges))) + }), {})) + })); }; const schema = Joi.object().keys({ @@ -103,8 +120,8 @@ export function initPutRolesApi( run_as: Joi.array().items(Joi.string()), }), kibana: Joi.object().keys({ - global: Joi.array().items(Joi.lazy(() => getGlobalItemsSchema())), - space: Joi.object().pattern(/^[a-z0-9_-]+$/, Joi.array().items(Joi.lazy(() => getSpaceItemsSchema()))) + global: Joi.lazy(() => getGlobalSchema()), + space: Joi.lazy(() => getSpaceSchema()), }) }); From e3a2393258957b30e84b15ab6bca7b078c57af1a Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 4 Dec 2018 11:20:28 -0800 Subject: [PATCH 02/18] Minimum is an array now --- .../security/server/routes/api/public/roles/put.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security/server/routes/api/public/roles/put.js b/x-pack/plugins/security/server/routes/api/public/roles/put.js index b85925b4c1b33..46910118a02e4 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/put.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/put.js @@ -23,7 +23,9 @@ export function initPutRolesApi( if (kibanaPrivileges.global) { kibanaApplicationPrivileges.push({ privileges: [ - ...kibanaPrivileges.global.minimum ? [PrivilegeSerializer.serializeGlobalReservedPrivilege(kibanaPrivileges.global.minimum)] : [], + ...kibanaPrivileges.global.minimum ? kibanaPrivileges.global.minimum.map( + privilege => PrivilegeSerializer.serializeGlobalReservedPrivilege(privilege) + ) : [], ...kibanaPrivileges.global.feature ? flatten( Object.entries(kibanaPrivileges.global.feature).map( ([featureName, privileges])=> privileges.map( @@ -41,7 +43,9 @@ export function initPutRolesApi( for(const [spaceId, spacePrivileges] of Object.entries(kibanaPrivileges.space)) { kibanaApplicationPrivileges.push({ privileges: [ - ...spacePrivileges.minimum ? [PrivilegeSerializer.serializeGlobalReservedPrivilege(spacePrivileges.minimum)] : [], + ...spacePrivileges.minimum ? spacePrivileges.minimum.map( + privilege => PrivilegeSerializer.serializeSpaceReservedPrivilege(privilege) + ) : [], ...spacePrivileges.feature ? flatten( Object.entries(spacePrivileges.feature).map( ([featureName, privileges])=> privileges.map( @@ -88,7 +92,7 @@ export function initPutRolesApi( }), {}); const featureSchema = Joi.object(featureObject); return Joi.object({ - minimum: Joi.string().valid(Object.keys(privileges.global)), + minimum: Joi.array().items(Joi.string().valid(Object.keys(privileges.global))), feature: featureSchema, }); }; @@ -96,7 +100,7 @@ export function initPutRolesApi( const getSpaceSchema = () => { const privileges = authorization.privileges.get(); return Joi.object().pattern(/^[a-z0-9_-]+$/, Joi.object({ - minimum: Joi.string().valid(Object.keys(privileges.space)), + minimum: Joi.array().items(Joi.string().valid(Object.keys(privileges.space))), feature: Joi.object(Object.entries(privileges.features).reduce((acc, [feature, featurePrivileges]) => ({ ...acc, [feature]: Joi.array().items(Joi.string().valid(Object.keys(featurePrivileges))) From 6427cdb484d7b1bdb2281427f7860c5aa3fd431a Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 4 Dec 2018 15:14:29 -0800 Subject: [PATCH 03/18] Updating get route to naively support the new structure --- .../lib/authorization/privilege_serializer.ts | 31 +++++++++- .../server/routes/api/public/roles/get.js | 56 ++++++++++++++++--- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts index 537172c69c625..d51a268fbf2f3 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts @@ -7,8 +7,24 @@ const featurePrefix = 'feature_'; const spacePrefix = 'space_'; const reservedPrivilegeNames = ['all', 'read']; +const spaceReservedPrivilegeNames = reservedPrivilegeNames.map( + privilegeName => `${spacePrefix}${privilegeName}` +); + +interface FeaturePrivilege { + featureId: string; + privilege: string; +} export class PrivilegeSerializer { + public static isReservedGlobalPrivilege(privilegeName: string) { + return reservedPrivilegeNames.includes(privilegeName); + } + + public static isReservedSpacePrivilege(privilegeName: string) { + return spaceReservedPrivilegeNames.includes(privilegeName); + } + public static serializeGlobalReservedPrivilege(privilegeName: string) { if (!reservedPrivilegeNames.includes(privilegeName)) { throw new Error('Unrecognized global reserved privilege'); @@ -26,7 +42,7 @@ export class PrivilegeSerializer { } public static serializeFeaturePrivilege(featureName: string, privilegeName: string) { - return `${featurePrefix}${featureName}_${privilegeName}`; + return `${featurePrefix}${featureName}.${privilegeName}`; } public static serializePrivilegeAssignedGlobally(privilege: string) { @@ -45,6 +61,19 @@ export class PrivilegeSerializer { return `${featurePrefix}${privilege}`; } + public static deserializeFeaturePrivilege(privilege: string): FeaturePrivilege { + if (privilege.indexOf(featurePrefix) !== 0) { + throw new Error(`privilege '${privilege}' was expected to start with '${featurePrefix}'`); + } + + const withoutPrefix = privilege.slice(featurePrefix.length); + const indexOfFirstUnderscore = withoutPrefix.indexOf('.'); + return { + featureId: withoutPrefix.slice(0, indexOfFirstUnderscore), + privilege: withoutPrefix.slice(indexOfFirstUnderscore + 1), + }; + } + public static deserializePrivilegeAssignedGlobally(privilege: string) { if (privilege.startsWith(featurePrefix)) { return privilege.slice(featurePrefix.length); diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.js b/x-pack/plugins/security/server/routes/api/public/roles/get.js index 8c32763bb74f7..968e634f19e1d 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.js @@ -21,21 +21,59 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, return resourcePrivileges.reduce((result, { resource, privileges }) => { if (resource === GLOBAL_RESOURCE) { - result.global = _.uniq([ - ...result.global, - ...privileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedGlobally(privilege)) - ]); + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isReservedGlobalPrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isReservedGlobalPrivilege(privilege)); + + result.global = { + minimum: [ + ...result.global.minimum, + ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedGlobally(privilege)) + ], + features: { + ...result.global.features, + ...featurePrivileges.reduce((acc, privilege) => { + const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); + return { + ...acc, + [featurePrivilege.featureId]: [ + ...acc[featurePrivilege.privilege] || [], + featurePrivilege.privilege + ] + }; + }, {}) + } + }; return result; } + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isReservedSpacePrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isReservedSpacePrivilege(privilege)); const spaceId = ResourceSerializer.deserializeSpaceResource(resource); - result.space[spaceId] = _.uniq([ - ...result.space[spaceId] || [], - ...privileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedAtSpace(privilege)) - ]); + result.space[spaceId] = { + minimum: [ + ...result[spaceId] ? result[spaceId].minimum || [] : [], + ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedAtSpace(privilege)) + ], + features: { + ...result[spaceId] ? result[spaceId].features || [] : [], + ...featurePrivileges.reduce((acc, privilege) => { + const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); + return { + ...acc, + [featurePrivilege.featureId]: [ + ...acc[featurePrivilege.privilege] || [], + featurePrivilege.privilege + ] + }; + }, {}) + } + }; return result; }, { - global: [], + global: { + minimum: [], + features: {}, + }, space: {}, }); }; From 10c2b861416e2e889a54340ac907a6309996b30e Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 4 Dec 2018 15:41:59 -0800 Subject: [PATCH 04/18] Renaming and removing some serialized methods --- .../privilege_serializer.test.ts.snap | 8 +- .../privilege_serializer.test.ts | 70 +++++----------- .../lib/authorization/privilege_serializer.ts | 79 ++++++------------- .../authorization/privileges_serializer.ts | 4 +- .../server/routes/api/public/roles/get.js | 12 +-- .../server/routes/api/public/roles/put.js | 4 +- 6 files changed, 58 insertions(+), 119 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap index 21b8108923a20..f1ab376e12d54 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap @@ -1,11 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`#deserializePrivilegeAssignedAtSpace throws Error if not prefixed with space_ 1`] = `"Unable to deserialize all, should have started with space_ or feature_"`; +exports[`#deserializeSpaceMinimumPrivilege throws Error if not prefixed with space_ 1`] = `"Unable to deserialize all, should have started with space_ or feature_"`; -exports[`#deserializePrivilegeAssignedAtSpace throws Error if prefixed with space_ but not a reserved privilege 1`] = `"Unrecognized privilege assigned at space"`; +exports[`#deserializeSpaceMinimumPrivilege throws Error if prefixed with space_ but not a reserved privilege 1`] = `"Unrecognized privilege assigned at space"`; -exports[`#deserializePrivilegeAssignedGlobally throws Error if not prefixed with feature_ and isn't a reserved privilege 1`] = `"Unrecognized privilege assigned globally"`; +exports[`#deserializeGlobalMinimumPrivilege throws Error if not prefixed with feature_ and isn't a reserved privilege 1`] = `"Unrecognized privilege assigned globally"`; exports[`#serializeGlobalPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized global reserved privilege"`; -exports[`#serializeSpaceReservedPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized space reserved privilege"`; +exports[`#serializeSpaceMinimumPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized space reserved privilege"`; diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index 945ce33c49bfa..aa87c86138b28 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -9,35 +9,35 @@ import { PrivilegeSerializer } from './privilege_serializer'; describe('#serializeGlobalPrivilege', () => { test('throws Error if unrecognized privilege used', () => { expect(() => - PrivilegeSerializer.serializeGlobalReservedPrivilege('foo') + PrivilegeSerializer.serializeGlobalMinimumPrivilege('foo') ).toThrowErrorMatchingSnapshot(); }); test('returns all unmodified', () => { - const allResult = PrivilegeSerializer.serializeGlobalReservedPrivilege('all'); + const allResult = PrivilegeSerializer.serializeGlobalMinimumPrivilege('all'); expect(allResult).toBe('all'); }); test('returns read unmodified', () => { - const readResult = PrivilegeSerializer.serializeGlobalReservedPrivilege('read'); + const readResult = PrivilegeSerializer.serializeGlobalMinimumPrivilege('read'); expect(readResult).toBe('read'); }); }); -describe('#serializeSpaceReservedPrivilege', () => { +describe('#serializeSpaceMinimumPrivilege', () => { test('throws Error if unrecognized privilege used', () => { expect(() => - PrivilegeSerializer.serializeSpaceReservedPrivilege('foo') + PrivilegeSerializer.serializeSpaceMinimumPrivilege('foo') ).toThrowErrorMatchingSnapshot(); }); test('returns all prefixed with space_', () => { - const allResult = PrivilegeSerializer.serializeSpaceReservedPrivilege('all'); + const allResult = PrivilegeSerializer.serializeSpaceMinimumPrivilege('all'); expect(allResult).toBe('space_all'); }); test('returns read prefixed with space_', () => { - const readResult = PrivilegeSerializer.serializeSpaceReservedPrivilege('read'); + const readResult = PrivilegeSerializer.serializeSpaceMinimumPrivilege('read'); expect(readResult).toBe('space_read'); }); }); @@ -49,88 +49,54 @@ describe('#serializeFeaturePrivilege', () => { }); }); -describe('#serializePrivilegeAssignedGlobally', () => { - test(`returns 'all' when 'all' is provided`, () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedGlobally('all'); - expect(result).toBe('all'); - }); - - test(`returns 'read' when 'read' is provided`, () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedGlobally('read'); - expect(result).toBe('read'); - }); - - test('returns `feature_${privilege}` otherwise', () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedGlobally('foo'); - expect(result).toBe('feature_foo'); - }); -}); - -describe('#serializePrivilegeAssignedAtSpace', () => { - test(`returns 'space_all' when 'all' is provided`, () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedAtSpace('all'); - expect(result).toBe('space_all'); - }); - - test(`returns 'space_read' when 'read' is provided`, () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedAtSpace('read'); - expect(result).toBe('space_read'); - }); - - test('returns `feature_${privilege}` otherwise', () => { - const result = PrivilegeSerializer.serializePrivilegeAssignedAtSpace('foo'); - expect(result).toBe('feature_foo'); - }); -}); - -describe('#deserializePrivilegeAssignedGlobally', () => { +describe('#deserializeGlobalMinimumPrivilege', () => { test(`if prefixed with 'feature_' removes the prefix`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedGlobally('feature_foo'); + const result = PrivilegeSerializer.deserializeGlobalMinimumPrivilege('feature_foo'); expect(result).toBe('foo'); }); test(`throws Error if not prefixed with feature_ and isn't a reserved privilege`, () => { expect(() => - PrivilegeSerializer.deserializePrivilegeAssignedGlobally('foo') + PrivilegeSerializer.deserializeGlobalMinimumPrivilege('foo') ).toThrowErrorMatchingSnapshot(); }); test(`returns 'all' unprefixed if provided 'all'`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedGlobally('all'); + const result = PrivilegeSerializer.deserializeGlobalMinimumPrivilege('all'); expect(result).toBe('all'); }); test(`returns 'read' unprefixed if provided 'read'`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedGlobally('read'); + const result = PrivilegeSerializer.deserializeGlobalMinimumPrivilege('read'); expect(result).toBe('read'); }); }); -describe('#deserializePrivilegeAssignedAtSpace', () => { +describe('#deserializeSpaceMinimumPrivilege', () => { test(`if prefixed with 'feature_' removes the prefix`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedAtSpace('feature_foo'); + const result = PrivilegeSerializer.deserializeSpaceMinimumPrivilege('feature_foo'); expect(result).toBe('foo'); }); test(`throws Error if not prefixed with space_`, () => { expect(() => - PrivilegeSerializer.deserializePrivilegeAssignedAtSpace('all') + PrivilegeSerializer.deserializeSpaceMinimumPrivilege('all') ).toThrowErrorMatchingSnapshot(); }); test(`throws Error if prefixed with space_ but not a reserved privilege`, () => { expect(() => - PrivilegeSerializer.deserializePrivilegeAssignedAtSpace('space_foo') + PrivilegeSerializer.deserializeSpaceMinimumPrivilege('space_foo') ).toThrowErrorMatchingSnapshot(); }); test(`returns 'all' unprefixed if provided 'space_all'`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedAtSpace('space_all'); + const result = PrivilegeSerializer.deserializeSpaceMinimumPrivilege('space_all'); expect(result).toBe('all'); }); test(`returns 'read' unprefixed if provided 'space_read'`, () => { - const result = PrivilegeSerializer.deserializePrivilegeAssignedAtSpace('space_read'); + const result = PrivilegeSerializer.deserializeSpaceMinimumPrivilege('space_read'); expect(result).toBe('read'); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts index d51a268fbf2f3..e71c4729b0050 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts @@ -6,8 +6,8 @@ const featurePrefix = 'feature_'; const spacePrefix = 'space_'; -const reservedPrivilegeNames = ['all', 'read']; -const spaceReservedPrivilegeNames = reservedPrivilegeNames.map( +const minimumPrivilegeNames = ['all', 'read']; +const spaceMinimumPrivilegeNames = minimumPrivilegeNames.map( privilegeName => `${spacePrefix}${privilegeName}` ); @@ -17,25 +17,25 @@ interface FeaturePrivilege { } export class PrivilegeSerializer { - public static isReservedGlobalPrivilege(privilegeName: string) { - return reservedPrivilegeNames.includes(privilegeName); + public static isGlobalMinimumPrivilege(privilegeName: string) { + return minimumPrivilegeNames.includes(privilegeName); } - public static isReservedSpacePrivilege(privilegeName: string) { - return spaceReservedPrivilegeNames.includes(privilegeName); + public static isSpaceMinimumPrivilege(privilegeName: string) { + return spaceMinimumPrivilegeNames.includes(privilegeName); } - public static serializeGlobalReservedPrivilege(privilegeName: string) { - if (!reservedPrivilegeNames.includes(privilegeName)) { - throw new Error('Unrecognized global reserved privilege'); + public static serializeGlobalMinimumPrivilege(privilegeName: string) { + if (!minimumPrivilegeNames.includes(privilegeName)) { + throw new Error('Unrecognized global minimum privilege'); } return privilegeName; } - public static serializeSpaceReservedPrivilege(privilegeName: string) { - if (!reservedPrivilegeNames.includes(privilegeName)) { - throw new Error('Unrecognized space reserved privilege'); + public static serializeSpaceMinimumPrivilege(privilegeName: string) { + if (!minimumPrivilegeNames.includes(privilegeName)) { + throw new Error('Unrecognized space minimum privilege'); } return `${spacePrefix}${privilegeName}`; @@ -45,63 +45,36 @@ export class PrivilegeSerializer { return `${featurePrefix}${featureName}.${privilegeName}`; } - public static serializePrivilegeAssignedGlobally(privilege: string) { - if (reservedPrivilegeNames.includes(privilege)) { - return privilege; - } - - return `${featurePrefix}${privilege}`; - } - - public static serializePrivilegeAssignedAtSpace(privilege: string) { - if (reservedPrivilegeNames.includes(privilege)) { - return `${spacePrefix}${privilege}`; - } - - return `${featurePrefix}${privilege}`; - } - public static deserializeFeaturePrivilege(privilege: string): FeaturePrivilege { if (privilege.indexOf(featurePrefix) !== 0) { throw new Error(`privilege '${privilege}' was expected to start with '${featurePrefix}'`); } const withoutPrefix = privilege.slice(featurePrefix.length); - const indexOfFirstUnderscore = withoutPrefix.indexOf('.'); + const parts = withoutPrefix.split('.'); + if (parts.length !== 2) { + throw new Error(`Unable to deserialize feature privilege '${privilege}'`); + } + return { - featureId: withoutPrefix.slice(0, indexOfFirstUnderscore), - privilege: withoutPrefix.slice(indexOfFirstUnderscore + 1), + featureId: parts[0], + privilege: parts[1], }; } - public static deserializePrivilegeAssignedGlobally(privilege: string) { - if (privilege.startsWith(featurePrefix)) { - return privilege.slice(featurePrefix.length); - } - - if (reservedPrivilegeNames.includes(privilege)) { + public static deserializeGlobalMinimumPrivilege(privilege: string) { + if (minimumPrivilegeNames.includes(privilege)) { return privilege; } - throw new Error('Unrecognized privilege assigned globally'); + throw new Error('Unrecognized global minimum privilege'); } - public static deserializePrivilegeAssignedAtSpace(privilege: string) { - if (privilege.startsWith(featurePrefix)) { - return privilege.slice(featurePrefix.length); - } - - if (!privilege.startsWith(spacePrefix)) { - throw new Error( - `Unable to deserialize ${privilege}, should have started with ${spacePrefix} or ${featurePrefix}` - ); - } - - const privilegeName = privilege.slice(spacePrefix.length); - if (reservedPrivilegeNames.includes(privilegeName)) { - return privilegeName; + public static deserializeSpaceMinimumPrivilege(privilege: string) { + if (!spaceMinimumPrivilegeNames.includes(privilege)) { + throw new Error('Unrecognized space minimum privilege'); } - throw new Error('Unrecognized privilege assigned at space'); + return privilege.slice(spacePrefix.length); } } diff --git a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts index 4b8afd04a8e19..73521ee9810f9 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts @@ -30,7 +30,7 @@ export const serializePrivileges = ( [application]: { ...Object.entries(privilegeMap.global).reduce( (acc, [privilegeName, privilegeActions]) => { - const name = PrivilegeSerializer.serializeGlobalReservedPrivilege(privilegeName); + const name = PrivilegeSerializer.serializeGlobalMinimumPrivilege(privilegeName); acc[name] = { application, name: privilegeName, @@ -43,7 +43,7 @@ export const serializePrivileges = ( ), ...Object.entries(privilegeMap.space).reduce( (acc, [privilegeName, privilegeActions]) => { - const name = PrivilegeSerializer.serializeSpaceReservedPrivilege(privilegeName); + const name = PrivilegeSerializer.serializeSpaceMinimumPrivilege(privilegeName); acc[name] = { application, name, diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.js b/x-pack/plugins/security/server/routes/api/public/roles/get.js index 968e634f19e1d..d56d2f5745804 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.js @@ -21,13 +21,13 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, return resourcePrivileges.reduce((result, { resource, privileges }) => { if (resource === GLOBAL_RESOURCE) { - const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isReservedGlobalPrivilege(privilege)); - const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isReservedGlobalPrivilege(privilege)); + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)); result.global = { minimum: [ ...result.global.minimum, - ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedGlobally(privilege)) + ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeGlobalMinimumPrivilege(privilege)) ], features: { ...result.global.features, @@ -46,13 +46,13 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, return result; } - const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isReservedSpacePrivilege(privilege)); - const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isReservedSpacePrivilege(privilege)); + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); const spaceId = ResourceSerializer.deserializeSpaceResource(resource); result.space[spaceId] = { minimum: [ ...result[spaceId] ? result[spaceId].minimum || [] : [], - ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializePrivilegeAssignedAtSpace(privilege)) + ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeSpaceMinimumPrivilege(privilege)) ], features: { ...result[spaceId] ? result[spaceId].features || [] : [], diff --git a/x-pack/plugins/security/server/routes/api/public/roles/put.js b/x-pack/plugins/security/server/routes/api/public/roles/put.js index 46910118a02e4..fb0b287ca9c2d 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/put.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/put.js @@ -24,7 +24,7 @@ export function initPutRolesApi( kibanaApplicationPrivileges.push({ privileges: [ ...kibanaPrivileges.global.minimum ? kibanaPrivileges.global.minimum.map( - privilege => PrivilegeSerializer.serializeGlobalReservedPrivilege(privilege) + privilege => PrivilegeSerializer.serializeGlobalMinimumPrivilege(privilege) ) : [], ...kibanaPrivileges.global.feature ? flatten( Object.entries(kibanaPrivileges.global.feature).map( @@ -44,7 +44,7 @@ export function initPutRolesApi( kibanaApplicationPrivileges.push({ privileges: [ ...spacePrivileges.minimum ? spacePrivileges.minimum.map( - privilege => PrivilegeSerializer.serializeSpaceReservedPrivilege(privilege) + privilege => PrivilegeSerializer.serializeSpaceMinimumPrivilege(privilege) ) : [], ...spacePrivileges.feature ? flatten( Object.entries(spacePrivileges.feature).map( From 414be1d56350d6b1438880e5d5a41abc5f026a77 Mon Sep 17 00:00:00 2001 From: kobelb Date: Tue, 4 Dec 2018 15:54:54 -0800 Subject: [PATCH 05/18] Updating Role PUT api tests --- .../api_integration/apis/security/roles.js | 52 ++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/x-pack/test/api_integration/apis/security/roles.js b/x-pack/test/api_integration/apis/security/roles.js index 7e05e11944a0c..1c8d46dc53585 100644 --- a/x-pack/test/api_integration/apis/security/roles.js +++ b/x-pack/test/api_integration/apis/security/roles.js @@ -42,8 +42,23 @@ export default function ({ getService }) { run_as: ['watcher_user'], }, kibana: { - global: ['all', 'read'], - space: {} + global: { + minimum: ["read"], + feature: { + dashboard: ["read"], + dev_tools: ["all"], + } + }, + space: { + marketing: { + minimum: ["all"], + feature: { + dashboard: ["read"], + discover: ["all"], + ml: ["all"] + } + } + } } }) .expect(204); @@ -66,8 +81,13 @@ export default function ({ getService }) { applications: [ { application: 'kibana-.kibana', - privileges: ['all', 'read'], + privileges: ['read', 'feature_dashboard.read', 'feature_dev_tools.all'], resources: ['*'], + }, + { + application: 'kibana-.kibana', + privileges: ['space_all', 'feature_dashboard.read', 'feature_discover.all', 'feature_ml.all'], + resources: ['space:marketing'], } ], run_as: ['watcher_user'], @@ -140,8 +160,23 @@ export default function ({ getService }) { run_as: ['watcher_user'], }, kibana: { - global: ['all', 'read'], - space: {} + global: { + minimum: ["read"], + feature: { + dashboard: ["read"], + dev_tools: ["all"], + } + }, + space: { + marketing: { + minimum: ["all"], + feature: { + dashboard: ["read"], + discover: ["all"], + ml: ["all"] + } + } + } } }) .expect(204); @@ -164,9 +199,14 @@ export default function ({ getService }) { applications: [ { application: 'kibana-.kibana', - privileges: ['all', 'read'], + privileges: ['read', 'feature_dashboard.read', 'feature_dev_tools.all'], resources: ['*'], }, + { + application: 'kibana-.kibana', + privileges: ['space_all', 'feature_dashboard.read', 'feature_discover.all', 'feature_ml.all'], + resources: ['space:marketing'], + }, { application: 'logstash-default', privileges: ['logstash-privilege'], From f99ce9bb7f9b9885903e819a1a6bd8e008f9ecae Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 5 Dec 2018 07:43:14 -0800 Subject: [PATCH 06/18] Fixing PUT jest tests --- .../routes/api/public/roles/put.test.js | 335 +++++++++++++----- 1 file changed, 241 insertions(+), 94 deletions(-) diff --git a/x-pack/plugins/security/server/routes/api/public/roles/put.test.js b/x-pack/plugins/security/server/routes/api/public/roles/put.test.js index 110701cbf1751..8bc2cf28d2f77 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/put.test.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/put.test.js @@ -142,96 +142,212 @@ describe('PUT role', () => { }, }); - putRoleTest(`only allows known Kibana global or feature privileges`, { - name: 'foo-role', - payload: { - kibana: { - global: ['foo'] - } - }, - asserts: { - statusCode: 400, - result: { - error: 'Bad Request', - //eslint-disable-next-line max-len - message: `child \"kibana\" fails because [child \"global\" fails because [\"global\" at position 0 fails because [\"0\" must be one of [all, read, foo_foo-privilege-1, foo_foo-privilege-2, bar_bar-privilege-1, bar_bar-privilege-2]]]]`, + describe('global', () => { + putRoleTest(`only allows known Kibana global minimum privileges`, { + name: 'foo-role', + payload: { + kibana: { + global: { + minimum: ["foo"] + } + } + }, + asserts: { statusCode: 400, - validation: { - keys: ['kibana.global.0'], - source: 'payload', + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"global\" fails because [child \"minimum\" fails because [\"minimum\" at position 0 fails because [\"0\" must be one of [all, read]]]]]`, + statusCode: 400, + validation: { + keys: ['kibana.global.minimum.0'], + source: 'payload', + }, }, }, - }, - }); + }); - putRoleTest(`only allows known Kibana space or feature privileges`, { - name: 'foo-role', - payload: { - kibana: { - space: { - quz: ['foo'] + putRoleTest(`only allows known Kibana global features`, { + name: 'foo-role', + payload: { + kibana: { + global: { + feature: { + baz: ['all'] + } + } } - } - }, - asserts: { - statusCode: 400, - result: { - error: 'Bad Request', - //eslint-disable-next-line max-len - message: `child \"kibana\" fails because [child \"space\" fails because [child \"quz\" fails because [\"quz\" at position 0 fails because [\"0\" must be one of [all, read, foo_foo-privilege-1, foo_foo-privilege-2, bar_bar-privilege-1, bar_bar-privilege-2]]]]]`, + }, + asserts: { statusCode: 400, - validation: { - keys: ['kibana.space.quz.0'], - source: 'payload', + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"global\" fails because [child \"feature\" fails because [\"baz\" is not allowed]]]`, + statusCode: 400, + validation: { + keys: ['kibana.global.feature.baz'], + source: 'payload', + }, }, }, - }, - }); + }); - putRoleTest(`doesn't allow * space ID`, { - name: 'foo-role', - payload: { - kibana: { - space: { - '*': ['test-space-privilege-1'] + putRoleTest(`only allows known Kibana global feature privileges`, { + name: 'foo-role', + payload: { + kibana: { + global: { + feature: { + foo: ['foo-privilege-3'] + } + } } - } - }, - asserts: { - statusCode: 400, - result: { - error: 'Bad Request', - message: `child \"kibana\" fails because [child \"space\" fails because [\"*\" is not allowed]]`, + }, + asserts: { statusCode: 400, - validation: { - keys: ['kibana.space.*'], - source: 'payload', + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"global\" fails because [child \"feature\" fails because [child \"foo\" fails because [\"foo\" at position 0 fails because [\"0\" must be one of [foo-privilege-1, foo-privilege-2]]]]]]`, + statusCode: 400, + validation: { + keys: ['kibana.global.feature.foo.0'], + source: 'payload', + }, }, }, - }, + }); }); - putRoleTest(`doesn't allow * in a space ID`, { - name: 'foo-role', - payload: { - kibana: { - space: { - 'foo-*': ['test-space-privilege-1'] + describe('space', () => { + putRoleTest(`doesn't allow * space ID`, { + name: 'foo-role', + payload: { + kibana: { + space: { + '*': ['test-space-privilege-1'] + } } - } - }, - asserts: { - statusCode: 400, - result: { - error: 'Bad Request', - message: `child \"kibana\" fails because [child \"space\" fails because [\"foo-*\" is not allowed]]`, + }, + asserts: { statusCode: 400, - validation: { - keys: ['kibana.space.foo-*'], - source: 'payload', + result: { + error: 'Bad Request', + message: `child \"kibana\" fails because [child \"space\" fails because [\"*\" is not allowed]]`, + statusCode: 400, + validation: { + keys: ['kibana.space.*'], + source: 'payload', + }, }, }, - }, + }); + + putRoleTest(`doesn't allow * in a space ID`, { + name: 'foo-role', + payload: { + kibana: { + space: { + 'foo-*': ['test-space-privilege-1'] + } + } + }, + asserts: { + statusCode: 400, + result: { + error: 'Bad Request', + message: `child \"kibana\" fails because [child \"space\" fails because [\"foo-*\" is not allowed]]`, + statusCode: 400, + validation: { + keys: ['kibana.space.foo-*'], + source: 'payload', + }, + }, + }, + }); + + putRoleTest(`only allows known Kibana space minimum privileges`, { + name: 'foo-role', + payload: { + kibana: { + space: { + marketing: { + minimum: ["foo"] + } + } + } + }, + asserts: { + statusCode: 400, + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"space\" fails because [child \"marketing\" fails because [child \"minimum\" fails because [\"minimum\" at position 0 fails because [\"0\" must be one of [all, read]]]]]]`, + statusCode: 400, + validation: { + keys: ['kibana.space.marketing.minimum.0'], + source: 'payload', + }, + }, + }, + }); + + putRoleTest(`only allows known Kibana global features`, { + name: 'foo-role', + payload: { + kibana: { + space: { + marketing: { + feature: { + baz: ['all'] + } + } + } + } + }, + asserts: { + statusCode: 400, + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"space\" fails because [child \"marketing\" fails because [child \"feature\" fails because [\"baz\" is not allowed]]]]`, + statusCode: 400, + validation: { + keys: ['kibana.space.marketing.feature.baz'], + source: 'payload', + }, + }, + }, + }); + + putRoleTest(`only allows known Kibana global feature privileges`, { + name: 'foo-role', + payload: { + kibana: { + space: { + marketing: { + feature: { + foo: ['foo-privilege-3'] + } + } + } + } + }, + asserts: { + statusCode: 400, + result: { + error: 'Bad Request', + //eslint-disable-next-line max-len + message: `child \"kibana\" fails because [child \"space\" fails because [child \"marketing\" fails because [child \"feature\" fails because [child \"foo\" fails because [\"foo\" at position 0 fails because [\"0\" must be one of [foo-privilege-1, foo-privilege-2]]]]]]]`, + statusCode: 400, + validation: { + keys: ['kibana.space.marketing.feature.foo.0'], + source: 'payload', + }, + }, + }, + }); }); putRoleTest(`returns result of routePreCheckLicense`, { @@ -298,11 +414,27 @@ describe('PUT role', () => { run_as: ['test-run-as-1', 'test-run-as-2'], }, kibana: { - global: ['all', 'read', 'foo_foo-privilege-1', 'foo_foo-privilege-2', 'bar_bar-privilege-1', 'bar_bar-privilege-2'], - space: { - 'test-space-1': ['all', 'read', 'bar_bar-privilege-1', 'bar_bar-privilege-2'], - 'test-space-2': ['all', 'read', 'foo_foo-privilege-1', 'foo_foo-privilege-2'], + global: { + minimum: ['all', 'read'], + feature: { + foo: ['foo-privilege-1', 'foo-privilege-2'], + bar: ['bar-privilege-1', 'bar-privilege-2'] + } }, + space: { + 'test-space-1': { + minimum: ['all', 'read'], + feature: { + bar: ['bar-privilege-1', 'bar-privilege-2'] + } + }, + 'test-space-2': { + minimum: ['all', 'read'], + feature: { + foo: ['foo-privilege-1', 'foo-privilege-2'], + } + } + } }, }, preCheckLicenseImpl: defaultPreCheckLicenseImpl, @@ -321,10 +453,10 @@ describe('PUT role', () => { privileges: [ 'all', 'read', - 'feature_foo_foo-privilege-1', - 'feature_foo_foo-privilege-2', - 'feature_bar_bar-privilege-1', - 'feature_bar_bar-privilege-2', + 'feature_foo.foo-privilege-1', + 'feature_foo.foo-privilege-2', + 'feature_bar.bar-privilege-1', + 'feature_bar.bar-privilege-2', ], resources: [GLOBAL_RESOURCE], }, @@ -333,8 +465,8 @@ describe('PUT role', () => { privileges: [ 'space_all', 'space_read', - 'feature_bar_bar-privilege-1', - 'feature_bar_bar-privilege-2', + 'feature_bar.bar-privilege-1', + 'feature_bar.bar-privilege-2', ], resources: ['space:test-space-1'] }, @@ -343,8 +475,8 @@ describe('PUT role', () => { privileges: [ 'space_all', 'space_read', - 'feature_foo_foo-privilege-1', - 'feature_foo_foo-privilege-2', + 'feature_foo.foo-privilege-1', + 'feature_foo.foo-privilege-2', ], resources: ['space:test-space-2'] }, @@ -397,10 +529,26 @@ describe('PUT role', () => { run_as: ['test-run-as-1', 'test-run-as-2'], }, kibana: { - global: ['all', 'foo_foo-privilege-1', 'bar_bar-privilege-1'], + global: { + minimum: ['all'], + feature: { + foo: ['foo-privilege-1'], + bar: ['bar-privilege-1'] + } + }, space: { - 'test-space-1': ['all', 'foo_foo-privilege-2'], - 'test-space-2': ['read', 'bar_bar-privilege-2'], + 'test-space-1': { + minimum: ['all'], + feature: { + foo: ['foo-privilege-2'] + } + }, + 'test-space-2': { + minimum: ['read'], + feature: { + bar: ['bar-privilege-2'] + } + } } }, }, @@ -451,8 +599,8 @@ describe('PUT role', () => { application, privileges: [ 'all', - 'feature_foo_foo-privilege-1', - 'feature_bar_bar-privilege-1' + 'feature_foo.foo-privilege-1', + 'feature_bar.bar-privilege-1' ], resources: [GLOBAL_RESOURCE], }, @@ -460,7 +608,7 @@ describe('PUT role', () => { application, privileges: [ 'space_all', - 'feature_foo_foo-privilege-2' + 'feature_foo.foo-privilege-2' ], resources: ['space:test-space-1'] }, @@ -468,7 +616,7 @@ describe('PUT role', () => { application, privileges: [ 'space_read', - 'feature_bar_bar-privilege-2', + 'feature_bar.bar-privilege-2', ], resources: ['space:test-space-2'] }, @@ -521,10 +669,9 @@ describe('PUT role', () => { run_as: ['test-run-as-1', 'test-run-as-2'], }, kibana: { - global: [ - 'all', - 'read', - ], + global: { + minimum: ['all', 'read'] + }, }, }, preCheckLicenseImpl: defaultPreCheckLicenseImpl, From 9a2c3ddebf76e87a2522cf74cef3f6df2a588b7e Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 5 Dec 2018 09:10:27 -0800 Subject: [PATCH 07/18] Fixing GET tests --- .../server/routes/api/public/roles/get.js | 58 +- .../routes/api/public/roles/get.test.js | 1150 ++++++++++++++--- 2 files changed, 1011 insertions(+), 197 deletions(-) diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.js b/x-pack/plugins/security/server/routes/api/public/roles/get.js index d56d2f5745804..24994298d2479 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.js @@ -25,48 +25,42 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)); result.global = { - minimum: [ + minimum: _.uniq([ ...result.global.minimum, ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeGlobalMinimumPrivilege(privilege)) - ], - features: { - ...result.global.features, - ...featurePrivileges.reduce((acc, privilege) => { - const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); - return { - ...acc, - [featurePrivilege.featureId]: [ - ...acc[featurePrivilege.privilege] || [], - featurePrivilege.privilege - ] - }; - }, {}) - } + ]), + features: featurePrivileges.reduce((acc, privilege) => { + const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); + return { + ...acc, + [featurePrivilege.featureId]: _.uniq([ + ...acc[featurePrivilege.featureId] || [], + featurePrivilege.privilege + ]) + }; + }, result.global.features) }; return result; } + const spaceId = ResourceSerializer.deserializeSpaceResource(resource); const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); - const spaceId = ResourceSerializer.deserializeSpaceResource(resource); result.space[spaceId] = { - minimum: [ - ...result[spaceId] ? result[spaceId].minimum || [] : [], + minimum: _.uniq([ + ...result.space[spaceId] ? result.space[spaceId].minimum || [] : [], ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeSpaceMinimumPrivilege(privilege)) - ], - features: { - ...result[spaceId] ? result[spaceId].features || [] : [], - ...featurePrivileges.reduce((acc, privilege) => { - const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); - return { - ...acc, - [featurePrivilege.featureId]: [ - ...acc[featurePrivilege.privilege] || [], - featurePrivilege.privilege - ] - }; - }, {}) - } + ]), + features: featurePrivileges.reduce((acc, privilege) => { + const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); + return { + ...acc, + [featurePrivilege.featureId]: _.uniq([ + ...acc[featurePrivilege.featureId] || [], + featurePrivilege.privilege + ]) + }; + }, result.space[spaceId] ? result.space[spaceId].features || {} : {}) }; return result; }, { diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.test.js b/x-pack/plugins/security/server/routes/api/public/roles/get.test.js index 26d6288aa8eb4..c93b25c5c4f1c 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.test.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.test.js @@ -162,7 +162,10 @@ describe('GET roles', () => { run_as: ['other_user'], }, kibana: { - global: [], + global: { + minimum: [], + features: {}, + }, space: {}, }, _unrecognized_applications: [], @@ -171,116 +174,520 @@ describe('GET roles', () => { }, }); - getRolesTest(`transforms matching applications with * resource to kibana global privileges`, { - callWithRequestImpl: async () => ({ - first_role: { - cluster: [], - indices: [], - applications: [ - { - application, - privileges: ['read'], - resources: ['*'], + describe('global', () => { + getRolesTest(`transforms matching applications with * resource to kibana global minimum privileges`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['read'], + resources: ['*'], + }, + { + application, + privileges: ['all'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, }, + transient_metadata: { + enabled: true, + }, + }, + }), + asserts: { + statusCode: 200, + result: [ { - application, - privileges: ['all'], - resources: ['*'], + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: ['read', 'all'], + features: {} + }, + space: {}, + }, + _unrecognized_applications: [], }, ], - run_as: [], - metadata: { - _reserved: true, - }, - transient_metadata: { - enabled: true, + }, + }); + + getRolesTest(`transforms matching applications with * resource to kibana global minimum privileges, eliminating duplicates`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['read'], + resources: ['*'], + }, + { + application, + privileges: ['all', 'read'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: ['read', 'all'], + features: {} + }, + space: {}, + }, + _unrecognized_applications: [], + }, + ], }, - }), - asserts: { - statusCode: 200, - result: [ - { - name: 'first_role', + }); + + getRolesTest(`transforms matching applications with * resource to kibana global feature privileges`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['*'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-3'], + resources: ['*'], + }, + ], + run_as: [], metadata: { _reserved: true, }, transient_metadata: { enabled: true, }, - elasticsearch: { - cluster: [], - indices: [], - run_as: [], + }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + space: {}, + }, + _unrecognized_applications: [], }, - kibana: { - global: ['read', 'all'], - space: {}, + ], + }, + }); + + getRolesTest(`transforms matching applications with * resource to kibana global feature privileges, eliminating duplicates`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['*'], + }, + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-3'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, }, - _unrecognized_applications: [], }, - ], - }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + space: {}, + }, + _unrecognized_applications: [], + }, + ], + }, + }); }); - getRolesTest(`transforms matching applications with space resources to kibana space privileges`, { - callWithRequestImpl: async () => ({ - first_role: { - cluster: [], - indices: [], - applications: [ - { - application, - privileges: ['space_read'], - resources: ['space:marketing'], + describe('space', () => { + getRolesTest(`transforms matching applications with space resources to kibana space minimum privileges`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['space_read'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_read'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, }, - { - application, - privileges: ['space_all'], - resources: ['space:marketing'], + transient_metadata: { + enabled: true, }, + }, + }), + asserts: { + statusCode: 200, + result: [ { - application, - privileges: ['space_read'], - resources: ['space:engineering'], + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: ['read', 'all'], + features: {} + }, + engineering: { + minimum: ['read'], + features: {} + } + } + }, + _unrecognized_applications: [], }, ], - run_as: [], - metadata: { - _reserved: true, - }, - transient_metadata: { - enabled: true, + }, + }); + + getRolesTest(`transforms matching applications with space resources to kibana space minimum privileges, eliminating duplicates`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['space_read', 'space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_read'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: ['read', 'all'], + features: {} + }, + engineering: { + minimum: ['read'], + features: {} + } + } + }, + _unrecognized_applications: [], + }, + ], }, - }), - asserts: { - statusCode: 200, - result: [ - { - name: 'first_role', + }); + + getRolesTest(`transforms matching applications with space resources to kibana space feature privileges`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-3'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-1'], + resources: ['space:engineering'], + }, + ], + run_as: [], metadata: { _reserved: true, }, transient_metadata: { enabled: true, }, - elasticsearch: { - cluster: [], - indices: [], - run_as: [], + }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + engineering: { + minimum: [], + features: { + foo: ['foo-privilege-1'] + } + } + } + }, + _unrecognized_applications: [], }, - kibana: { - global: [], - space: { - marketing: ['read', 'all'], - engineering: ['read'], - } + ], + }, + }); + + getRolesTest(`transforms matching applications with space resources to kibana space feature privileges, eliminating duplcates`, { + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-3'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-1'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, }, - _unrecognized_applications: [], }, - ], - }, + }), + asserts: { + statusCode: 200, + result: [ + { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + engineering: { + minimum: [], + features: { + foo: ['foo-privilege-1'] + } + } + } + }, + _unrecognized_applications: [], + }, + ], + }, + }); }); getRolesTest(`ignores empty resources even though this shouldn't happen`, { @@ -321,7 +728,10 @@ describe('GET roles', () => { run_as: [], }, kibana: { - global: [], + global: { + minimum: [], + features: {}, + }, space: {} }, _unrecognized_applications: [], @@ -368,7 +778,10 @@ describe('GET roles', () => { run_as: [], }, kibana: { - global: [], + global: { + minimum: [], + features: {} + }, space: {}, }, _unrecognized_applications: ['kibana-.another-kibana'] @@ -441,7 +854,7 @@ describe('GET role', () => { }); getRoleTest(`returns error from callWithRequest`, { - name: 'foo-role', + name: 'first_role', callWithRequestImpl: async () => { throw Boom.notAcceptable('test not acceptable message'); }, @@ -531,7 +944,10 @@ describe('GET role', () => { run_as: ['other_user'], }, kibana: { - global: [], + global: { + minimum: [], + features: {}, + }, space: {}, }, _unrecognized_applications: [], @@ -539,114 +955,512 @@ describe('GET role', () => { }, }); - getRoleTest(`transforms matching applications with * resource to kibana global privileges`, { - name: 'first_role', - callWithRequestImpl: async () => ({ - first_role: { - cluster: [], - indices: [], - applications: [ - { - application, - privileges: ['read'], - resources: ['*'], - }, - { - application, - privileges: ['all'], - resources: ['*'], + describe('global', () => { + getRoleTest(`transforms matching applications with * resource to kibana global minimum privileges`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['read'], + resources: ['*'], + }, + { + application, + privileges: ['all'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, }, - ], - run_as: [], - metadata: { - _reserved: true, }, - transient_metadata: { - enabled: true, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: ['read', 'all'], + features: {} + }, + space: {}, + }, + _unrecognized_applications: [], }, }, - }), - asserts: { - statusCode: 200, - result: { - name: 'first_role', - metadata: { - _reserved: true, + }); + + getRoleTest(`transforms matching applications with * resource to kibana global minimum privileges, eliminating duplicates`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['read'], + resources: ['*'], + }, + { + application, + privileges: ['all', 'read'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, - transient_metadata: { - enabled: true, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: ['read', 'all'], + features: {} + }, + space: {}, + }, + _unrecognized_applications: [], }, - elasticsearch: { + }, + }); + + getRoleTest(`transforms matching applications with * resource to kibana global feature privileges`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { cluster: [], indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['*'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-3'], + resources: ['*'], + }, + ], run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, - kibana: { - global: ['read', 'all'], - space: {}, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + space: {}, + }, + _unrecognized_applications: [], }, - _unrecognized_applications: [], }, - }, + }); + + getRoleTest(`transforms matching applications with * resource to kibana global feature privileges, eliminating duplicates`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['*'], + }, + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-3'], + resources: ['*'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + }, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + space: {}, + }, + _unrecognized_applications: [], + }, + }, + }); }); - getRoleTest(`transforms matching applications with space resource to kibana space privileges`, { - name: 'first_role', - callWithRequestImpl: async () => ({ - first_role: { - cluster: [], - indices: [], - applications: [ - { - application, - privileges: ['space_read'], - resources: ['space:marketing'], + describe('space', () => { + getRoleTest(`transforms matching applications with space resources to kibana space minimum privileges`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['space_read'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_read'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, }, - { - application, - privileges: ['space_all'], - resources: ['space:marketing'], + transient_metadata: { + enabled: true, }, - { - application, - privileges: ['space_read'], - resources: ['space:engineering'], + }, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, }, - ], - run_as: [], - metadata: { - _reserved: true, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: ['read', 'all'], + features: {} + }, + engineering: { + minimum: ['read'], + features: {} + } + } + }, + _unrecognized_applications: [], }, - transient_metadata: { - enabled: true, + }, + }); + + getRoleTest(`transforms matching applications with space resources to kibana space minimum privileges, eliminating duplicates`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['space_read', 'space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_all'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['space_read'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + }, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: ['read', 'all'], + features: {} + }, + engineering: { + minimum: ['read'], + features: {} + } + } + }, + _unrecognized_applications: [], }, }, - }), - asserts: { - statusCode: 200, - result: { - name: 'first_role', - metadata: { - _reserved: true, + }); + + getRoleTest(`transforms matching applications with space resources to kibana space feature privileges`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { + cluster: [], + indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-3'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-1'], + resources: ['space:engineering'], + }, + ], + run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, - transient_metadata: { - enabled: true, + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + engineering: { + minimum: [], + features: { + foo: ['foo-privilege-1'] + } + } + } + }, + _unrecognized_applications: [], }, - elasticsearch: { + }, + }); + + getRoleTest(`transforms matching applications with space resources to kibana space feature privileges, eliminating duplcates`, { + name: 'first_role', + callWithRequestImpl: async () => ({ + first_role: { cluster: [], indices: [], + applications: [ + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-2', 'feature_bar.bar-privilege-1'], + resources: ['space:marketing'], + }, + { + application, + privileges: ['feature_foo.foo-privilege-1', 'feature_foo.foo-privilege-3'], + resources: ['space:marketing'], + }, + { + application, + privileges: [ 'feature_foo.foo-privilege-1'], + resources: ['space:engineering'], + }, + ], run_as: [], + metadata: { + _reserved: true, + }, + transient_metadata: { + enabled: true, + }, }, - kibana: { - global: [], - space: { - marketing: ['read', 'all'], - engineering: ['read'] + }), + asserts: { + statusCode: 200, + result: { + name: 'first_role', + metadata: { + _reserved: true, }, + transient_metadata: { + enabled: true, + }, + elasticsearch: { + cluster: [], + indices: [], + run_as: [], + }, + kibana: { + global: { + minimum: [], + features: {} + }, + space: { + marketing: { + minimum: [], + features: { + foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], + bar: ['bar-privilege-1'] + } + }, + engineering: { + minimum: [], + features: { + foo: ['foo-privilege-1'] + } + } + } + }, + _unrecognized_applications: [], }, - _unrecognized_applications: [], }, - }, + }); }); getRoleTest(`ignores empty resources even though this shouldn't happen`, { @@ -687,8 +1501,11 @@ describe('GET role', () => { run_as: [], }, kibana: { - global: [], - space: {}, + global: { + minimum: [], + features: {}, + }, + space: {} }, _unrecognized_applications: [], }, @@ -733,10 +1550,13 @@ describe('GET role', () => { run_as: [], }, kibana: { - global: [], + global: { + minimum: [], + features: {} + }, space: {}, }, - _unrecognized_applications: ['kibana-.another-kibana'], + _unrecognized_applications: ['kibana-.another-kibana'] }, }, }); From af59591bb479e1214f9cd79db1d3903624dfec19 Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 5 Dec 2018 12:50:43 -0800 Subject: [PATCH 08/18] Updating PrivilegeSerializer tests --- .../privilege_serializer.test.ts.snap | 20 +++-- .../privilege_serializer.test.ts | 86 ++++++++++++++++--- .../lib/authorization/privilege_serializer.ts | 22 +++-- 3 files changed, 97 insertions(+), 31 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap index f1ab376e12d54..30411135b012c 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privilege_serializer.test.ts.snap @@ -1,11 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`#deserializeSpaceMinimumPrivilege throws Error if not prefixed with space_ 1`] = `"Unable to deserialize all, should have started with space_ or feature_"`; +exports[`#deserializeFeaturePrivilege throws error when deserializing feature_.privilege-1 1`] = `"Feature privilege 'feature_.privilege-1' didn't match pattern"`; -exports[`#deserializeSpaceMinimumPrivilege throws Error if prefixed with space_ but not a reserved privilege 1`] = `"Unrecognized privilege assigned at space"`; +exports[`#deserializeFeaturePrivilege throws error when deserializing feature_foo. 1`] = `"Feature privilege 'feature_foo.' didn't match pattern"`; -exports[`#deserializeGlobalMinimumPrivilege throws Error if not prefixed with feature_ and isn't a reserved privilege 1`] = `"Unrecognized privilege assigned globally"`; +exports[`#deserializeFeaturePrivilege throws error when deserializing feature_foo_privilege-1 1`] = `"Feature privilege 'feature_foo_privilege-1' didn't match pattern"`; -exports[`#serializeGlobalPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized global reserved privilege"`; +exports[`#deserializeFeaturePrivilege throws error when deserializing feature-foo.privilege-1 1`] = `"Feature privilege 'feature-foo.privilege-1' didn't match pattern"`; -exports[`#serializeSpaceMinimumPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized space reserved privilege"`; +exports[`#deserializeFeaturePrivilege throws error when deserializing foo_feature_foo.privilege-1 1`] = `"Feature privilege 'foo_feature_foo.privilege-1' didn't match pattern"`; + +exports[`#deserializeGlobalMinimumPrivilege throws Error if isn't a minimum privilege 1`] = `"Unrecognized global minimum privilege"`; + +exports[`#deserializeSpaceMinimumPrivilege throws Error if prefixed with space_ but not a reserved privilege 1`] = `"Unrecognized space minimum privilege"`; + +exports[`#deserializeSpaceMinimumPrivilege throws Error if provided 'all' 1`] = `"Unrecognized space minimum privilege"`; + +exports[`#serializeGlobalMinimumPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized global minimum privilege"`; + +exports[`#serializeSpaceMinimumPrivilege throws Error if unrecognized privilege used 1`] = `"Unrecognized space minimum privilege"`; diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index aa87c86138b28..f37d1846d5170 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -6,7 +6,37 @@ import { PrivilegeSerializer } from './privilege_serializer'; -describe('#serializeGlobalPrivilege', () => { +describe(`#isGlobalMinimumPrivilege`, () => { + ['all', 'read'].forEach(validValue => { + test(`returns true for '${validValue}'`, () => { + expect(PrivilegeSerializer.isGlobalMinimumPrivilege(validValue)).toBe(true); + }); + }); + + ['space_all', 'space_read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach( + validValue => { + test(`returns true for '${validValue}'`, () => { + expect(PrivilegeSerializer.isGlobalMinimumPrivilege(validValue)).toBe(false); + }); + } + ); +}); + +describe(`#isSpaceMinimumPrivilege`, () => { + ['space_all', 'space_read'].forEach(validValue => { + test(`returns true for '${validValue}'`, () => { + expect(PrivilegeSerializer.isSpaceMinimumPrivilege(validValue)).toBe(true); + }); + }); + + ['all', 'read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach(validValue => { + test(`returns true for '${validValue}'`, () => { + expect(PrivilegeSerializer.isSpaceMinimumPrivilege(validValue)).toBe(false); + }); + }); +}); + +describe('#serializeGlobalMinimumPrivilege', () => { test('throws Error if unrecognized privilege used', () => { expect(() => PrivilegeSerializer.serializeGlobalMinimumPrivilege('foo') @@ -43,19 +73,52 @@ describe('#serializeSpaceMinimumPrivilege', () => { }); describe('#serializeFeaturePrivilege', () => { - test('returns `feature_${featureName}_${privilegeName}`', () => { + test('returns `feature_${featureName}.${privilegeName}`', () => { const result = PrivilegeSerializer.serializeFeaturePrivilege('foo', 'bar'); - expect(result).toBe('feature_foo_bar'); + expect(result).toBe('feature_foo.bar'); }); }); -describe('#deserializeGlobalMinimumPrivilege', () => { - test(`if prefixed with 'feature_' removes the prefix`, () => { - const result = PrivilegeSerializer.deserializeGlobalMinimumPrivilege('feature_foo'); - expect(result).toBe('foo'); +describe('#deserializeFeaturePrivilege', () => { + [ + { + privilege: 'feature_foo.privilege-1', + expectedResult: { + featureId: 'foo', + privilege: 'privilege-1', + }, + }, + { + privilege: 'feature_foo_bar.foo_privilege-1', + expectedResult: { + featureId: 'foo_bar', + privilege: 'foo_privilege-1', + }, + }, + ].forEach(({ privilege, expectedResult }) => { + test(`deserializes '${privilege}' to ${JSON.stringify(expectedResult)}`, () => { + const result = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); + expect(result).toEqual(expectedResult); + }); + }); + + [ + 'feature-foo.privilege-1', // doesn't start with feature_ + 'foo_feature_foo.privilege-1', // also doesn't start with feature_ + 'feature_foo_privilege-1', // no '.' + 'feature_foo.', // has a '.' but nothing after it + 'feature_.privilege-1', // nothing before the '.' + ].forEach(privilege => { + test(`throws error when deserializing ${privilege}`, () => { + expect(() => + PrivilegeSerializer.deserializeFeaturePrivilege(privilege) + ).toThrowErrorMatchingSnapshot(); + }); }); +}); - test(`throws Error if not prefixed with feature_ and isn't a reserved privilege`, () => { +describe('#deserializeGlobalMinimumPrivilege', () => { + test(`throws Error if isn't a minimum privilege`, () => { expect(() => PrivilegeSerializer.deserializeGlobalMinimumPrivilege('foo') ).toThrowErrorMatchingSnapshot(); @@ -73,12 +136,7 @@ describe('#deserializeGlobalMinimumPrivilege', () => { }); describe('#deserializeSpaceMinimumPrivilege', () => { - test(`if prefixed with 'feature_' removes the prefix`, () => { - const result = PrivilegeSerializer.deserializeSpaceMinimumPrivilege('feature_foo'); - expect(result).toBe('foo'); - }); - - test(`throws Error if not prefixed with space_`, () => { + test(`throws Error if provided 'all'`, () => { expect(() => PrivilegeSerializer.deserializeSpaceMinimumPrivilege('all') ).toThrowErrorMatchingSnapshot(); diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts index e71c4729b0050..6567d9ff3fac2 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts @@ -10,6 +10,9 @@ const minimumPrivilegeNames = ['all', 'read']; const spaceMinimumPrivilegeNames = minimumPrivilegeNames.map( privilegeName => `${spacePrefix}${privilegeName}` ); +const deserializeFeaturePrivilegeRegexp = new RegExp( + `^${featurePrefix}([a-zA-Z0-9_-]+)\\.([a-zA-Z0-9_-]+)$` +); interface FeaturePrivilege { featureId: string; @@ -46,24 +49,19 @@ export class PrivilegeSerializer { } public static deserializeFeaturePrivilege(privilege: string): FeaturePrivilege { - if (privilege.indexOf(featurePrefix) !== 0) { - throw new Error(`privilege '${privilege}' was expected to start with '${featurePrefix}'`); - } - - const withoutPrefix = privilege.slice(featurePrefix.length); - const parts = withoutPrefix.split('.'); - if (parts.length !== 2) { - throw new Error(`Unable to deserialize feature privilege '${privilege}'`); + const match = privilege.match(deserializeFeaturePrivilegeRegexp); + if (!match) { + throw new Error(`Feature privilege '${privilege}' didn't match pattern`); } return { - featureId: parts[0], - privilege: parts[1], + featureId: match[1], + privilege: match[2], }; } public static deserializeGlobalMinimumPrivilege(privilege: string) { - if (minimumPrivilegeNames.includes(privilege)) { + if (PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)) { return privilege; } @@ -71,7 +69,7 @@ export class PrivilegeSerializer { } public static deserializeSpaceMinimumPrivilege(privilege: string) { - if (!spaceMinimumPrivilegeNames.includes(privilege)) { + if (!PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)) { throw new Error('Unrecognized space minimum privilege'); } From fcf2a3ba33f2ac8cd9b7fa550cdb96cbc52a93c6 Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 5 Dec 2018 13:26:43 -0800 Subject: [PATCH 09/18] Renaming features to feature for the GET, so we're consistent --- .../server/routes/api/public/roles/get.js | 10 +-- .../routes/api/public/roles/get.test.js | 76 +++++++++---------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.js b/x-pack/plugins/security/server/routes/api/public/roles/get.js index 24994298d2479..fd60df408c115 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.js @@ -29,7 +29,7 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, ...result.global.minimum, ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeGlobalMinimumPrivilege(privilege)) ]), - features: featurePrivileges.reduce((acc, privilege) => { + feature: featurePrivileges.reduce((acc, privilege) => { const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); return { ...acc, @@ -38,7 +38,7 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, featurePrivilege.privilege ]) }; - }, result.global.features) + }, result.global.feature) }; return result; } @@ -51,7 +51,7 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, ...result.space[spaceId] ? result.space[spaceId].minimum || [] : [], ...minimumPrivileges.map(privilege => PrivilegeSerializer.deserializeSpaceMinimumPrivilege(privilege)) ]), - features: featurePrivileges.reduce((acc, privilege) => { + feature: featurePrivileges.reduce((acc, privilege) => { const featurePrivilege = PrivilegeSerializer.deserializeFeaturePrivilege(privilege); return { ...acc, @@ -60,13 +60,13 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, featurePrivilege.privilege ]) }; - }, result.space[spaceId] ? result.space[spaceId].features || {} : {}) + }, result.space[spaceId] ? result.space[spaceId].feature || {} : {}) }; return result; }, { global: { minimum: [], - features: {}, + feature: {}, }, space: {}, }); diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.test.js b/x-pack/plugins/security/server/routes/api/public/roles/get.test.js index c93b25c5c4f1c..532b300ba921a 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.test.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.test.js @@ -164,7 +164,7 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {}, + feature: {}, }, space: {}, }, @@ -220,7 +220,7 @@ describe('GET roles', () => { kibana: { global: { minimum: ['read', 'all'], - features: {} + feature: {} }, space: {}, }, @@ -275,7 +275,7 @@ describe('GET roles', () => { kibana: { global: { minimum: ['read', 'all'], - features: {} + feature: {} }, space: {}, }, @@ -330,7 +330,7 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } @@ -388,7 +388,7 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } @@ -453,16 +453,16 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: ['read', 'all'], - features: {} + feature: {} }, engineering: { minimum: ['read'], - features: {} + feature: {} } } }, @@ -522,16 +522,16 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: ['read', 'all'], - features: {} + feature: {} }, engineering: { minimum: ['read'], - features: {} + feature: {} } } }, @@ -591,19 +591,19 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } }, engineering: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1'] } } @@ -665,19 +665,19 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } }, engineering: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1'] } } @@ -730,7 +730,7 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {}, + feature: {}, }, space: {} }, @@ -780,7 +780,7 @@ describe('GET roles', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: {}, }, @@ -946,7 +946,7 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {}, + feature: {}, }, space: {}, }, @@ -1001,7 +1001,7 @@ describe('GET role', () => { kibana: { global: { minimum: ['read', 'all'], - features: {} + feature: {} }, space: {}, }, @@ -1055,7 +1055,7 @@ describe('GET role', () => { kibana: { global: { minimum: ['read', 'all'], - features: {} + feature: {} }, space: {}, }, @@ -1109,7 +1109,7 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } @@ -1166,7 +1166,7 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } @@ -1230,16 +1230,16 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: ['read', 'all'], - features: {} + feature: {} }, engineering: { minimum: ['read'], - features: {} + feature: {} } } }, @@ -1298,16 +1298,16 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: ['read', 'all'], - features: {} + feature: {} }, engineering: { minimum: ['read'], - features: {} + feature: {} } } }, @@ -1366,19 +1366,19 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } }, engineering: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1'] } } @@ -1439,19 +1439,19 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: { marketing: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1', 'foo-privilege-2', 'foo-privilege-3'], bar: ['bar-privilege-1'] } }, engineering: { minimum: [], - features: { + feature: { foo: ['foo-privilege-1'] } } @@ -1503,7 +1503,7 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {}, + feature: {}, }, space: {} }, @@ -1552,7 +1552,7 @@ describe('GET role', () => { kibana: { global: { minimum: [], - features: {} + feature: {} }, space: {}, }, From ace7efbaec7e35ca779ec586fa38c11a40986fde Mon Sep 17 00:00:00 2001 From: kobelb Date: Wed, 5 Dec 2018 15:02:21 -0800 Subject: [PATCH 10/18] Validating features and feature privileges --- .../lib/feature_registry/feature_registry.ts | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) 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 fcf9babd3bbd6..c21c4aed56551 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 @@ -5,6 +5,7 @@ */ import { IconType } from '@elastic/eui'; +import Joi from 'joi'; import _ from 'lodash'; export interface FeaturePrivilegeDefinition { @@ -23,7 +24,7 @@ export interface FeaturePrivilegeDefinition { export interface Feature { id: string; name: string; - validLicenses?: Array<'basic' | 'gold' | 'platinum'>; + validLicenses?: Array<'basic' | 'standard' | 'gold' | 'platinum'>; icon?: IconType; description?: string; navLinkId?: string; @@ -32,9 +33,50 @@ export interface Feature { }; } +const schema = Joi.object({ + id: Joi.string() + .regex(/^[a-zA-Z0-9_-]+$/) + .required(), + name: Joi.string().required(), + validLicenses: Joi.array().items(Joi.string().valid('basic', 'standard', 'gold', 'platinum')), + icon: Joi.string(), + description: Joi.string(), + navLinkId: Joi.string(), + privileges: Joi.object() + .pattern( + /^[a-zA-Z0-9_-]+$/, + Joi.object({ + metadata: Joi.object({ + tooltip: Joi.string(), + }), + api: Joi.array().items(Joi.string()), + app: Joi.array() + .items(Joi.string()) + .required(), + savedObject: Joi.object({ + all: Joi.array() + .items(Joi.string()) + .required(), + read: Joi.array() + .items(Joi.string()) + .required(), + }).required(), + ui: Joi.array() + .items(Joi.string()) + .required(), + }) + ) + .required(), +}); + const features: Record = {}; export function registerFeature(feature: Feature) { + const validateResult = Joi.validate(feature, schema); + if (validateResult.error) { + throw validateResult.error; + } + if (feature.id in features) { throw new Error(`Feature with id ${feature.id} is already registered.`); } From ab33c2108eedd60091e1c2a641c571e487a226f1 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 6 Dec 2018 05:47:47 -0800 Subject: [PATCH 11/18] Update x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts Co-Authored-By: kobelb --- .../server/lib/authorization/privilege_serializer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index f37d1846d5170..ee4a9261265ea 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -15,7 +15,7 @@ describe(`#isGlobalMinimumPrivilege`, () => { ['space_all', 'space_read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach( validValue => { - test(`returns true for '${validValue}'`, () => { + test(`returns false for '${invalidValue}'`, () => { expect(PrivilegeSerializer.isGlobalMinimumPrivilege(validValue)).toBe(false); }); } From 74a5b5f4e1896b83af5e65eaa3053ba97338fdf0 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 6 Dec 2018 05:47:57 -0800 Subject: [PATCH 12/18] Update x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts Co-Authored-By: kobelb --- .../server/lib/authorization/privilege_serializer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index ee4a9261265ea..15b68e27ebed3 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -16,7 +16,7 @@ describe(`#isGlobalMinimumPrivilege`, () => { ['space_all', 'space_read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach( validValue => { test(`returns false for '${invalidValue}'`, () => { - expect(PrivilegeSerializer.isGlobalMinimumPrivilege(validValue)).toBe(false); + expect(PrivilegeSerializer.isGlobalMinimumPrivilege(invalidValue)).toBe(false); }); } ); From 852dc3b0d6367985cd4fa2c35a4d0b8fd0187d3d Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 6 Dec 2018 05:48:04 -0800 Subject: [PATCH 13/18] Update x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts Co-Authored-By: kobelb --- .../server/lib/authorization/privilege_serializer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index 15b68e27ebed3..8c70fca2b8139 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -14,7 +14,7 @@ describe(`#isGlobalMinimumPrivilege`, () => { }); ['space_all', 'space_read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach( - validValue => { + invalidValue => { test(`returns false for '${invalidValue}'`, () => { expect(PrivilegeSerializer.isGlobalMinimumPrivilege(invalidValue)).toBe(false); }); From 8bce406e30c62347aa7454e6492ee4ecbc0989f8 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 6 Dec 2018 06:03:06 -0800 Subject: [PATCH 14/18] Renaming some variables/members of the PrivilegesSerializer --- .../privilege_serializer.test.ts | 8 ++++---- .../lib/authorization/privilege_serializer.ts | 19 ++++++++++--------- .../server/routes/api/public/roles/get.js | 8 ++++---- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts index 8c70fca2b8139..5d99e39ff1c45 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.test.ts @@ -9,14 +9,14 @@ import { PrivilegeSerializer } from './privilege_serializer'; describe(`#isGlobalMinimumPrivilege`, () => { ['all', 'read'].forEach(validValue => { test(`returns true for '${validValue}'`, () => { - expect(PrivilegeSerializer.isGlobalMinimumPrivilege(validValue)).toBe(true); + expect(PrivilegeSerializer.isSerializedGlobalMinimumPrivilege(validValue)).toBe(true); }); }); ['space_all', 'space_read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach( invalidValue => { test(`returns false for '${invalidValue}'`, () => { - expect(PrivilegeSerializer.isGlobalMinimumPrivilege(invalidValue)).toBe(false); + expect(PrivilegeSerializer.isSerializedGlobalMinimumPrivilege(invalidValue)).toBe(false); }); } ); @@ -25,13 +25,13 @@ describe(`#isGlobalMinimumPrivilege`, () => { describe(`#isSpaceMinimumPrivilege`, () => { ['space_all', 'space_read'].forEach(validValue => { test(`returns true for '${validValue}'`, () => { - expect(PrivilegeSerializer.isSpaceMinimumPrivilege(validValue)).toBe(true); + expect(PrivilegeSerializer.isSerializedSpaceMinimumPrivilege(validValue)).toBe(true); }); }); ['all', 'read', 'foo', 'bar', 'feature_foo', 'feature_foo.privilege1'].forEach(validValue => { test(`returns true for '${validValue}'`, () => { - expect(PrivilegeSerializer.isSpaceMinimumPrivilege(validValue)).toBe(false); + expect(PrivilegeSerializer.isSerializedSpaceMinimumPrivilege(validValue)).toBe(false); }); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts index 6567d9ff3fac2..0d823f08a3d20 100644 --- a/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privilege_serializer.ts @@ -7,7 +7,8 @@ const featurePrefix = 'feature_'; const spacePrefix = 'space_'; const minimumPrivilegeNames = ['all', 'read']; -const spaceMinimumPrivilegeNames = minimumPrivilegeNames.map( +const globalMinimumPrivileges = [...minimumPrivilegeNames]; +const spaceMinimumPrivileges = minimumPrivilegeNames.map( privilegeName => `${spacePrefix}${privilegeName}` ); const deserializeFeaturePrivilegeRegexp = new RegExp( @@ -20,12 +21,12 @@ interface FeaturePrivilege { } export class PrivilegeSerializer { - public static isGlobalMinimumPrivilege(privilegeName: string) { - return minimumPrivilegeNames.includes(privilegeName); + public static isSerializedGlobalMinimumPrivilege(privilegeName: string) { + return globalMinimumPrivileges.includes(privilegeName); } - public static isSpaceMinimumPrivilege(privilegeName: string) { - return spaceMinimumPrivilegeNames.includes(privilegeName); + public static isSerializedSpaceMinimumPrivilege(privilegeName: string) { + return spaceMinimumPrivileges.includes(privilegeName); } public static serializeGlobalMinimumPrivilege(privilegeName: string) { @@ -44,8 +45,8 @@ export class PrivilegeSerializer { return `${spacePrefix}${privilegeName}`; } - public static serializeFeaturePrivilege(featureName: string, privilegeName: string) { - return `${featurePrefix}${featureName}.${privilegeName}`; + public static serializeFeaturePrivilege(featureId: string, privilegeName: string) { + return `${featurePrefix}${featureId}.${privilegeName}`; } public static deserializeFeaturePrivilege(privilege: string): FeaturePrivilege { @@ -61,7 +62,7 @@ export class PrivilegeSerializer { } public static deserializeGlobalMinimumPrivilege(privilege: string) { - if (PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)) { + if (PrivilegeSerializer.isSerializedGlobalMinimumPrivilege(privilege)) { return privilege; } @@ -69,7 +70,7 @@ export class PrivilegeSerializer { } public static deserializeSpaceMinimumPrivilege(privilege: string) { - if (!PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)) { + if (!PrivilegeSerializer.isSerializedSpaceMinimumPrivilege(privilege)) { throw new Error('Unrecognized space minimum privilege'); } diff --git a/x-pack/plugins/security/server/routes/api/public/roles/get.js b/x-pack/plugins/security/server/routes/api/public/roles/get.js index fd60df408c115..3642476183666 100644 --- a/x-pack/plugins/security/server/routes/api/public/roles/get.js +++ b/x-pack/plugins/security/server/routes/api/public/roles/get.js @@ -21,8 +21,8 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, return resourcePrivileges.reduce((result, { resource, privileges }) => { if (resource === GLOBAL_RESOURCE) { - const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)); - const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isGlobalMinimumPrivilege(privilege)); + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isSerializedGlobalMinimumPrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isSerializedGlobalMinimumPrivilege(privilege)); result.global = { minimum: _.uniq([ @@ -44,8 +44,8 @@ export function initGetRolesApi(server, callWithRequest, routePreCheckLicenseFn, } const spaceId = ResourceSerializer.deserializeSpaceResource(resource); - const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); - const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isSpaceMinimumPrivilege(privilege)); + const minimumPrivileges = privileges.filter(privilege => PrivilegeSerializer.isSerializedSpaceMinimumPrivilege(privilege)); + const featurePrivileges = privileges.filter(privilege => !PrivilegeSerializer.isSerializedSpaceMinimumPrivilege(privilege)); result.space[spaceId] = { minimum: _.uniq([ ...result.space[spaceId] ? result.space[spaceId].minimum || [] : [], From 62293a7657bd185f57e6c5325c429d9f3cc56bed Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 6 Dec 2018 06:12:51 -0800 Subject: [PATCH 15/18] Fixing privileges serializer tests --- .../privileges_serializer.test.ts.snap | 6 +-- .../privileges_serializer.test.ts | 42 +++++++++---------- .../authorization/privileges_serializer.ts | 2 +- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privileges_serializer.test.ts.snap b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privileges_serializer.test.ts.snap index e3b35497622b9..257b93dfb3d48 100644 --- a/x-pack/plugins/security/server/lib/authorization/__snapshots__/privileges_serializer.test.ts.snap +++ b/x-pack/plugins/security/server/lib/authorization/__snapshots__/privileges_serializer.test.ts.snap @@ -1,7 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`features throws error if feature privileges would conflict 1`] = `"Detected conflicting feature privilege name: feature_foo_bar_baz"`; +exports[`global throws Error if unrecognized global privilege is specified 1`] = `"Unrecognized global minimum privilege"`; -exports[`global throws Error if unrecognized global privilege is specified 1`] = `"Unrecognized global reserved privilege"`; - -exports[`space throws Error if unrecognized space privilege is specified 1`] = `"Unrecognized space reserved privilege"`; +exports[`space throws Error if unrecognized space privilege is specified 1`] = `"Unrecognized space minimum privilege"`; diff --git a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.test.ts b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.test.ts index 0fdaadb2172c5..664467088adcb 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.test.ts @@ -115,48 +115,46 @@ describe('features', () => { }, }); expect(result[application]).toEqual({ - feature_foo_quz: { + 'feature_foo.quz': { application, - name: 'feature_foo_quz', + name: 'feature_foo.quz', actions: ['action-1', 'action-2'], metadata: {}, }, - feature_foo_qux: { + 'feature_foo.qux': { application, - name: 'feature_foo_qux', + name: 'feature_foo.qux', actions: ['action-3', 'action-4'], metadata: {}, }, - feature_bar_quz: { + 'feature_bar.quz': { application, - name: 'feature_bar_quz', + name: 'feature_bar.quz', actions: ['action-1', 'action-2'], metadata: {}, }, - feature_bar_qux: { + 'feature_bar.qux': { application, - name: 'feature_bar_qux', + name: 'feature_bar.qux', actions: ['action-3', 'action-4'], metadata: {}, }, }); }); - test(`throws error if feature privileges would conflict`, () => { + test(`feature privileges don't conflict`, () => { const application = 'foo-application'; - expect(() => { - serializePrivileges(application, { - global: {}, - space: {}, - features: { - foo_bar: { - baz: ['action-1', 'action-2'], - }, - foo: { - bar_baz: ['action-1', 'action-2'], - }, + serializePrivileges(application, { + global: {}, + space: {}, + features: { + foo_bar: { + baz: ['action-1', 'action-2'], }, - }); - }).toThrowErrorMatchingSnapshot(); + foo: { + bar_baz: ['action-1', 'action-2'], + }, + }, + }); }); }); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts index 73521ee9810f9..dcd3ba3346669 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges_serializer.ts @@ -59,7 +59,7 @@ export const serializePrivileges = ( Object.entries(featurePrivileges).forEach(([privilegeName, privilegeActions]) => { const name = PrivilegeSerializer.serializeFeaturePrivilege(featureName, privilegeName); if (Object.keys(acc).includes(name)) { - throw new Error(`Detected conflicting feature privilege name: ${name}`); + throw new Error(`Detected duplicate feature privilege name: ${name}`); } acc[name] = { application, From ed24b8700619b870370c4cd88318474ab291a99b Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 6 Dec 2018 06:26:38 -0800 Subject: [PATCH 16/18] Fixing register privileges with cluster tests --- .../register_privileges_with_cluster.test.js | 88 +++++++++---------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js index 8aeb0ba9003c0..703eb5e303527 100644 --- a/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js +++ b/x-pack/plugins/security/server/lib/authorization/register_privileges_with_cluster.test.js @@ -239,15 +239,15 @@ registerPrivilegesWithClusterTest(`inserts privileges when we don't have any exi actions: ['action:read'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo_all'], metadata: {}, }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:bar_read'], metadata: {}, } @@ -346,14 +346,14 @@ registerPrivilegesWithClusterTest(`updates privileges when global actions don't actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:not-quz'], } } @@ -373,15 +373,15 @@ registerPrivilegesWithClusterTest(`updates privileges when global actions don't actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], metadata: {}, }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:quz'], metadata: {}, } @@ -422,14 +422,14 @@ registerPrivilegesWithClusterTest(`updates privileges when space actions don't m actions: ['action:not-bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:quz'], } } @@ -449,15 +449,15 @@ registerPrivilegesWithClusterTest(`updates privileges when space actions don't m actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], metadata: {}, }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:quz'], metadata: {}, } @@ -498,14 +498,14 @@ registerPrivilegesWithClusterTest(`updates privileges when feature actions don't actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:not-quz'], } } @@ -525,15 +525,15 @@ registerPrivilegesWithClusterTest(`updates privileges when feature actions don't actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:baz'], metadata: {}, }, - feature_bar_read: { + 'feature_bar.read': { application, - name: 'feature_bar_read', + name: 'feature_bar.read', actions: ['action:quz'], metadata: {}, } @@ -572,9 +572,9 @@ registerPrivilegesWithClusterTest(`updates privileges when global privilege adde actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, } @@ -601,9 +601,9 @@ registerPrivilegesWithClusterTest(`updates privileges when global privilege adde actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, } @@ -642,9 +642,9 @@ registerPrivilegesWithClusterTest(`updates privileges when space privilege added actions: ['action:not-bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, } @@ -671,9 +671,9 @@ registerPrivilegesWithClusterTest(`updates privileges when space privilege added actions: ['action:quz'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, } @@ -712,9 +712,9 @@ registerPrivilegesWithClusterTest(`updates privileges when feature privilege add actions: ['action:not-bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, } @@ -735,15 +735,15 @@ registerPrivilegesWithClusterTest(`updates privileges when feature privilege add actions: ['action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:foo-all'], metadata: {}, }, - feature_foo_read: { + 'feature_foo.read': { application, - name: 'feature_foo_read', + name: 'feature_foo.read', actions: ['action:foo-read'], metadata: {}, } @@ -780,9 +780,9 @@ registerPrivilegesWithClusterTest(`doesn't update privileges when order of actio actions: ['action:quz', 'action:bar'], metadata: {}, }, - feature_foo_all: { + 'feature_foo.all': { application, - name: 'feature_foo_all', + name: 'feature_foo.all', actions: ['action:bar-all', 'action:foo-all'], metadata: {}, } From 9b8d765b15d5be72901ebbdd1adde85039d56038 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 6 Dec 2018 10:18:59 -0800 Subject: [PATCH 17/18] Fixing the role creation for the api integration tests --- .../common/lib/create_users_and_roles.ts | 191 +++++++----- .../common/lib/create_users_and_roles.ts | 278 +++++++++++------- 2 files changed, 291 insertions(+), 178 deletions(-) diff --git a/x-pack/test/saved_object_api_integration/common/lib/create_users_and_roles.ts b/x-pack/test/saved_object_api_integration/common/lib/create_users_and_roles.ts index b5e1b97fdf0a6..0949273e37900 100644 --- a/x-pack/test/saved_object_api_integration/common/lib/create_users_and_roles.ts +++ b/x-pack/test/saved_object_api_integration/common/lib/create_users_and_roles.ts @@ -7,99 +7,142 @@ import { SuperTest } from 'supertest'; import { AUTHENTICATION } from './authentication'; export const createUsersAndRoles = async (es: any, supertest: SuperTest) => { - await supertest.put('/api/security/role/kibana_legacy_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana'], - privileges: ['manage', 'read', 'index', 'delete'], - }, - ], - }, - }); + await supertest + .put('/api/security/role/kibana_legacy_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana'], + privileges: ['manage', 'read', 'index', 'delete'], + }, + ], + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_legacy_dashboard_only_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana'], - privileges: ['read', 'view_index_metadata'], - }, - ], - }, - }); + await supertest + .put('/api/security/role/kibana_legacy_dashboard_only_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana'], + privileges: ['read', 'view_index_metadata'], + }, + ], + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_dual_privileges_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana'], - privileges: ['manage', 'read', 'index', 'delete'], + await supertest + .put('/api/security/role/kibana_dual_privileges_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana'], + privileges: ['manage', 'read', 'index', 'delete'], + }, + ], + }, + kibana: { + global: { + minimum: ['all'], }, - ], - }, - kibana: { - global: ['all'], - }, - }); + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_dual_privileges_dashboard_only_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana'], - privileges: ['read', 'view_index_metadata'], + await supertest + .put('/api/security/role/kibana_dual_privileges_dashboard_only_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana'], + privileges: ['read', 'view_index_metadata'], + }, + ], + }, + kibana: { + global: { + minimum: ['read'], }, - ], - }, - kibana: { - global: ['read'], - }, - }); + }, + }) + .expect(204); await supertest.put('/api/security/role/kibana_rbac_user').send({ kibana: { - global: ['all'], + global: { + minimum: ['all'], + }, }, }); - await supertest.put('/api/security/role/kibana_rbac_dashboard_only_user').send({ - kibana: { - global: ['read'], - }, - }); + await supertest + .put('/api/security/role/kibana_rbac_dashboard_only_user') + .send({ + kibana: { + global: { + minimum: ['read'], + }, + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_default_space_all_user').send({ - kibana: { - space: { - default: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_default_space_all_user') + .send({ + kibana: { + space: { + default: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_default_space_read_user').send({ - kibana: { - space: { - default: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_default_space_read_user') + .send({ + kibana: { + space: { + default: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_all_user').send({ - kibana: { - space: { - space_1: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_all_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_read_user').send({ - kibana: { - space: { - space_1: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_read_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); await es.shield.putUser({ username: AUTHENTICATION.NOT_A_KIBANA_USER.username, diff --git a/x-pack/test/spaces_api_integration/common/lib/create_users_and_roles.ts b/x-pack/test/spaces_api_integration/common/lib/create_users_and_roles.ts index 7fbfca1057eab..17aabc9890f82 100644 --- a/x-pack/test/spaces_api_integration/common/lib/create_users_and_roles.ts +++ b/x-pack/test/spaces_api_integration/common/lib/create_users_and_roles.ts @@ -7,133 +7,203 @@ import { SuperTest } from 'supertest'; import { AUTHENTICATION } from './authentication'; export const createUsersAndRoles = async (es: any, supertest: SuperTest) => { - await supertest.put('/api/security/role/kibana_legacy_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana*'], - privileges: ['manage', 'read', 'index', 'delete'], - }, - ], - }, - }); + await supertest + .put('/api/security/role/kibana_legacy_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana*'], + privileges: ['manage', 'read', 'index', 'delete'], + }, + ], + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_legacy_dashboard_only_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana*'], - privileges: ['read', 'view_index_metadata'], - }, - ], - }, - }); + await supertest + .put('/api/security/role/kibana_legacy_dashboard_only_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana*'], + privileges: ['read', 'view_index_metadata'], + }, + ], + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_dual_privileges_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana*'], - privileges: ['manage', 'read', 'index', 'delete'], + await supertest + .put('/api/security/role/kibana_dual_privileges_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana*'], + privileges: ['manage', 'read', 'index', 'delete'], + }, + ], + }, + kibana: { + global: { + minimum: ['all'], }, - ], - }, - kibana: { - global: ['all'], - }, - }); + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_dual_privileges_dashboard_only_user').send({ - elasticsearch: { - indices: [ - { - names: ['.kibana*'], - privileges: ['read', 'view_index_metadata'], + await supertest + .put('/api/security/role/kibana_dual_privileges_dashboard_only_user') + .send({ + elasticsearch: { + indices: [ + { + names: ['.kibana*'], + privileges: ['read', 'view_index_metadata'], + }, + ], + }, + kibana: { + global: { + minimum: ['read'], }, - ], - }, - kibana: { - global: ['read'], - }, - }); + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_user').send({ - kibana: { - global: ['all'], - }, - }); + await supertest + .put('/api/security/role/kibana_rbac_user') + .send({ + kibana: { + global: { + minimum: ['all'], + }, + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_dashboard_only_user').send({ - kibana: { - global: ['read'], - }, - }); + await supertest + .put('/api/security/role/kibana_rbac_dashboard_only_user') + .send({ + kibana: { + global: { + minimum: ['read'], + }, + }, + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_default_space_all_user').send({ - kibana: { - space: { - default: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_default_space_all_user') + .send({ + kibana: { + space: { + default: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_default_space_read_user').send({ - kibana: { - space: { - default: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_default_space_read_user') + .send({ + kibana: { + space: { + default: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_all_user').send({ - kibana: { - space: { - space_1: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_all_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_read_user').send({ - kibana: { - space: { - space_1: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_read_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_2_all_user').send({ - kibana: { - space: { - space_2: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_space_2_all_user') + .send({ + kibana: { + space: { + space_2: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_2_read_user').send({ - kibana: { - space: { - space_2: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_space_2_read_user') + .send({ + kibana: { + space: { + space_2: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_2_all_user').send({ - kibana: { - space: { - space_1: ['all'], - space_2: ['all'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_2_all_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['all'], + }, + space_2: { + minimum: ['all'], + }, + }, }, - }, - }); + }) + .expect(204); - await supertest.put('/api/security/role/kibana_rbac_space_1_2_read_user').send({ - kibana: { - space: { - space_1: ['read'], - space_2: ['read'], + await supertest + .put('/api/security/role/kibana_rbac_space_1_2_read_user') + .send({ + kibana: { + space: { + space_1: { + minimum: ['read'], + }, + space_2: { + minimum: ['read'], + }, + }, }, - }, - }); + }) + .expect(204); await es.shield.putUser({ username: AUTHENTICATION.NOT_A_KIBANA_USER.username, From 3e19d4a74630a902098b4fa2032d78af070b9011 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 6 Dec 2018 11:47:49 -0800 Subject: [PATCH 18/18] Generalizing regex within the feature registry --- .../server/lib/feature_registry/feature_registry.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 c21c4aed56551..42c1751c0ae8c 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 @@ -33,9 +33,11 @@ export interface Feature { }; } +const featurePrivilegePartRegex = /^[a-zA-Z0-9_-]+$/; + const schema = Joi.object({ id: Joi.string() - .regex(/^[a-zA-Z0-9_-]+$/) + .regex(featurePrivilegePartRegex) .required(), name: Joi.string().required(), validLicenses: Joi.array().items(Joi.string().valid('basic', 'standard', 'gold', 'platinum')), @@ -44,7 +46,7 @@ const schema = Joi.object({ navLinkId: Joi.string(), privileges: Joi.object() .pattern( - /^[a-zA-Z0-9_-]+$/, + featurePrivilegePartRegex, Joi.object({ metadata: Joi.object({ tooltip: Joi.string(),