fixed sapces only suite

gmmorris · Jun 10, 2020 · 0aaaef5 · 0aaaef5
1 parent 6b789ec
commit 0aaaef5
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 108 deletions.
diff --git a/x-pack/plugins/alerts/server/alerts_authorization.mock.ts b/x-pack/plugins/alerts/server/alerts_authorization.mock.ts
@@ -12,7 +12,8 @@ export type AlertsAuthorizationMock = jest.Mocked<Schema>;
 const createAlertsAuthorizationMock = () => {
   const mocked: AlertsAuthorizationMock = {
     ensureAuthorized: jest.fn(),
-    filterByAuthorized: jest.fn(),
+    checkAlertTypeAuthorization: jest.fn(),
+    getFindAuthorizationFilter: jest.fn(),
   };
   return mocked;
 };

diff --git a/x-pack/plugins/alerts/server/alerts_authorization.ts b/x-pack/plugins/alerts/server/alerts_authorization.ts
@@ -87,7 +87,36 @@ export class AlertsAuthorization {
     }
   }
 
-  public async filterByAuthorized(
+  public async getFindAuthorizationFilter(): Promise<{
+    filter?: string;
+    ensureAlertTypeIsAuthorized: (alertTypeId: string) => void;
+  }> {
+    if (this.authorization) {
+      const authorizedAlertTypes = await this.checkAlertTypeAuthorization(
+        this.alertTypeRegistry.list(),
+        'find'
+      );
+
+      if (!authorizedAlertTypes.size) {
+        throw Boom.forbidden(`Unauthorized to find a any alert types`);
+      }
+
+      const authorizedAlertTypeIds = new Set(pluck([...authorizedAlertTypes], 'id'));
+      return {
+        filter: `(${this.asFiltersByAlertTypeAndConsumer(authorizedAlertTypes).join(' or ')})`,
+        ensureAlertTypeIsAuthorized: (alertTypeId: string) => {
+          if (!authorizedAlertTypeIds.has(alertTypeId)) {
+            throw Boom.forbidden(`Unauthorized to find "${alertTypeId}" alerts`);
+          }
+        },
+      };
+    }
+    return {
+      ensureAlertTypeIsAuthorized: (alertTypeId: string) => {},
+    };
+  }
+
+  public async checkAlertTypeAuthorization(
     alertTypes: Set<RegistryAlertType>,
     operation: string
   ): Promise<Set<RegistryAlertTypeWithAuth>> {
@@ -147,4 +176,15 @@ export class AlertsAuthorization {
       }))
     );
   }
+
+  private asFiltersByAlertTypeAndConsumer(alertTypes: Set<RegistryAlertTypeWithAuth>): string[] {
+    return Array.from(alertTypes).reduce<string[]>((filters, { id, authorizedConsumers }) => {
+      filters.push(
+        `(alert.attributes.alertTypeId:${id} and (${authorizedConsumers
+          .map((consumer) => `alert.attributes.consumer:${consumer}`)
+          .join(' or ')}))`
+      );
+      return filters;
+    }, []);
+  }
 }
diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts
@@ -2144,6 +2144,9 @@ describe('find()', () => {
     },
   ]);
   beforeEach(() => {
+    authorization.getFindAuthorizationFilter.mockResolvedValue({
+      ensureAlertTypeIsAuthorized() {},
+    });
     unsecuredSavedObjectsClient.find.mockResolvedValueOnce({
       total: 1,
       per_page: 10,
@@ -2179,7 +2182,7 @@ describe('find()', () => {
       ],
     });
     alertTypeRegistry.list.mockReturnValue(listedTypes);
-    authorization.filterByAuthorized.mockResolvedValue(
+    authorization.checkAlertTypeAuthorization.mockResolvedValue(
       new Set([
         {
           id: 'myType',
@@ -2230,7 +2233,6 @@ describe('find()', () => {
     expect(unsecuredSavedObjectsClient.find.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         Object {
-          "filter": "((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp))",
           "type": "alert",
         },
       ]
@@ -2239,51 +2241,28 @@ describe('find()', () => {
 
   describe('authorization', () => {
     test('ensures user is query filter types down to those the user is authorized to find', async () => {
-      authorization.filterByAuthorized.mockResolvedValue(
-        new Set([
-          {
-            id: 'myType',
-            name: 'Test',
-            actionGroups: [{ id: 'default', name: 'Default' }],
-            defaultActionGroupId: 'default',
-            producer: 'alerts',
-            authorizedConsumers: ['myApp'],
-          },
-          {
-            id: 'myOtherType',
-            name: 'Test',
-            actionGroups: [{ id: 'default', name: 'Default' }],
-            defaultActionGroupId: 'default',
-            producer: 'alerts',
-            authorizedConsumers: ['myApp', 'myOtherApp'],
-          },
-        ])
-      );
+      authorization.getFindAuthorizationFilter.mockResolvedValue({
+        filter:
+          '((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))',
+        ensureAlertTypeIsAuthorized() {},
+      });
 
       const alertsClient = new AlertsClient(alertsClientParams);
-      await alertsClient.find({ options: {} });
+      await alertsClient.find({ options: { filter: 'someTerm' } });
 
       const [options] = unsecuredSavedObjectsClient.find.mock.calls[0];
       expect(options.filter).toMatchInlineSnapshot(
-        `"((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))"`
+        `"someTerm and ((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))"`
       );
-      expect(authorization.filterByAuthorized).toHaveBeenCalledWith(listedTypes, 'find');
+      expect(authorization.getFindAuthorizationFilter).toHaveBeenCalledTimes(1);
     });
 
-    test('short circuits if user is not authorized to find any types', async () => {
-      authorization.filterByAuthorized.mockResolvedValue(new Set([]));
-
+    test('throws if user is not authorized to find any types', async () => {
       const alertsClient = new AlertsClient(alertsClientParams);
-      expect(await alertsClient.find({ options: {} })).toEqual({
-        data: [],
-        page: 0,
-        perPage: 0,
-        total: 0,
-      });
-
-      expect(unsecuredSavedObjectsClient.find).toHaveBeenCalledTimes(0);
-
-      expect(authorization.filterByAuthorized).toHaveBeenCalledWith(listedTypes, 'find');
+      authorization.getFindAuthorizationFilter.mockRejectedValue(new Error('not authorized'));
+      await expect(alertsClient.find({ options: {} })).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"not authorized"`
+      );
     });
   });
 });
@@ -3679,7 +3658,7 @@ describe('listAlertTypes', () => {
 
   test('should return a list of AlertTypes that exist in the registry', async () => {
     alertTypeRegistry.list.mockReturnValue(setOfAlertTypes);
-    authorization.filterByAuthorized.mockResolvedValue(
+    authorization.checkAlertTypeAuthorization.mockResolvedValue(
       new Set([
         { ...myAppAlertType, authorizedConsumers: ['alerts', 'myApp', 'myOtherApp'] },
         { ...alertingAlertType, authorizedConsumers: ['alerts', 'myApp', 'myOtherApp'] },
@@ -3726,7 +3705,7 @@ describe('listAlertTypes', () => {
           authorizedConsumers: ['myApp'],
         },
       ]);
-      authorization.filterByAuthorized.mockResolvedValue(authorizedTypes);
+      authorization.checkAlertTypeAuthorization.mockResolvedValue(authorizedTypes);
 
       expect(await alertsClient.listAlertTypes()).toEqual(authorizedTypes);
     });

diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts
@@ -247,38 +247,25 @@ export class AlertsClient {
     }
   }
 
-  public async find({
-    options: { filter, ...options } = {},
-  }: { options?: FindOptions } = {}): Promise<FindResult> {
-    const filters = filter ? [filter] : [];
-
-    const authorizedAlertTypes = await this.authorization.filterByAuthorized(
-      this.alertTypeRegistry.list(),
-      'find'
-    );
-    const authorizedAlertTypeIds = new Set(pluck([...authorizedAlertTypes], 'id'));
-
-    if (!authorizedAlertTypes.size) {
-      // the current user isn't authorized to get any alertTypes
-      // we can short circuit here
-      return {
-        page: 0,
-        perPage: 0,
-        total: 0,
-        data: [],
-      };
+  public async find({ options = {} }: { options?: FindOptions } = {}): Promise<FindResult> {
+    const {
+      filter: authorizationFilter,
+      ensureAlertTypeIsAuthorized,
+    } = await this.authorization.getFindAuthorizationFilter();
+
+    if (authorizationFilter) {
+      options.filter = options.filter
+        ? `${options.filter} and ${authorizationFilter}`
+        : authorizationFilter;
     }
 
-    filters.push(`(${this.asFiltersByAlertTypeAndConsumer(authorizedAlertTypes).join(' or ')})`);
-
     const {
       page,
       per_page: perPage,
       total,
       saved_objects: data,
     } = await this.unsecuredSavedObjectsClient.find<RawAlert>({
       ...options,
-      filter: filters.join(` and `),
       type: 'alert',
     });
 
@@ -287,9 +274,7 @@ export class AlertsClient {
       perPage,
       total,
       data: data.map(({ id, attributes, updated_at, references }) => {
-        if (!authorizedAlertTypeIds.has(attributes.alertTypeId)) {
-          throw Boom.forbidden(`Unauthorized to find "${attributes.alertTypeId}" alerts`);
-        }
+        ensureAlertTypeIsAuthorized(attributes.alertTypeId);
         return this.getAlertFromRaw(id, attributes, updated_at, references);
       }),
     };
@@ -688,7 +673,10 @@ export class AlertsClient {
   }
 
   public async listAlertTypes() {
-    return await this.authorization.filterByAuthorized(this.alertTypeRegistry.list(), 'get');
+    return await this.authorization.checkAlertTypeAuthorization(
+      this.alertTypeRegistry.list(),
+      'get'
+    );
   }
 
   private async scheduleAlert(id: string, alertTypeId: string) {
@@ -808,15 +796,4 @@ export class AlertsClient {
       references,
     };
   }
-
-  private asFiltersByAlertTypeAndConsumer(alertTypes: Set<RegistryAlertTypeWithAuth>): string[] {
-    return Array.from(alertTypes).reduce<string[]>((filters, { id, authorizedConsumers }) => {
-      for (const consumer of authorizedConsumers) {
-        filters.push(
-          `(alert.attributes.alertTypeId:${id} and alert.attributes.consumer:${consumer})`
-        );
-      }
-      return filters;
-    }, []);
-  }
 }