added more acceptance tests around alert deletion, enabling, find and…

… get with auth
gmmorris · Jun 8, 2020 · e15946c · e15946c
1 parent 2b84902
commit e15946c
Show file tree

Hide file tree

Showing 25 changed files with 950 additions and 108 deletions.
diff --git a/examples/alerting_example/server/plugin.ts b/examples/alerting_example/server/plugin.ts
@@ -42,9 +42,7 @@ export class AlertingExamplePlugin implements Plugin<void, void, AlertingExample
       privileges: {
         all: {
           alerting: {
-            globally: {
-              all: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
-            },
+            all: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
           },
           savedObject: {
             all: [],
@@ -54,9 +52,7 @@ export class AlertingExamplePlugin implements Plugin<void, void, AlertingExample
         },
         read: {
           alerting: {
-            globally: {
-              read: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
-            },
+            read: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
           },
           savedObject: {
             all: [],

diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts
@@ -5,7 +5,7 @@
  */
 
 import Boom from 'boom';
-import { omit, isEqual, pluck } from 'lodash';
+import { omit, isEqual, pluck, mapValues } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import {
   Logger,
@@ -670,29 +670,41 @@ export class AlertsClient {
     const { authorization } = this;
     if (authorization) {
       const alertType = this.alertTypeRegistry.get(alertTypeId);
-      const requiredPrivilegedScopes =
+      const requiredPrivilegesByScope = {
+        consumer: authorization.actions.alerting.get(alertTypeId, consumer, operation),
+        producer: authorization.actions.alerting.get(alertTypeId, alertType.producer, operation),
+      };
+
+      const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
+      const { hasAllRequested, privileges } = await checkPrivileges(
         consumer === AlertsFeatureId || consumer === alertType.producer
           ? [
               // skip consumer privilege checks under `alerts` as all alert types can
               // be created under `alerts` if you have producer level privileges
-              alertType.producer,
+              requiredPrivilegesByScope.producer,
             ]
           : [
               // check for access at consumer level
-              consumer,
+              requiredPrivilegesByScope.consumer,
               // check for access at producer level
-              alertType.producer,
-            ];
-
-      const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
-      const { hasAllRequested } = await checkPrivileges(
-        requiredPrivilegedScopes.map((scope) =>
-          authorization.actions.alerting.get(alertTypeId, scope, operation)
-        )
+              requiredPrivilegesByScope.producer,
+            ]
       );
+
       if (!hasAllRequested) {
+        const authorizedPrivileges = pluck(
+          privileges.filter((privilege) => privilege.authorized),
+          'privilege'
+        );
+        const unauthorizedScopes = mapValues(
+          requiredPrivilegesByScope,
+          (privilege) => !authorizedPrivileges.includes(privilege)
+        );
+
         throw Boom.forbidden(
-          `Unauthorized to ${operation} a "${alertTypeId}" alert for "${consumer}"`
+          `Unauthorized to ${operation} a "${alertTypeId}" alert ${
+            unauthorizedScopes.consumer ? `for "${consumer}"` : `by "${alertType.producer}"`
+          }`
         );
       }
     }

diff --git a/x-pack/plugins/features/server/feature_schema.ts b/x-pack/plugins/features/server/feature_schema.ts
@@ -36,10 +36,6 @@ const privilegeSchema = Joi.object({
   alerting: Joi.object({
     all: Joi.array().items(Joi.string()),
     read: Joi.array().items(Joi.string()),
-    globally: Joi.object({
-      all: Joi.array().items(Joi.string()),
-      read: Joi.array().items(Joi.string()),
-    }),
   }),
   savedObject: Joi.object({
     all: Joi.array().items(Joi.string()).required(),

diff --git a/x-pack/plugins/security/server/authorization/actions/__snapshots__/alerting.test.ts.snap b/x-pack/plugins/security/server/authorization/actions/__snapshots__/alerting.test.ts.snap
diff --git a/x-pack/plugins/security/server/index.ts b/x-pack/plugins/security/server/index.ts
@@ -30,7 +30,7 @@ export {
 export { AuditLogger } from './audit';
 export { SecurityPluginSetup };
 export { AuthenticatedUser } from '../common/model';
-export { Actions, CheckPrivilegesResponse } from './authorization';
+export { Actions } from './authorization';
 
 export const config: PluginConfigDescriptor<TypeOf<typeof ConfigSchema>> = {
   schema: ConfigSchema,

diff --git a/.../test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/plugin.ts b/.../test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/plugin.ts
@@ -35,12 +35,7 @@ export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, Fixtu
             read: [],
           },
           alerting: {
-            all: [
-              'test.restricted-noop',
-              'test.unrestricted-noop',
-              'test.fake-built-in',
-              'test.noop',
-            ],
+            all: ['test.restricted-noop', 'test.unrestricted-noop', 'test.noop'],
           },
           ui: [],
         },
@@ -51,7 +46,7 @@ export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, Fixtu
             read: ['alert'],
           },
           alerting: {
-            read: ['test.restricted-noop', 'test.unrestricted-noop', 'test.fake-built-in'],
+            read: ['test.restricted-noop', 'test.unrestricted-noop'],
           },
           ui: [],
         },

diff --git a/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts b/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts
@@ -267,14 +267,22 @@ export class AlertUtils {
   }
 }
 
-export function getUnauthorizedErrorMessage(
+export function getConsumerUnauthorizedErrorMessage(
   operation: string,
   alertType: string,
   consumer: string
 ) {
   return `Unauthorized to ${operation} a "${alertType}" alert for "${consumer}"`;
 }
 
+export function getProducerUnauthorizedErrorMessage(
+  operation: string,
+  alertType: string,
+  producer: string
+) {
+  return `Unauthorized to ${operation} a "${alertType}" alert by "${producer}"`;
+}
+
 function getDefaultAlwaysFiringAlertData(reference: string, actionId: string) {
   const messageTemplate = `
 alertId: {{alertId}},

diff --git a/x-pack/test/alerting_api_integration/common/lib/index.ts b/x-pack/test/alerting_api_integration/common/lib/index.ts
@@ -8,7 +8,11 @@ export { ObjectRemover } from './object_remover';
 export { getUrlPrefix } from './space_test_utils';
 export { ES_TEST_INDEX_NAME, ESTestIndexTool } from './es_test_index_tool';
 export { getTestAlertData } from './get_test_alert_data';
-export { AlertUtils, getUnauthorizedErrorMessage } from './alert_utils';
+export {
+  AlertUtils,
+  getConsumerUnauthorizedErrorMessage,
+  getProducerUnauthorizedErrorMessage,
+} from './alert_utils';
 export { TaskManagerUtils } from './task_manager_utils';
 export * from './test_assertions';
 export { checkAAD } from './check_aad';
diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/scenarios.ts b/x-pack/test/alerting_api_integration/security_and_spaces/scenarios.ts
@@ -79,7 +79,6 @@ const Space1All: User = {
           alerts: ['all'],
           actions: ['all'],
           alertsFixture: ['all'],
-          alertsRestrictedFixture: ['read'],
         },
         spaces: ['space1'],
       },

diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts
@@ -14,7 +14,7 @@ import {
   getTestAlertData,
   ObjectRemover,
   AlertUtils,
-  getUnauthorizedErrorMessage,
+  getConsumerUnauthorizedErrorMessage,
   TaskManagerUtils,
 } from '../../../common/lib';
 
@@ -87,7 +87,7 @@ export default function alertTests({ getService }: FtrProviderContext) {
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -186,7 +186,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -378,7 +378,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -467,7 +467,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.authorization',
                   'alertsFixture'
@@ -586,7 +586,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -675,7 +675,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -746,7 +746,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -801,7 +801,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -848,7 +848,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -898,7 +898,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'
@@ -948,7 +948,7 @@ instanceStateValue: true
               expect(response.statusCode).to.eql(403);
               expect(response.body).to.eql({
                 error: 'Forbidden',
-                message: getUnauthorizedErrorMessage(
+                message: getConsumerUnauthorizedErrorMessage(
                   'create',
                   'test.always-firing',
                   'alertsFixture'