From af4864b46234114f21aef03f5c4f3caf8151f93a Mon Sep 17 00:00:00 2001
From: MT Lewis <mtlewis@users.noreply.github.com>
Date: Wed, 26 Jan 2022 19:38:01 +0000
Subject: [PATCH] search-backend: deduplicate authorize calls

Signed-off-by: MT Lewis <mtlewis@users.noreply.github.com>
---
 plugins/search-backend/package.json           |  1 +
 .../service/AuthorizedSearchEngine.test.ts    | 66 +++++++++++++++++++
 .../src/service/AuthorizedSearchEngine.ts     | 13 +++-
 3 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/plugins/search-backend/package.json b/plugins/search-backend/package.json
index 1db6a0a910..049c199164 100644
--- a/plugins/search-backend/package.json
+++ b/plugins/search-backend/package.json
@@ -34,6 +34,7 @@
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",
     "lodash": "^4.17.21",
+    "qs": "^6.10.1",
     "winston": "^3.2.1",
     "yn": "^4.0.0",
     "zod": "^3.11.6"
diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts
index 878e24bb07..0a7b45e164 100644
--- a/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts
+++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.test.ts
@@ -257,6 +257,72 @@ describe('AuthorizedSearchEngine', () => {
     );
   });
 
+  it('should deduplicate authorization queries when resourceRefs match', async () => {
+    const searchResults = [
+      {
+        type: 'templates',
+        document: {
+          title: `doc_0_a`,
+          authorization: { resourceRef: `template_doc_0` },
+        } as IndexableDocument,
+      },
+      {
+        type: 'templates',
+        document: {
+          title: `doc_0_b`,
+          authorization: { resourceRef: `template_doc_0` },
+        } as IndexableDocument,
+      },
+    ];
+
+    mockedQuery.mockImplementation(async () => ({
+      results: searchResults,
+    }));
+
+    mockedAuthorize.mockImplementation(async queries =>
+      queries.map(query => {
+        if (query.resourceRef) {
+          return {
+            result: AuthorizeResult.ALLOW,
+          };
+        }
+
+        return {
+          result: AuthorizeResult.CONDITIONAL,
+        } as AuthorizeDecision;
+      }),
+    );
+
+    await expect(
+      authorizedSearchEngine.query({ term: '', types: ['templates'] }, options),
+    ).resolves.toEqual({ results: searchResults });
+
+    expect(mockedAuthorize).toHaveBeenCalledTimes(2);
+    expect(mockedAuthorize).toHaveBeenNthCalledWith(
+      1,
+      [
+        {
+          permission: expect.objectContaining({
+            name: 'search.templates.read',
+          }),
+        },
+      ],
+      { token: 'token' },
+    );
+    expect(mockedAuthorize).toHaveBeenNthCalledWith(
+      2,
+      [
+        {
+          permission: expect.objectContaining({
+            name: 'search.templates.read',
+          }),
+          resourceRef: 'template_doc_0',
+        },
+      ],
+      { token: 'token' },
+    );
+  });
+
   it('should perform search until the target number of results is reached', async () => {
     mockedAuthorize.mockImplementation(async queries =>
       queries.map(query => {
diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts
index d13fb2c02a..d0488af00d 100644
--- a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts
+++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts
@@ -15,6 +15,7 @@
  */
 
 import { compact, zipObject } from 'lodash';
+import qs from 'qs';
 import DataLoader from 'dataloader';
 import {
   AuthorizeDecision,
@@ -87,8 +88,16 @@ export class AuthorizedSearchEngine implements SearchEngine {
   ): Promise<SearchResultSet> {
     const queryStartTime = Date.now();
 
-    const authorizer = new DataLoader((requests: readonly AuthorizeQuery[]) =>
-      this.permissions.authorize(requests.slice(), options),
+    const authorizer = new DataLoader(
+      (requests: readonly AuthorizeQuery[]) =>
+        this.permissions.authorize(requests.slice(), options),
+      {
+        // Serialize the permission name and resourceRef as
+        // a query string to avoid collisions from overlapping
+        // permission names and resourceRefs.
+        cacheKeyFn: ({ permission: { name }, resourceRef }) =>
+          qs.stringify({ name, resourceRef }),
+      },
     );
     const requestedTypes = query.types || Object.keys(this.types);