Refactor auth filter to use temporary search pipeline

nulib · Jun 17, 2024 · f6af740 · f6af740
1 parent d7637c7
commit f6af740
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 48 deletions.
diff --git a/node/package.json b/node/package.json
@@ -22,6 +22,7 @@
     "chai": "^4.2.0",
     "chai-http": "^4.3.0",
     "choma": "^1.2.1",
+    "deep-equal-in-any-order": "^2.0.6",
     "eslint": "^8.32.0",
     "eslint-plugin-json": "^3.1.0",
     "husky": "^8.0.3",

diff --git a/node/src/api/request/pipeline.js b/node/src/api/request/pipeline.js
@@ -1,18 +1,27 @@
 const sortJson = require("sort-json");
 
-function filterFor(query, event) {
-  const matchTheQuery = query;
+function filterFor(event) {
   const beUnpublished = { term: { published: false } };
   const beRestricted = { term: { visibility: "Private" } };
 
-  let filter = { must: [matchTheQuery] };
-  if (!event.userToken.isSuperUser()) {
-    filter.must_not = event.userToken.isReadingRoom()
-      ? [beUnpublished]
-      : [beUnpublished, beRestricted];
+  if (event.userToken.isSuperUser()) {
+    return null;
   }
 
-  return { bool: filter };
+  return {
+    filter_query: {
+      tag: "access_filter",
+      description:
+        "Restricts access to unpublished and restricted items based on user's access level",
+      query: {
+        bool: {
+          must_not: event.userToken.isReadingRoom()
+            ? [beUnpublished]
+            : [beUnpublished, beRestricted],
+        },
+      },
+    },
+  };
 }
 
 module.exports = class RequestPipeline {
@@ -28,7 +37,16 @@ module.exports = class RequestPipeline {
   // - Add `track_total_hits` to search context (so we can get accurate hits.total.value)
 
   authFilter(event) {
-    this.searchContext.query = filterFor(this.searchContext.query, event);
+    if (event.queryStringParameters?.search_pipeline) {
+      return this;
+    }
+
+    const filterProcessor = filterFor(event);
+    if (filterProcessor != null) {
+      this.searchContext.search_pipeline = {
+        request_processors: [filterProcessor],
+      };
+    }
     this.searchContext.track_total_hits = true;
 
     return this;

diff --git a/node/src/package-lock.json b/node/src/package-lock.json
diff --git a/node/test/unit/api/request/pipeline.test.js b/node/test/unit/api/request/pipeline.test.js
@@ -1,16 +1,25 @@
 "use strict";
 
 const chai = require("chai");
+const deepEqualInAnyOrder = require("deep-equal-in-any-order");
 const expect = chai.expect;
 
 const ApiToken = requireSource("api/api-token");
 const RequestPipeline = requireSource("api/request/pipeline");
 
+chai.use(deepEqualInAnyOrder);
+
+const findFilterQuery = (searchContext) => {
+  if (!searchContext.search_pipeline?.request_processors) return null;
+  const filter = searchContext.search_pipeline.request_processors.find(
+    (processor) => processor?.filter_query?.tag == "access_filter"
+  );
+  return filter?.filter_query?.query?.bool;
+};
+
 describe("RequestPipeline", () => {
   helpers.saveEnvironment();
 
-  let event = helpers.mockEvent("GET", "/search").render();
-
   const requestBody = {
     query: { match: { term: { title: "The Title" } } },
     size: 50,
@@ -20,8 +29,9 @@ describe("RequestPipeline", () => {
     aggs: { collection: { terms: { field: "contributor.label", size: 10 } } },
   };
 
-  let pipeline;
+  let event, pipeline;
   beforeEach(() => {
+    event = helpers.mockEvent("GET", "/search").render();
     pipeline = new RequestPipeline(requestBody);
   });
 
@@ -30,11 +40,13 @@ describe("RequestPipeline", () => {
 
     const result = pipeline.authFilter(helpers.preprocess(event));
     expect(result.searchContext.size).to.eq(50);
-    expect(result.searchContext.query.bool.must).to.include(requestBody.query);
-    expect(result.searchContext.query.bool.must_not).to.deep.include(
-      { term: { visibility: "Private" } },
-      { term: { published: false } }
-    );
+    expect(result.searchContext.query).to.eq(requestBody.query);
+    expect(findFilterQuery(result.searchContext)).to.deep.equalInAnyOrder({
+      must_not: [
+        { term: { visibility: "Private" } },
+        { term: { published: false } },
+      ],
+    });
   });
 
   it("serializes JSON", () => {
@@ -48,28 +60,23 @@ describe("RequestPipeline", () => {
       // process.env.READING_ROOM_IPS = "192.168.0.1,172.16.10.2";
       const result = pipeline.authFilter(helpers.preprocess(event));
       expect(result.searchContext.size).to.eq(50);
-      expect(result.searchContext.query.bool.must).to.include(
-        requestBody.query
-      );
-      expect(result.searchContext.query.bool.must_not).to.deep.include(
-        { term: { visibility: "Private" } },
-        { term: { published: false } }
-      );
+      expect(result.searchContext.query).to.eq(requestBody.query);
+      expect(findFilterQuery(result.searchContext)).to.deep.equalInAnyOrder({
+        must_not: [
+          { term: { visibility: "Private" } },
+          { term: { published: false } },
+        ],
+      });
     });
 
     it("includes private results if the user is in the reading room", () => {
+      event = helpers.preprocess(event);
       event.userToken = new ApiToken().readingRoom();
-
-      const result = pipeline.authFilter(helpers.preprocess(event));
+      const result = pipeline.authFilter(event);
       expect(result.searchContext.size).to.eq(50);
-      expect(result.searchContext.query.bool.must).to.include(
-        requestBody.query
-      );
-      expect(result.searchContext.query.bool.must_not).to.deep.include({
-        term: { published: false },
-      });
-      expect(result.searchContext.query.bool.must_not).not.to.deep.include({
-        term: { visibility: "Private" },
+      expect(result.searchContext.query).to.eq(requestBody.query);
+      expect(findFilterQuery(result.searchContext)).to.deep.equal({
+        must_not: [{ term: { published: false } }],
       });
     });
   });
@@ -81,24 +88,34 @@ describe("RequestPipeline", () => {
       // process.env.READING_ROOM_IPS = "192.168.0.1,172.16.10.2";
       const result = pipeline.authFilter(helpers.preprocess(event));
       expect(result.searchContext.size).to.eq(50);
-      expect(result.searchContext.query.bool.must).to.include(
-        requestBody.query
-      );
-      expect(result.searchContext.query.bool.must_not).to.deep.include(
-        { term: { visibility: "Private" } },
-        { term: { published: false } }
-      );
+      expect(result.searchContext.query).to.eq(requestBody.query);
+      expect(findFilterQuery(result.searchContext)).to.deep.equalInAnyOrder({
+        must_not: [
+          { term: { visibility: "Private" } },
+          { term: { published: false } },
+        ],
+      });
     });
 
     it("includes private results if the user is in the reading room", () => {
+      event = helpers.preprocess(event);
       event.userToken = new ApiToken().superUser();
 
-      const result = pipeline.authFilter(helpers.preprocess(event));
+      const result = pipeline.authFilter(event);
       expect(result.searchContext.size).to.eq(50);
-      expect(result.searchContext.query.bool.must).to.include(
-        requestBody.query
-      );
-      expect(result.searchContext.query.bool).not.to.have.any.keys("must_not");
+      expect(result.searchContext.query).to.eq(requestBody.query);
+      expect(findFilterQuery(result.searchContext)).to.be.null;
+    });
+  });
+
+  describe("search_pipeline in request", () => {
+    it("does not add a search filter when a pipeline is specified", () => {
+      event.queryStringParameters = {
+        ...event.queryStringParameters,
+        search_pipeline: "alternate-pipeline",
+      };
+      const result = pipeline.authFilter(helpers.preprocess(event));
+      expect(result.searchContext).not.to.have.keys("search_pipeline");
     });
   });
 });