Introduce reserved ml privilege for the apm_user role (elastic#72266)

Co-authored-by: Elastic Machine <[email protected]>
legrego · Jul 28, 2020 · 357bc9a · 357bc9a
1 parent 88b373f
commit 357bc9a
Show file tree

Hide file tree

Showing 21 changed files with 102 additions and 107 deletions.
diff --git a/x-pack/plugins/infra/server/features.ts b/x-pack/plugins/infra/server/features.ts
@@ -14,11 +14,11 @@ export const METRICS_FEATURE = {
   order: 700,
   icon: 'metricsApp',
   navLinkId: 'metrics',
-  app: ['infra', 'kibana'],
+  app: ['infra', 'metrics', 'kibana'],
   catalogue: ['infraops'],
   privileges: {
     all: {
-      app: ['infra', 'kibana'],
+      app: ['infra', 'metrics', 'kibana'],
       catalogue: ['infraops'],
       api: ['infra', 'actions-read', 'actions-all', 'alerting-read', 'alerting-all'],
       savedObject: {
@@ -38,7 +38,7 @@ export const METRICS_FEATURE = {
       ],
     },
     read: {
-      app: ['infra', 'kibana'],
+      app: ['infra', 'metrics', 'kibana'],
       catalogue: ['infraops'],
       api: ['infra', 'actions-read', 'actions-all', 'alerting-read', 'alerting-all'],
       savedObject: {
@@ -66,11 +66,11 @@ export const LOGS_FEATURE = {
   order: 800,
   icon: 'logsApp',
   navLinkId: 'logs',
-  app: ['infra', 'kibana'],
+  app: ['infra', 'logs', 'kibana'],
   catalogue: ['infralogging'],
   privileges: {
     all: {
-      app: ['infra', 'kibana'],
+      app: ['infra', 'logs', 'kibana'],
       catalogue: ['infralogging'],
       api: ['infra'],
       savedObject: {
@@ -80,7 +80,7 @@ export const LOGS_FEATURE = {
       ui: ['show', 'configureSource', 'save'],
     },
     read: {
-      app: ['infra', 'kibana'],
+      app: ['infra', 'logs', 'kibana'],
       catalogue: ['infralogging'],
       api: ['infra'],
       savedObject: {

diff --git a/x-pack/plugins/ml/common/types/capabilities.ts b/x-pack/plugins/ml/common/types/capabilities.ts
@@ -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
@@ -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];
@@ -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,
+    },
   };
 }
 

diff --git a/x-pack/plugins/ml/server/plugin.ts b/x-pack/plugins/ml/server/plugin.ts
@@ -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,
@@ -108,6 +108,10 @@ export class MlServerPlugin implements Plugin<MlPluginSetup, MlPluginStart, Plug
             id: 'ml_admin',
             privilege: admin,
           },
+          {
+            id: 'ml_apm_user',
+            privilege: apmUser,
+          },
         ],
       },
     });

diff --git a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.test.ts
@@ -50,7 +50,7 @@ describe('usingPrivileges', () => {
           new Feature({
             id: 'fooFeature',
             name: 'Foo Feature',
-            app: ['fooApp'],
+            app: ['fooApp', 'foo'],
             navLinkId: 'foo',
             privileges: null,
           }),
@@ -126,7 +126,7 @@ describe('usingPrivileges', () => {
           new Feature({
             id: 'fooFeature',
             name: 'Foo Feature',
-            app: [],
+            app: ['foo'],
             navLinkId: 'foo',
             privileges: null,
           }),
@@ -259,7 +259,7 @@ describe('usingPrivileges', () => {
           id: 'barFeature',
           name: 'Bar Feature',
           navLinkId: 'bar',
-          app: [],
+          app: ['bar'],
           privileges: null,
         }),
       ],
@@ -409,7 +409,7 @@ describe('all', () => {
         new Feature({
           id: 'fooFeature',
           name: 'Foo Feature',
-          app: [],
+          app: ['foo'],
           navLinkId: 'foo',
           privileges: null,
         }),

diff --git a/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts b/x-pack/plugins/security/server/authorization/disable_ui_capabilities.ts
@@ -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 = (

diff --git a/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/navlink.ts b/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/navlink.ts
@@ -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));
   }
 }
diff --git a/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts b/x-pack/plugins/security/server/authorization/privileges/privileges.test.ts
@@ -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],
     });
   });
 
@@ -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'),
@@ -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'),
@@ -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`, () => {

diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts
@@ -23,7 +23,7 @@ const features = ([
     id: 'feature_2',
     name: 'Feature 2',
     navLinkId: 'feature2',
-    app: [],
+    app: ['feature2'],
     catalogue: ['feature2Entry'],
     management: {
       kibana: ['somethingElse'],

diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts
@@ -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;
       }

diff --git a/x-pack/test/api_integration/apis/security/privileges.ts b/x-pack/test/api_integration/apis/security/privileges.ts
@@ -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

diff --git a/x-pack/test/api_integration/apis/security/privileges_basic.ts b/x-pack/test/api_integration/apis/security/privileges_basic.ts
@@ -39,7 +39,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

diff --git a/x-pack/test/functional/apps/infra/feature_controls/infrastructure_security.ts b/x-pack/test/functional/apps/infra/feature_controls/infrastructure_security.ts
@@ -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');
       });
     });
   });

diff --git a/x-pack/test/functional/apps/infra/feature_controls/infrastructure_spaces.ts b/x-pack/test/functional/apps/infra/feature_controls/infrastructure_spaces.ts
@@ -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');
       });
     });
 

diff --git a/x-pack/test/functional/apps/infra/feature_controls/logs_security.ts b/x-pack/test/functional/apps/infra/feature_controls/logs_security.ts
@@ -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');
       });
     });
   });

diff --git a/x-pack/test/functional/apps/infra/feature_controls/logs_spaces.ts b/x-pack/test/functional/apps/infra/feature_controls/logs_spaces.ts
@@ -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');
       });
     });
   });

diff --git a/x-pack/test/ui_capabilities/common/features.ts b/x-pack/test/ui_capabilities/common/features.ts
@@ -5,7 +5,7 @@
  */
 
 interface Feature {
-  navLinkId: string;
+  app: string[];
 }
 
 export interface Features {