Skip to content

Commit

Permalink
Introduce reserved ml privilege for the apm_user role (#72266)
Browse files Browse the repository at this point in the history
Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
legrego and elasticmachine authored Jul 28, 2020
1 parent 46fb847 commit 09b11b6
Show file tree
Hide file tree
Showing 21 changed files with 102 additions and 107 deletions.
12 changes: 6 additions & 6 deletions x-pack/plugins/infra/server/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ export const METRICS_FEATURE = {
order: 700,
icon: 'metricsApp',
navLinkId: 'metrics',
app: ['infra', 'kibana'],
app: ['infra', 'metrics', 'kibana'],
catalogue: ['infraops'],
alerting: [METRIC_THRESHOLD_ALERT_TYPE_ID, METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID],
privileges: {
all: {
app: ['infra', 'kibana'],
app: ['infra', 'metrics', 'kibana'],
catalogue: ['infraops'],
api: ['infra'],
savedObject: {
Expand All @@ -35,7 +35,7 @@ export const METRICS_FEATURE = {
ui: ['show', 'configureSource', 'save', 'alerting:show'],
},
read: {
app: ['infra', 'kibana'],
app: ['infra', 'metrics', 'kibana'],
catalogue: ['infraops'],
api: ['infra'],
savedObject: {
Expand All @@ -58,12 +58,12 @@ export const LOGS_FEATURE = {
order: 800,
icon: 'logsApp',
navLinkId: 'logs',
app: ['infra', 'kibana'],
app: ['infra', 'logs', 'kibana'],
catalogue: ['infralogging'],
alerting: [LOG_DOCUMENT_COUNT_ALERT_TYPE_ID],
privileges: {
all: {
app: ['infra', 'kibana'],
app: ['infra', 'logs', 'kibana'],
catalogue: ['infralogging'],
api: ['infra'],
savedObject: {
Expand All @@ -76,7 +76,7 @@ export const LOGS_FEATURE = {
ui: ['show', 'configureSource', 'save'],
},
read: {
app: ['infra', 'kibana'],
app: ['infra', 'logs', 'kibana'],
catalogue: ['infralogging'],
api: ['infra'],
alerting: {
Expand Down
16 changes: 16 additions & 0 deletions x-pack/plugins/ml/common/types/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import { KibanaRequest } from 'kibana/server';
import { PLUGIN_ID } from '../constants/app';

export const apmUserMlCapabilities = {
canGetJobs: false,
};

export const userMlCapabilities = {
canAccessML: false,
// Anomaly Detection
Expand Down Expand Up @@ -68,6 +72,7 @@ export function getDefaultCapabilities(): MlCapabilities {
}

export function getPluginPrivileges() {
const apmUserMlCapabilitiesKeys = Object.keys(apmUserMlCapabilities);
const userMlCapabilitiesKeys = Object.keys(userMlCapabilities);
const adminMlCapabilitiesKeys = Object.keys(adminMlCapabilities);
const allMlCapabilitiesKeys = [...adminMlCapabilitiesKeys, ...userMlCapabilitiesKeys];
Expand Down Expand Up @@ -101,6 +106,17 @@ export function getPluginPrivileges() {
read: savedObjects,
},
},
apmUser: {
excludeFromBasePrivileges: true,
app: [],
catalogue: [],
savedObject: {
all: [],
read: [],
},
api: apmUserMlCapabilitiesKeys.map((k) => `ml:${k}`),
ui: apmUserMlCapabilitiesKeys,
},
};
}

Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/ml/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
}

public setup(coreSetup: CoreSetup, plugins: PluginsSetup): MlPluginSetup {
const { admin, user } = getPluginPrivileges();
const { admin, user, apmUser } = getPluginPrivileges();

plugins.features.registerFeature({
id: PLUGIN_ID,
Expand Down Expand Up @@ -108,6 +108,10 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
id: 'ml_admin',
privilege: admin,
},
{
id: 'ml_apm_user',
privilege: apmUser,
},
],
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('usingPrivileges', () => {
new Feature({
id: 'fooFeature',
name: 'Foo Feature',
app: ['fooApp'],
app: ['fooApp', 'foo'],
navLinkId: 'foo',
privileges: null,
}),
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('usingPrivileges', () => {
new Feature({
id: 'fooFeature',
name: 'Foo Feature',
app: [],
app: ['foo'],
navLinkId: 'foo',
privileges: null,
}),
Expand Down Expand Up @@ -262,7 +262,7 @@ describe('usingPrivileges', () => {
id: 'barFeature',
name: 'Bar Feature',
navLinkId: 'bar',
app: [],
app: ['bar'],
privileges: null,
}),
],
Expand Down Expand Up @@ -412,7 +412,7 @@ describe('all', () => {
new Feature({
id: 'fooFeature',
name: 'Foo Feature',
app: [],
app: ['foo'],
navLinkId: 'foo',
privileges: null,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ export function disableUICapabilitiesFactory(
logger: Logger,
authz: AuthorizationServiceSetup
) {
// nav links are sourced from two places:
// 1) The `navLinkId` property. This is deprecated and will be removed (https://github.com/elastic/kibana/issues/66217)
// 2) The apps property. The Kibana Platform associates nav links to the app which registers it, in a 1:1 relationship.
// This behavior is replacing the `navLinkId` property above.
// nav links are sourced from the apps property.
// The Kibana Platform associates nav links to the app which registers it, in a 1:1 relationship.
// This behavior is replacing the `navLinkId` property.
const featureNavLinkIds = features
.flatMap((feature) => [feature.navLinkId, ...feature.app])
.flatMap((feature) => feature.app)
.filter((navLinkId) => navLinkId != null);

const shouldDisableFeatureUICapability = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import { BaseFeaturePrivilegeBuilder } from './feature_privilege_builder';

export class FeaturePrivilegeNavlinkBuilder extends BaseFeaturePrivilegeBuilder {
public getActions(privilegeDefinition: FeatureKibanaPrivileges, feature: Feature): string[] {
const appNavLinks = feature.app.map((app) => this.actions.ui.get('navLinks', app));
return feature.navLinkId
? [this.actions.ui.get('navLinks', feature.navLinkId), ...appNavLinks]
: appNavLinks;
return (privilegeDefinition.app ?? []).map((app) => this.actions.ui.get('navLinks', app));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,8 @@ describe('features', () => {

const actual = privileges.get();
expect(actual).toHaveProperty('features.foo-feature', {
all: [
actions.login,
actions.version,
actions.ui.get('navLinks', 'kibana:foo'),
actions.ui.get('navLinks', 'app-1'),
actions.ui.get('navLinks', 'app-2'),
],
read: [
actions.login,
actions.version,
actions.ui.get('navLinks', 'kibana:foo'),
actions.ui.get('navLinks', 'app-1'),
actions.ui.get('navLinks', 'app-2'),
],
all: [actions.login, actions.version],
read: [actions.login, actions.version],
});
});

Expand Down Expand Up @@ -275,7 +263,6 @@ describe('features', () => {
actions.ui.get('catalogue', 'all-catalogue-2'),
actions.ui.get('management', 'all-management', 'all-management-1'),
actions.ui.get('management', 'all-management', 'all-management-2'),
actions.ui.get('navLinks', 'kibana:foo'),
actions.savedObject.get('all-savedObject-all-1', 'bulk_get'),
actions.savedObject.get('all-savedObject-all-1', 'get'),
actions.savedObject.get('all-savedObject-all-1', 'find'),
Expand Down Expand Up @@ -386,7 +373,6 @@ describe('features', () => {
actions.ui.get('catalogue', 'read-catalogue-2'),
actions.ui.get('management', 'read-management', 'read-management-1'),
actions.ui.get('management', 'read-management', 'read-management-2'),
actions.ui.get('navLinks', 'kibana:foo'),
actions.savedObject.get('read-savedObject-all-1', 'bulk_get'),
actions.savedObject.get('read-savedObject-all-1', 'get'),
actions.savedObject.get('read-savedObject-all-1', 'find'),
Expand Down Expand Up @@ -644,12 +630,7 @@ describe('reserved', () => {
const privileges = privilegesFactory(actions, mockXPackMainPlugin as any, mockLicenseService);

const actual = privileges.get();
expect(actual).toHaveProperty('reserved.foo', [
actions.version,
actions.ui.get('navLinks', 'kibana:foo'),
actions.ui.get('navLinks', 'app-1'),
actions.ui.get('navLinks', 'app-2'),
]);
expect(actual).toHaveProperty('reserved.foo', [actions.version]);
});

test(`actions only specified at the privilege are alright too`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const features = ([
id: 'feature_2',
name: 'Feature 2',
navLinkId: 'feature2',
app: [],
app: ['feature2'],
catalogue: ['feature2Entry'],
management: {
kibana: ['somethingElse'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ function toggleDisabledFeatures(

for (const feature of disabledFeatures) {
// Disable associated navLink, if one exists
const featureNavLinks = feature.navLinkId ? [feature.navLinkId, ...feature.app] : feature.app;
featureNavLinks.forEach((app) => {
feature.app.forEach((app) => {
if (navLinks.hasOwnProperty(app) && !enabledAppEntries.has(app)) {
navLinks[app] = false;
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/api_integration/apis/security/privileges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function ({ getService }: FtrProviderContext) {
},
global: ['all', 'read'],
space: ['all', 'read'],
reserved: ['ml_user', 'ml_admin', 'monitoring'],
reserved: ['ml_user', 'ml_admin', 'ml_apm_user', 'monitoring'],
};

await supertest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default function ({ getService }: FtrProviderContext) {
},
global: ['all', 'read'],
space: ['all', 'read'],
reserved: ['ml_user', 'ml_admin', 'monitoring'],
reserved: ['ml_user', 'ml_admin', 'ml_apm_user', 'monitoring'],
};

await supertest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,19 +423,19 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(navLinks).to.not.contain(['Metrics']);
});

it(`metrics app is inaccessible and Application Not Found message is rendered`, async () => {
await PageObjects.common.navigateToApp('infraOps');
await testSubjects.existOrFail('~appNotFoundPageContent');
await PageObjects.common.navigateToUrlWithBrowserHistory(
'infraOps',
'/inventory',
undefined,
{
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
}
it(`metrics app is inaccessible and returns a 404`, async () => {
await PageObjects.common.navigateToActualUrl('infraOps', '', {
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
});
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
})
);
await testSubjects.existOrFail('~appNotFoundPageContent');
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,19 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

it(`metrics app is inaccessible and Application Not Found message is rendered`, async () => {
await PageObjects.common.navigateToApp('infraOps', {
await PageObjects.common.navigateToActualUrl('infraOps', '', {
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
basePath: '/s/custom_space',
});
await testSubjects.existOrFail('~appNotFoundPageContent');
await PageObjects.common.navigateToUrlWithBrowserHistory(
'infraOps',
'/inventory',
undefined,
{
basePath: '/s/custom_space',
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
}
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
})
);
await testSubjects.existOrFail('~appNotFoundPageContent');
});
});

Expand Down
24 changes: 12 additions & 12 deletions x-pack/test/functional/apps/infra/feature_controls/logs_security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,19 +187,19 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(navLinks).to.not.contain('Logs');
});

it(`logs app is inaccessible and Application Not Found message is rendered`, async () => {
await PageObjects.common.navigateToApp('infraLogs');
await testSubjects.existOrFail('~appNotFoundPageContent');
await PageObjects.common.navigateToUrlWithBrowserHistory(
'infraLogs',
'/stream',
undefined,
{
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
}
it(`logs app is inaccessible and returns a 404`, async () => {
await PageObjects.common.navigateToActualUrl('infraLogs', '', {
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
});
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
})
);
await testSubjects.existOrFail('~appNotFoundPageContent');
});
});
});
Expand Down
22 changes: 10 additions & 12 deletions x-pack/test/functional/apps/infra/feature_controls/logs_spaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,21 +80,19 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

it(`logs app is inaccessible and Application Not Found message is rendered`, async () => {
await PageObjects.common.navigateToApp('infraLogs', {
await PageObjects.common.navigateToActualUrl('infraLogs', '', {
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
basePath: '/s/custom_space',
});
await testSubjects.existOrFail('~appNotFoundPageContent');
await PageObjects.common.navigateToUrlWithBrowserHistory(
'infraLogs',
'/stream',
undefined,
{
basePath: '/s/custom_space',
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
}
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
})
);
await testSubjects.existOrFail('~appNotFoundPageContent');
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/ui_capabilities/common/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

interface Feature {
navLinkId: string;
app: string[];
}

export interface Features {
Expand Down
Loading

0 comments on commit 09b11b6

Please sign in to comment.