Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce reserved ml privilege for the apm_user role #72266

Merged
merged 12 commits into from
Jul 28, 2020
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'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a silly question: why is this PR updating the infra app and not the apm app?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APM is already "compliant" with the new requirement that your navLinkId value must be included in your app array. Infra was one of the few that weren't already configured this way:

navLinkId: 'apm',
app: ['apm', 'kibana'],

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 @@ -96,6 +101,17 @@ export function getPluginPrivileges() {
api: userMlCapabilitiesKeys.map((k) => `ml:${k}`),
ui: userMlCapabilitiesKeys,
},
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', '', {
legrego marked this conversation as resolved.
Show resolved Hide resolved
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