Skip to content

Commit 7531931

Browse files
mbkleinmbklein
committed
Refactor auth filtering to be more efficient
Add `visibility` query param Update OpenAPI spec
1 parent 67f21be commit 7531931

4 files changed

Lines changed: 120 additions & 51 deletions

File tree

‎api/src/api/request/pipeline.js‎

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,57 @@
11
const sortJson = require("sort-json");
22
const { defaultSearchSize } = require("../../environment");
33

4-
function filterFor(query, event) {
5-
const matchTheQuery = query;
6-
const beUnpublished = { term: { published: false } };
7-
const beRestricted = { term: { visibility: "Private" } };
8-
9-
let filter = { must: [matchTheQuery], must_not: [] };
10-
11-
if (!event.userToken.can("read:Unpublished")) {
12-
filter.must_not.push(beUnpublished);
13-
}
4+
function filterFor(event) {
5+
const publishedValues = event.userToken.can("read:Unpublished")
6+
? [true, false]
7+
: [true];
8+
const userVisibility = new Set(
9+
event.userToken.can("read:Private")
10+
? ["Private", "Institution", "Public"]
11+
: ["Institution", "Public"]
12+
);
13+
const requestVisibility = event?.queryStringParameters?.visibility
14+
?.split(",")
15+
?.map((v) => v[0].toUpperCase() + v.slice(1)) || [
16+
"Private",
17+
"Institution",
18+
"Public",
19+
];
20+
const visibilityValues = requestVisibility.filter((v) =>
21+
userVisibility.has(v)
22+
);
23+
24+
return [
25+
{ terms: { published: publishedValues } },
26+
{ terms: { visibility: visibilityValues } },
27+
];
28+
}
1429

15-
if (!event.userToken.can("read:Private")) {
16-
filter.must_not.push(beRestricted);
30+
function addFilter(query, filter) {
31+
let result = { ...query };
32+
if (result.bool) {
33+
result.bool.filter ||= [];
34+
result.bool.filter.push(...filter);
35+
} else if (result.neural) {
36+
const boolFilter = { bool: { filter: filter } };
37+
if (result.neural.filter) {
38+
boolFilter.bool.filter.push(result.neural.filter);
39+
}
40+
const neuralField = Object.keys(result.neural)[0];
41+
result.neural[neuralField].filter = boolFilter;
42+
} else if (result.hybrid) {
43+
result.hybrid.queries = result.hybrid.queries.map((subQuery) =>
44+
addFilter(subQuery, filter)
45+
);
46+
} else {
47+
result = {
48+
bool: {
49+
must: [result],
50+
filter: filter,
51+
},
52+
};
1753
}
18-
19-
return { bool: filter };
54+
return result;
2055
}
2156

2257
module.exports = class RequestPipeline {
@@ -33,19 +68,11 @@ module.exports = class RequestPipeline {
3368
// - Add `track_total_hits` to search context (so we can get accurate hits.total.value)
3469

3570
authFilter(event) {
36-
if (this.searchContext.query?.hybrid?.queries) {
37-
this.searchContext.query = {
38-
hybrid: {
39-
queries: this.searchContext.query.hybrid.queries.map((query) =>
40-
filterFor(query, event)
41-
),
42-
},
43-
};
44-
} else {
45-
this.searchContext.query = filterFor(this.searchContext.query, event);
46-
}
71+
this.searchContext.query = addFilter(
72+
this.searchContext.query,
73+
filterFor(event)
74+
);
4775
this.searchContext.track_total_hits = true;
48-
4976
return this;
5077
}
5178

‎api/test/unit/api/request/pipeline.test.js‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ describe("RequestPipeline", () => {
3636
expect(result.searchContext.query.bool.must).to.deep.include(
3737
requestBody.query
3838
);
39-
expect(result.searchContext.query.bool.must_not).to.deep.include(
40-
{ term: { visibility: "Private" } },
41-
{ term: { published: false } }
39+
expect(result.searchContext.query.bool.filter).to.deep.include(
40+
{ terms: { visibility: ["Institution", "Public"] } },
41+
{ terms: { published: [true] } }
4242
);
4343
});
4444

@@ -56,9 +56,9 @@ describe("RequestPipeline", () => {
5656
expect(result.searchContext.query.bool.must).to.deep.include(
5757
requestBody.query
5858
);
59-
expect(result.searchContext.query.bool.must_not).to.deep.include(
60-
{ term: { visibility: "Private" } },
61-
{ term: { published: false } }
59+
expect(result.searchContext.query.bool.filter).to.deep.include(
60+
{ terms: { visibility: ["Institution", "Public"] } },
61+
{ terms: { published: [true] } }
6262
);
6363
});
6464

@@ -71,11 +71,11 @@ describe("RequestPipeline", () => {
7171
expect(result.searchContext.query.bool.must).to.deep.include(
7272
requestBody.query
7373
);
74-
expect(result.searchContext.query.bool.must_not).to.deep.include({
75-
term: { published: false },
74+
expect(result.searchContext.query.bool.filter).to.deep.include({
75+
terms: { published: [true] },
7676
});
77-
expect(result.searchContext.query.bool.must_not).not.to.deep.include({
78-
term: { visibility: "Private" },
77+
expect(result.searchContext.query.bool.filter).to.deep.include({
78+
terms: { visibility: ["Private", "Institution", "Public"] },
7979
});
8080
});
8181
});
@@ -90,9 +90,9 @@ describe("RequestPipeline", () => {
9090
expect(result.searchContext.query.bool.must).to.deep.include(
9191
requestBody.query
9292
);
93-
expect(result.searchContext.query.bool.must_not).to.deep.include(
94-
{ term: { visibility: "Private" } },
95-
{ term: { published: false } }
93+
expect(result.searchContext.query.bool.filter).to.deep.include(
94+
{ terms: { visibility: ["Institution", "Public"] } },
95+
{ terms: { published: [true] } }
9696
);
9797
});
9898

@@ -105,7 +105,12 @@ describe("RequestPipeline", () => {
105105
expect(result.searchContext.query.bool.must).to.deep.include(
106106
requestBody.query
107107
);
108-
expect(result.searchContext.query.bool.must_not).to.be.empty;
108+
expect(result.searchContext.query.bool.filter).to.deep.include({
109+
terms: { published: [true, false] },
110+
});
111+
expect(result.searchContext.query.bool.filter).to.deep.include({
112+
terms: { visibility: ["Private", "Institution", "Public"] },
113+
});
109114
});
110115
});
111116

@@ -140,15 +145,25 @@ describe("RequestPipeline", () => {
140145
};
141146
pipeline = new RequestPipeline(requestBody);
142147
const result = pipeline.authFilter(event);
143-
for (const i in requestBody.query.hybrid.queries) {
144-
const originalQuery = requestBody.query.hybrid.queries[i];
145-
const newQuery = result.searchContext.query.hybrid.queries[i];
146-
expect(newQuery.bool.must).to.deep.include(originalQuery);
147-
expect(newQuery.bool.must_not).to.deep.include(
148-
{ term: { visibility: "Private" } },
149-
{ term: { published: false } }
150-
);
151-
}
148+
const [originalNeuralQuery, originalMatchQuery] =
149+
requestBody.query.hybrid.queries;
150+
const [newNeuralQuery, newMatchQuery] =
151+
result.searchContext.query.hybrid.queries;
152+
153+
expect(newNeuralQuery.neural.embedding).to.deep.include(
154+
originalNeuralQuery.neural.embedding
155+
);
156+
expect(newMatchQuery).to.deep.include(originalMatchQuery);
157+
expect(
158+
newNeuralQuery.neural.embedding.filter.bool.filter
159+
).to.deep.include(
160+
{ terms: { visibility: ["Institution", "Public"] } },
161+
{ terms: { published: [true] } }
162+
);
163+
expect(newMatchQuery.bool.filter).to.deep.include(
164+
{ terms: { visibility: ["Institution", "Public"] } },
165+
{ terms: { published: [true] } }
166+
);
152167
});
153168
});
154169

‎docs/docs/spec/openapi.yaml‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ components:
3636
bearerAuth:
3737
type: http
3838
scheme: bearer
39-
bearerFormat: JWT
39+
bearerFormat: JWT
4040
security:
4141
- anonymous: []
4242
- bearerAuth: []
@@ -279,19 +279,25 @@ paths:
279279
- $ref: "./types.yaml#/components/parameters/size"
280280
- $ref: "./types.yaml#/components/parameters/sort"
281281
- $ref: "./types.yaml#/components/parameters/as"
282+
- $ref: "./types.yaml#/components/parameters/visibility"
282283
responses:
283284
200:
284285
$ref: "./types.yaml#/components/responses/SearchResponse"
285286
post:
286287
operationId: postSearch
287288
tags:
288289
- Search
290+
parameters:
291+
- $ref: "./types.yaml#/components/parameters/page"
292+
- $ref: "./types.yaml#/components/parameters/size"
293+
- $ref: "./types.yaml#/components/parameters/sort"
294+
- $ref: "./types.yaml#/components/parameters/as"
295+
- $ref: "./types.yaml#/components/parameters/visibility"
289296
requestBody:
290297
content:
291298
application/json:
292299
schema:
293300
type: object
294-
295301
responses:
296302
200:
297303
$ref: "./types.yaml#/components/responses/SearchResponse"
@@ -308,6 +314,7 @@ paths:
308314
- $ref: "./types.yaml#/components/parameters/size"
309315
- $ref: "./types.yaml#/components/parameters/sort"
310316
- $ref: "./types.yaml#/components/parameters/as"
317+
- $ref: "./types.yaml#/components/parameters/visibility"
311318
responses:
312319
200:
313320
$ref: "./types.yaml#/components/responses/SearchResponse"
@@ -321,6 +328,12 @@ paths:
321328
- $ref: "./types.yaml#/components/parameters/size"
322329
- $ref: "./types.yaml#/components/parameters/sort"
323330
- $ref: "./types.yaml#/components/parameters/as"
331+
- $ref: "./types.yaml#/components/parameters/visibility"
332+
requestBody:
333+
content:
334+
application/json:
335+
schema:
336+
type: object
324337
responses:
325338
200:
326339
$ref: "./types.yaml#/components/responses/SearchResponse"

‎docs/docs/spec/types.yaml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,20 @@ components:
103103
type: integer
104104
minimum: 1
105105
maximum: 300
106+
visibility:
107+
name: visibility
108+
in: query
109+
required: false
110+
description: Filter search results by visibility
111+
explode: false
112+
schema:
113+
type: array
114+
items:
115+
type: string
116+
enum:
117+
- public
118+
- institution
119+
- private
106120
responses:
107121
DocumentResponse:
108122
description: A single document response

0 commit comments

Comments
 (0)