maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Support multiple reserved feature privileges (#61980)

Browse source 
* support multiple reserved feature privileges

* update reserved privilege ids

* additional testing

* Add ml_user and ml_admin reserved privileges

* prrevent reserved privilege ids from sttarting with 'reserved_'

* address pr feedback: dedicated reserved privilege type

* re-enable ML test suites

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Larry Gregory 2020-04-08 13:03:15 -04:00 committed by GitHub
parent 730dcbf638
commit 9d89a4fb49
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 512 additions and 133 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
x-pack/plugins/features/common/feature.ts
View file

@ -7,6 +7,7 @@
import { RecursiveReadonly } from '@kbn/utility-types';
import { FeatureKibanaPrivileges } from './feature_kibana_privileges';
import { SubFeatureConfig, SubFeature } from './sub_feature';
import { ReservedKibanaPrivilege } from './reserved_kibana_privilege';
/**
* Interface for registering a feature.
@ -122,8 +123,8 @@ export interface FeatureConfig {
* @private
*/
reserved?: {
privilege: FeatureKibanaPrivileges;
description: string;
privileges: ReservedKibanaPrivilege[];
};
}

12
x-pack/plugins/features/common/reserved_kibana_privilege.ts Normal file
View file

@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { FeatureKibanaPrivileges } from '.';
export interface ReservedKibanaPrivilege {
id: string;
privilege: FeatureKibanaPrivileges;
}

254
x-pack/plugins/features/server/feature_registry.test.ts
View file

@ -110,19 +110,24 @@ describe('FeatureRegistry', () => {
],
privilegesTooltip: 'some fancy tooltip',
reserved: {
privilege: {
catalogue: ['foo'],
management: {
foo: ['bar'],
privileges: [
{
id: 'reserved',
privilege: {
catalogue: ['foo'],
management: {
foo: ['bar'],
},
app: ['app1'],
savedObject: {
all: ['space', 'etc', 'telemetry'],
read: ['canvas', 'config', 'url'],
},
api: ['someApiEndpointTag', 'anotherEndpointTag'],
ui: ['allowsFoo', 'showBar', 'showBaz'],
},
},
app: ['app1'],
savedObject: {
all: ['space', 'etc', 'telemetry'],
read: ['canvas', 'config', 'url'],
},
api: ['someApiEndpointTag', 'anotherEndpointTag'],
ui: ['allowsFoo', 'showBar', 'showBaz'],
},
],
description: 'some completely adequate description',
},
};
@ -264,13 +269,18 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'foo',
privilege: {
ui: [],
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'reserved',
privilege: {
ui: [],
savedObject: {
all: [],
read: [],
},
},
},
},
],
},
};
@ -278,7 +288,7 @@ describe('FeatureRegistry', () => {
featureRegistry.register(feature);
const result = featureRegistry.getAll();
const reservedPrivilege = result[0]!.reserved!.privilege;
const reservedPrivilege = result[0]!.reserved!.privileges[0].privilege;
expect(reservedPrivilege.savedObject.all).toEqual(['telemetry']);
expect(reservedPrivilege.savedObject.read).toEqual(['config', 'url']);
});
@ -520,14 +530,19 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
app: ['foo', 'bar', 'baz'],
},
},
ui: [],
app: ['foo', 'bar', 'baz'],
},
],
},
};
@ -546,14 +561,19 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
app: ['foo', 'bar'],
},
},
ui: [],
app: ['foo', 'bar'],
},
],
},
};
@ -666,15 +686,20 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
catalogue: ['foo', 'bar', 'baz'],
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'reserved',
privilege: {
catalogue: ['foo', 'bar', 'baz'],
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
ui: [],
app: [],
},
],
},
};
@ -694,15 +719,20 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
catalogue: ['foo', 'bar'],
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'reserved',
privilege: {
catalogue: ['foo', 'bar'],
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
ui: [],
app: [],
},
],
},
};
@ -840,18 +870,23 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
catalogue: ['bar'],
management: {
kibana: ['hey-there'],
privileges: [
{
id: 'reserved',
privilege: {
catalogue: ['bar'],
management: {
kibana: ['hey-there'],
},
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
],
},
};
@ -874,18 +909,23 @@ describe('FeatureRegistry', () => {
privileges: null,
reserved: {
description: 'something',
privilege: {
catalogue: ['bar'],
management: {
kibana: ['hey-there'],
privileges: [
{
id: 'reserved',
privilege: {
catalogue: ['bar'],
management: {
kibana: ['hey-there'],
},
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
],
},
};
@ -896,6 +936,78 @@ describe('FeatureRegistry', () => {
);
});
it('allows multiple reserved feature privileges to be registered', () => {
const feature: FeatureConfig = {
id: 'test-feature',
name: 'Test Feature',
app: [],
privileges: null,
reserved: {
description: 'my reserved privileges',
privileges: [
{
id: 'a_reserved_1',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
{
id: 'a_reserved_2',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
],
},
};
const featureRegistry = new FeatureRegistry();
featureRegistry.register(feature);
const result = featureRegistry.getAll();
expect(result).toHaveLength(1);
expect(result[0].reserved?.privileges).toHaveLength(2);
});
it('does not allow reserved privilege ids to start with "reserved_"', () => {
const feature: FeatureConfig = {
id: 'test-feature',
name: 'Test Feature',
app: [],
privileges: null,
reserved: {
description: 'my reserved privileges',
privileges: [
{
id: 'reserved_1',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
app: [],
},
},
],
},
};
const featureRegistry = new FeatureRegistry();
expect(() => featureRegistry.register(feature)).toThrowErrorMatchingInlineSnapshot(
`"child \\"reserved\\" fails because [child \\"privileges\\" fails because [\\"privileges\\" at position 0 fails because [child \\"id\\" fails because [\\"id\\" with value \\"reserved_1\\" fails to match the required pattern: /^(?!reserved_)[a-zA-Z0-9_-]+$/]]]]"`
);
});
it('cannot register feature after getAll has been called', () => {
const feature1: FeatureConfig = {
id: 'test-feature',

4
x-pack/plugins/features/server/feature_registry.ts
View file

@ -39,9 +39,9 @@ export class FeatureRegistry {
function applyAutomaticPrivilegeGrants(feature: FeatureConfig): FeatureConfig {
const allPrivilege = feature.privileges?.all;
const readPrivilege = feature.privileges?.read;
const reservedPrivilege = feature.reserved?.privilege;
const reservedPrivileges = (feature.reserved?.privileges ?? []).map(rp => rp.privilege);
applyAutomaticAllPrivilegeGrants(allPrivilege, reservedPrivilege);
applyAutomaticAllPrivilegeGrants(allPrivilege, ...reservedPrivileges);
applyAutomaticReadPrivilegeGrants(readPrivilege);
return feature;

16
x-pack/plugins/features/server/feature_schema.ts
View file

@ -18,6 +18,7 @@ const prohibitedFeatureIds: Array<keyof UICapabilities> = ['catalogue', 'managem
const featurePrivilegePartRegex = /^[a-zA-Z0-9_-]+$/;
const subFeaturePrivilegePartRegex = /^[a-zA-Z0-9_-]+$/;
const managementSectionIdRegex = /^[a-zA-Z0-9_-]+$/;
const reservedFeaturePrrivilegePartRegex = /^(?!reserved_)[a-zA-Z0-9_-]+$/;
export const uiCapabilitiesRegex = /^[a-zA-Z0-9:_-]+$/;
const managementSchema = Joi.object().pattern(
@ -118,8 +119,17 @@ const schema = Joi.object({
}),
privilegesTooltip: Joi.string(),
reserved: Joi.object({
privilege: privilegeSchema.required(),
description: Joi.string().required(),
privileges: Joi.array()
.items(
Joi.object({
id: Joi.string()
.regex(reservedFeaturePrrivilegePartRegex)
.required(),
privilege: privilegeSchema.required(),
})
)
.required(),
}),
});
@ -209,7 +219,9 @@ export function validateFeature(feature: FeatureConfig) {
privilegeEntries.push(...Object.entries(feature.privileges));
}
if (feature.reserved) {
privilegeEntries.push(['reserved', feature.reserved.privilege]);
feature.reserved.privileges.forEach(reservedPrivilege => {
privilegeEntries.push([reservedPrivilege.id, reservedPrivilege.privilege]);
});
}
if (privilegeEntries.length === 0) {

35
x-pack/plugins/ml/server/plugin.ts
View file

@ -79,19 +79,36 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
catalogue: [PLUGIN_ID],
privileges: null,
reserved: {
privilege: {
app: [PLUGIN_ID, 'kibana'],
catalogue: [PLUGIN_ID],
savedObject: {
all: [],
read: [],
},
ui: [],
},
description: i18n.translate('xpack.ml.feature.reserved.description', {
defaultMessage:
'To grant users access, you should also assign either the machine_learning_user or machine_learning_admin role.',
}),
privileges: [
{
id: 'ml_user',
privilege: {
app: [PLUGIN_ID, 'kibana'],
catalogue: [PLUGIN_ID],
savedObject: {
all: [],
read: [],
},
ui: [],
},
},
{
id: 'ml_admin',
privilege: {
app: [PLUGIN_ID, 'kibana'],
catalogue: [PLUGIN_ID],
savedObject: {
all: [],
read: [],
},
ui: [],
},
},
],
},
});

23
x-pack/plugins/monitoring/server/plugin.ts
View file

@ -276,18 +276,23 @@ export class Plugin {
catalogue: ['monitoring'],
privileges: null,
reserved: {
privilege: {
app: ['monitoring', 'kibana'],
catalogue: ['monitoring'],
savedObject: {
all: [],
read: [],
},
ui: [],
},
description: i18n.translate('xpack.monitoring.feature.reserved.description', {
defaultMessage: 'To grant users access, you should also assign the monitoring_user role.',
}),
privileges: [
{
id: 'monitoring',
privilege: {
app: ['monitoring', 'kibana'],
catalogue: ['monitoring'],
savedObject: {
all: [],
read: [],
},
ui: [],
},
},
],
},
});
}

5
x-pack/plugins/security/server/authorization/index.ts
View file

@ -29,6 +29,7 @@ import { initAppAuthorization } from './app_authorization';
import { initAPIAuthorization } from './api_authorization';
import { disableUICapabilitiesFactory } from './disable_ui_capabilities';
import { validateFeaturePrivileges } from './validate_feature_privileges';
import { validateReservedPrivileges } from './validate_reserved_privileges';
import { registerPrivilegesWithCluster } from './register_privileges_with_cluster';
import { APPLICATION_PREFIX } from '../../common/constants';
import { SecurityLicense } from '../../common/licensing';
@ -121,7 +122,9 @@ export function setupAuthorization({
},
registerPrivilegesWithCluster: async () => {
validateFeaturePrivileges(featuresService.getFeatures());
const features = featuresService.getFeatures();
validateFeaturePrivileges(features);
validateReservedPrivileges(features);
await registerPrivilegesWithCluster(logger, privileges, applicationName, clusterClient);
},

51
x-pack/plugins/security/server/authorization/privileges/privileges.test.ts
View file

@ -409,13 +409,18 @@ describe('features', () => {
},
privileges: null,
reserved: {
privilege: {
savedObject: {
all: ['ignore-me-1', 'ignore-me-2'],
read: ['ignore-me-1', 'ignore-me-2'],
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['ignore-me-1', 'ignore-me-2'],
read: ['ignore-me-1', 'ignore-me-2'],
},
ui: ['ignore-me-1'],
},
},
ui: ['ignore-me-1'],
},
],
description: '',
},
}),
@ -591,13 +596,18 @@ describe('reserved', () => {
},
privileges: null,
reserved: {
privilege: {
savedObject: {
all: [],
read: [],
privileges: [
{
id: 'foo',
privilege: {
savedObject: {
all: [],
read: [],
},
ui: [],
},
},
ui: [],
},
],
description: '',
},
}),
@ -627,13 +637,18 @@ describe('reserved', () => {
app: [],
privileges: null,
reserved: {
privilege: {
savedObject: {
all: ['savedObject-all-1', 'savedObject-all-2'],
read: ['savedObject-read-1', 'savedObject-read-2'],
privileges: [
{
id: 'foo',
privilege: {
savedObject: {
all: ['savedObject-all-1', 'savedObject-all-2'],
read: ['savedObject-read-1', 'savedObject-read-2'],
},
ui: ['ui-1', 'ui-2'],
},
},
ui: ['ui-1', 'ui-2'],
},
],
description: '',
},
}),

10
x-pack/plugins/security/server/authorization/privileges/privileges.ts
View file

@ -110,10 +110,12 @@ export function privilegesFactory(
},
reserved: features.reduce((acc: Record<string, string[]>, feature: Feature) => {
if (feature.reserved) {
acc[feature.id] = [
actions.version,
...featurePrivilegeBuilder.getActions(feature.reserved!.privilege, feature),
];
feature.reserved.privileges.forEach(reservedPrivilege => {
acc[reservedPrivilege.id] = [
actions.version,
...uniq(featurePrivilegeBuilder.getActions(reservedPrivilege.privilege, feature)),
];
});
}
return acc;
}, {}),

17
x-pack/plugins/security/server/authorization/validate_feature_privileges.test.ts
View file

@ -26,13 +26,18 @@ it('allows features with reserved privileges to be defined', () => {
privileges: null,
reserved: {
description: 'foo',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
ui: [],
},
],
},
});

181
x-pack/plugins/security/server/authorization/validate_reserved_privileges.test.ts Normal file
View file

@ -0,0 +1,181 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { Feature } from '../../../features/server';
import { validateReservedPrivileges } from './validate_reserved_privileges';
it('allows features to be defined without privileges', () => {
const feature: Feature = new Feature({
id: 'foo',
name: 'foo',
app: [],
privileges: null,
});
validateReservedPrivileges([feature]);
});
it('allows features with a single reserved privilege to be defined', () => {
const feature: Feature = new Feature({
id: 'foo',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
validateReservedPrivileges([feature]);
});
it('allows multiple features with reserved privileges to be defined', () => {
const feature1: Feature = new Feature({
id: 'foo',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved-1',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
const feature2: Feature = new Feature({
id: 'foo2',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved-2',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
validateReservedPrivileges([feature1, feature2]);
});
it('prevents a feature from specifying the same reserved privilege id', () => {
const feature1: Feature = new Feature({
id: 'foo',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
expect(() => validateReservedPrivileges([feature1])).toThrowErrorMatchingInlineSnapshot(
`"Duplicate reserved privilege id detected: reserved. This is not allowed."`
);
});
it('prevents features from sharing a reserved privilege id', () => {
const feature1: Feature = new Feature({
id: 'foo',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
const feature2: Feature = new Feature({
id: 'foo2',
name: 'foo',
app: [],
privileges: null,
reserved: {
description: 'foo',
privileges: [
{
id: 'reserved',
privilege: {
savedObject: {
all: ['foo'],
read: ['bar'],
},
ui: [],
},
},
],
},
});
expect(() => validateReservedPrivileges([feature1, feature2])).toThrowErrorMatchingInlineSnapshot(
`"Duplicate reserved privilege id detected: reserved. This is not allowed."`
);
});

20
x-pack/plugins/security/server/authorization/validate_reserved_privileges.ts Normal file
View file

@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { Feature } from '../../../features/server';
export function validateReservedPrivileges(features: Feature[]) {
const seenPrivilegeIds = new Set<string>();
for (const feature of features) {
(feature?.reserved?.privileges ?? []).forEach(({ id }) => {
if (seenPrivilegeIds.has(id)) {
throw new Error(`Duplicate reserved privilege id detected: ${id}. This is not allowed.`);
}
seenPrivilegeIds.add(id);
});
}
}

5
x-pack/test/api_integration/apis/ml/index.ts
View file

@ -9,10 +9,7 @@ import { FtrProviderContext } from '../../ftr_provider_context';
export default function({ getService, loadTestFile }: FtrProviderContext) {
const ml = getService('ml');
// ML tests need to be disabled in orde to get the ES snapshot with
// https://github.com/elastic/elasticsearch/pull/54713 promoted
// and should be re-enabled as part of https://github.com/elastic/kibana/pull/61980
describe.skip('Machine Learning', function() {
describe('Machine Learning', function() {
this.tags(['mlqa']);
before(async () => {

2
x-pack/test/api_integration/apis/security/privileges.ts
View file

@ -41,7 +41,7 @@ export default function({ getService }: FtrProviderContext) {
},
global: ['all', 'read'],
space: ['all', 'read'],
reserved: ['ml', 'monitoring'],
reserved: ['ml_user', 'ml_admin', 'monitoring'],
};
await supertest

2
x-pack/test/api_integration/apis/security/privileges_basic.ts
View file

@ -39,7 +39,7 @@ export default function({ getService }: FtrProviderContext) {
},
global: ['all', 'read'],
space: ['all', 'read'],
reserved: ['ml', 'monitoring'],
reserved: ['ml_user', 'ml_admin', 'monitoring'],
};
await supertest

5
x-pack/test/functional/apps/machine_learning/index.ts
View file

@ -8,10 +8,7 @@ import { FtrProviderContext } from '../../ftr_provider_context';
export default function({ getService, loadTestFile }: FtrProviderContext) {
const ml = getService('ml');
// ML tests need to be disabled in orde to get the ES snapshot with
// https://github.com/elastic/elasticsearch/pull/54713 promoted
// and should be re-enabled as part of https://github.com/elastic/kibana/pull/61980
describe.skip('machine learning', function() {
describe('machine learning', function() {
this.tags('ciGroup3');
before(async () => {