From f6af7403300127235ba1ea616bdf334d4c917c1a Mon Sep 17 00:00:00 2001 From: "Michael B. Klein" Date: Mon, 17 Jun 2024 18:16:59 +0000 Subject: [PATCH] Refactor auth filter to use temporary search pipeline --- node/package.json | 1 + node/src/api/request/pipeline.js | 36 ++++++-- node/src/package-lock.json | 4 +- node/test/unit/api/request/pipeline.test.js | 91 ++++++++++++--------- 4 files changed, 84 insertions(+), 48 deletions(-) diff --git a/node/package.json b/node/package.json index 36a09936..f0c363ed 100644 --- 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 index d60fdd1f..5798033d 100644 --- 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 index b451bfec..59d345eb 100644 --- a/node/src/package-lock.json +++ b/node/src/package-lock.json @@ -1,12 +1,12 @@ { "name": "dc-api", - "version": "2.2.0", + "version": "2.3.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "dc-api", - "version": "2.2.0", + "version": "2.3.1", "license": "Apache-2.0", "devDependencies": { "@aws-crypto/sha256-browser": "^2.0.1", diff --git a/node/test/unit/api/request/pipeline.test.js b/node/test/unit/api/request/pipeline.test.js index f7e039f4..16b21e69 100644 --- 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"); }); }); });