Skip to content

Commit 3644409

Browse files
darrellwardedarrellwarde
committed
Merge remote-tracking branch 'upstream/dev' into HEAD
2 parents 9851a46 + 469c04e commit 3644409

36 files changed

+538
-649
lines changed

‎.changeset/young-ligers-poke.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@neo4j/graphql": patch
3+
---
4+
5+
Improve the checking for non-existent parameters when evaluating authorization rules

‎packages/graphql/src/schema/resolvers/wrapper.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,13 @@ export const wrapResolver =
104104
jwtParam: new Cypher.NamedParam("jwt", jwt),
105105
isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
106106
claims: jwtPayloadFieldsMap,
107-
jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
108107
};
109108
} catch (e) {
110109
const isAuthenticated = false;
111110
context.authorization = {
112111
isAuthenticated,
113112
jwtParam: new Cypher.NamedParam("jwt", {}),
114113
isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
115-
jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
116114
};
117115
}
118116
}
@@ -125,7 +123,6 @@ export const wrapResolver =
125123
jwt,
126124
jwtParam: new Cypher.NamedParam("jwt", jwt),
127125
isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
128-
jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
129126
};
130127
}
131128

‎packages/graphql/src/translate/authorization/utils/populate-where-params.test.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { populateWhereParams } from "./populate-where-params";
2525
describe("populateWhereParams", () => {
2626
let context: Context;
2727
let jwtParam: Cypher.Param;
28-
let jwtDefault: Cypher.Param;
2928

3029
beforeAll(() => {
3130
const jwt = {
@@ -39,14 +38,12 @@ describe("populateWhereParams", () => {
3938
};
4039

4140
jwtParam = new Cypher.Param(jwt);
42-
jwtDefault = new Cypher.Param({});
4341

4442
context = new ContextBuilder({
4543
authorization: {
4644
jwtParam,
4745
isAuthenticated: true,
4846
isAuthenticatedParam: new Cypher.Param(true),
49-
jwtDefault,
5047
},
5148
}).instance();
5249
});
@@ -57,7 +54,7 @@ describe("populateWhereParams", () => {
5754
};
5855

5956
expect(populateWhereParams({ where, context })).toEqual({
60-
id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
57+
id: jwtParam.property("sub"),
6158
});
6259
});
6360

@@ -67,7 +64,7 @@ describe("populateWhereParams", () => {
6764
};
6865

6966
expect(populateWhereParams({ where, context })).toEqual({
70-
id: Cypher.coalesce(jwtParam.property("some", "other", "claim"), jwtDefault),
67+
id: jwtParam.property("some", "other", "claim"),
7168
});
7269
});
7370

@@ -80,7 +77,7 @@ describe("populateWhereParams", () => {
8077

8178
expect(populateWhereParams({ where, context })).toEqual({
8279
user: {
83-
id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
80+
id: jwtParam.property("sub"),
8481
},
8582
});
8683
});
@@ -105,12 +102,12 @@ describe("populateWhereParams", () => {
105102
AND: [
106103
{
107104
user: {
108-
id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
105+
id: jwtParam.property("sub"),
109106
},
110107
},
111108
{
112109
user: {
113-
role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
110+
role_IN: jwtParam.property("roles"),
114111
},
115112
},
116113
],
@@ -159,26 +156,26 @@ describe("populateWhereParams", () => {
159156
AND: [
160157
{
161158
user: {
162-
id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
159+
id: jwtParam.property("sub"),
163160
},
164161
},
165162
{
166163
user: {
167-
role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
164+
role_IN: jwtParam.property("roles"),
168165
},
169166
},
170167
],
171168
},
172169
{
173170
user: {
174-
role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
171+
role_IN: jwtParam.property("roles"),
175172
},
176173
},
177174
],
178175
},
179176
{
180177
user: {
181-
role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
178+
role_IN: jwtParam.property("roles"),
182179
},
183180
},
184181
],

‎packages/graphql/src/translate/authorization/utils/populate-where-params.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,7 @@ export function populateWhereParams({ where, context }: { where: GraphQLWhereArg
3939

4040
const jwtProperty = context.authorization.jwtParam.property(...(mappedPath || path).split("."));
4141

42-
// coalesce jwt parameter values to be an empty map which can be evaluated in a boolean expression
43-
// this is because comparing against null will always produce null, which results in errors in apoc.util.validatePredicate
44-
// comparing a node property against a map will always results in false, because maps cannot be used as properties
45-
const coalesce = Cypher.coalesce(jwtProperty, context.authorization.jwtDefault);
46-
47-
parsed[k] = coalesce;
42+
parsed[k] = jwtProperty;
4843
} else if (v.startsWith("$context")) {
4944
const path = v.substring(9);
5045
const contextValueParameter = new Cypher.Param(dotProp.get(context, path));

‎packages/graphql/src/translate/authorization/where/create-authorization-where-predicate.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export function createAuthorizationWherePredicate({
7474
whereInput: populateWhereParams({ where: value, context }),
7575
targetElement: target,
7676
useExistExpr,
77+
checkParameterExistence: true,
7778
});
7879

7980
if (predicate) {

‎packages/graphql/src/translate/where/create-where-predicate.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ export function createWherePredicate({
3333
context,
3434
element,
3535
useExistExpr = true,
36+
checkParameterExistence,
3637
}: {
3738
targetElement: Cypher.Variable;
3839
whereInput: GraphQLWhereArg;
3940
context: Context;
4041
element: GraphElement;
4142
useExistExpr?: boolean;
43+
checkParameterExistence?: boolean;
4244
}): PredicateReturn {
4345
const whereFields = Object.entries(whereInput);
4446
const predicates: Cypher.Predicate[] = [];
@@ -51,6 +53,8 @@ export function createWherePredicate({
5153
targetElement,
5254
context,
5355
value: asArray(value),
56+
useExistExpr,
57+
checkParameterExistence,
5458
});
5559
if (predicate) {
5660
predicates.push(predicate);
@@ -66,6 +70,7 @@ export function createWherePredicate({
6670
targetElement,
6771
context,
6872
useExistExpr,
73+
checkParameterExistence,
6974
});
7075
if (predicate) {
7176
predicates.push(predicate);
@@ -87,12 +92,16 @@ function createNestedPredicate({
8792
targetElement,
8893
context,
8994
value,
95+
useExistExpr,
96+
checkParameterExistence,
9097
}: {
9198
key: LogicalOperator;
9299
element: GraphElement;
93100
targetElement: Cypher.Variable;
94101
context: Context;
95102
value: Array<GraphQLWhereArg>;
103+
useExistExpr?: boolean;
104+
checkParameterExistence?: boolean;
96105
}): PredicateReturn {
97106
const nested: Cypher.Predicate[] = [];
98107
let subqueries: Cypher.CompositeClause | undefined;
@@ -103,6 +112,8 @@ function createNestedPredicate({
103112
element,
104113
targetElement,
105114
context,
115+
useExistExpr,
116+
checkParameterExistence,
106117
});
107118
if (predicate) {
108119
nested.push(predicate);

‎packages/graphql/src/translate/where/property-operations/create-connection-operation.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,15 @@ export function createConnectionOperation({
3636
parentNode,
3737
operator,
3838
useExistExpr = true,
39+
checkParameterExistence,
3940
}: {
4041
connectionField: ConnectionField;
4142
value: any;
4243
context: Context;
4344
parentNode: Cypher.Node;
4445
operator: string | undefined;
4546
useExistExpr?: boolean;
47+
checkParameterExistence?: boolean;
4648
}): PredicateReturn {
4749
let nodeEntries: Record<string, any>;
4850

@@ -99,6 +101,7 @@ export function createConnectionOperation({
99101
whereOperator: operator as WhereOperator,
100102
refEdge: contextRelationship,
101103
useExistExpr,
104+
checkParameterExistence,
102105
});
103106

104107
operations.push(predicate);
@@ -119,6 +122,7 @@ export function createConnectionWherePropertyOperation({
119122
node,
120123
edge,
121124
useExistExpr = true,
125+
checkParameterExistence,
122126
}: {
123127
whereInput: ConnectionWhereArg;
124128
context: Context;
@@ -127,6 +131,7 @@ export function createConnectionWherePropertyOperation({
127131
edgeRef: Cypher.Variable;
128132
targetNode: Cypher.Node;
129133
useExistExpr?: boolean;
134+
checkParameterExistence?: boolean;
130135
}): PredicateReturn {
131136
const preComputedSubqueriesResult: (Cypher.CompositeClause | undefined)[] = [];
132137
const params: (Cypher.Predicate | undefined)[] = [];
@@ -142,6 +147,7 @@ export function createConnectionWherePropertyOperation({
142147
node,
143148
edge,
144149
useExistExpr,
150+
checkParameterExistence,
145151
});
146152
subOperations.push(predicate);
147153
if (preComputedSubqueries && !preComputedSubqueries.empty)
@@ -161,6 +167,7 @@ export function createConnectionWherePropertyOperation({
161167
context,
162168
element: edge,
163169
useExistExpr,
170+
checkParameterExistence,
164171
});
165172

166173
params.push(result);
@@ -189,6 +196,7 @@ export function createConnectionWherePropertyOperation({
189196
context,
190197
element: node,
191198
useExistExpr,
199+
checkParameterExistence,
192200
});
193201

194202
// NOTE: _NOT is handled by the size()=0

‎packages/graphql/src/translate/where/property-operations/create-property-where.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,15 @@ export function createPropertyWhere({
4242
targetElement,
4343
context,
4444
useExistExpr = true,
45+
checkParameterExistence,
4546
}: {
4647
key: string;
4748
value: any;
4849
element: GraphElement;
4950
targetElement: Cypher.Variable;
5051
context: Context;
5152
useExistExpr?: boolean;
53+
checkParameterExistence?: boolean;
5254
}): PredicateReturn {
5355
const match = whereRegEx.exec(key);
5456
if (!match) {
@@ -126,6 +128,7 @@ export function createPropertyWhere({
126128
value,
127129
isNot,
128130
useExistExpr,
131+
checkParameterExistence,
129132
});
130133
}
131134

@@ -138,6 +141,7 @@ export function createPropertyWhere({
138141
parentNode: targetElement as Cypher.Node,
139142
operator,
140143
useExistExpr,
144+
checkParameterExistence,
141145
});
142146
}
143147

@@ -157,22 +161,24 @@ export function createPropertyWhere({
157161
(x) => x.fieldName === fieldName && x.typeMeta.name === "Duration"
158162
);
159163

164+
const param =
165+
value instanceof Cypher.Param || value instanceof Cypher.Property || value instanceof Cypher.Function
166+
? value
167+
: new Cypher.Param(value);
168+
160169
const comparisonOp = createComparisonOperation({
161170
propertyRefOrCoalesce: propertyRef,
162171
// When dealing with authorization input, references to JWT will already be a param
163172
// TODO: Pre-parse all where input in a manner similar to populateWhereParams, which substitutes all values for params
164-
param:
165-
value instanceof Cypher.Param || value instanceof Cypher.Property || value instanceof Cypher.Function
166-
? value
167-
: new Cypher.Param(value),
173+
param,
168174
operator,
169175
durationField,
170176
pointField,
171177
});
172-
if (isNot) {
173-
return {
174-
predicate: Cypher.not(comparisonOp),
175-
};
176-
}
177-
return { predicate: comparisonOp };
178+
179+
const comparison = isNot ? Cypher.not(comparisonOp) : comparisonOp;
180+
181+
const predicate = checkParameterExistence ? Cypher.and(Cypher.isNotNull(param), comparison) : comparison;
182+
183+
return { predicate };
178184
}

0 commit comments

Comments
 (0)