Improve the checking for non-existent parameters when evaluating authorization rules

.changeset/young-ligers-poke.md

@@ -0,0 +1,5 @@
+---
+"@neo4j/graphql": patch
+---
+
+Improve the checking for non-existent parameters when evaluating authorization rules

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

@@ -118,15 +118,13 @@ export const wrapResolver =
                         jwtParam: new Cypher.NamedParam("jwt", jwt),
                         isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
                         claims: jwtPayloadFieldsMap,
-                        jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
                     };
                 } catch (e) {
                     const isAuthenticated = false;
                     context.authorization = {
                         isAuthenticated,
                         jwtParam: new Cypher.NamedParam("jwt", {}),
                         isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
-                        jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
                     };
                 }
             }
@@ -152,7 +150,6 @@ export const wrapResolver =
                 jwt,
                 jwtParam: new Cypher.NamedParam("jwt", jwt),
                 isAuthenticatedParam: new Cypher.NamedParam("isAuthenticated", isAuthenticated),
-                jwtDefault: new Cypher.NamedParam("jwtDefault", {}),
             };
         }
 

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

@@ -25,7 +25,6 @@ import { populateWhereParams } from "./populate-where-params";
 describe("populateWhereParams", () => {
     let context: Context;
     let jwtParam: Cypher.Param;
-    let jwtDefault: Cypher.Param;
 
     beforeAll(() => {
         const jwt = {
@@ -39,14 +38,12 @@ describe("populateWhereParams", () => {
         };
 
         jwtParam = new Cypher.Param(jwt);
-        jwtDefault = new Cypher.Param({});
 
         context = new ContextBuilder({
             authorization: {
                 jwtParam,
                 isAuthenticated: true,
                 isAuthenticatedParam: new Cypher.Param(true),
-                jwtDefault,
             },
         }).instance();
     });
@@ -57,7 +54,7 @@ describe("populateWhereParams", () => {
         };
 
         expect(populateWhereParams({ where, context })).toEqual({
-            id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
+            id: jwtParam.property("sub"),
         });
     });
 
@@ -67,7 +64,7 @@ describe("populateWhereParams", () => {
         };
 
         expect(populateWhereParams({ where, context })).toEqual({
-            id: Cypher.coalesce(jwtParam.property("some", "other", "claim"), jwtDefault),
+            id: jwtParam.property("some", "other", "claim"),
         });
     });
 
@@ -80,7 +77,7 @@ describe("populateWhereParams", () => {
 
         expect(populateWhereParams({ where, context })).toEqual({
             user: {
-                id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
+                id: jwtParam.property("sub"),
             },
         });
     });
@@ -105,12 +102,12 @@ describe("populateWhereParams", () => {
             AND: [
                 {
                     user: {
-                        id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
+                        id: jwtParam.property("sub"),
                     },
                 },
                 {
                     user: {
-                        role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
+                        role_IN: jwtParam.property("roles"),
                     },
                 },
             ],
@@ -159,26 +156,26 @@ describe("populateWhereParams", () => {
                             AND: [
                                 {
                                     user: {
-                                        id: Cypher.coalesce(jwtParam.property("sub"), jwtDefault),
+                                        id: jwtParam.property("sub"),
                                     },
                                 },
                                 {
                                     user: {
-                                        role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
+                                        role_IN: jwtParam.property("roles"),
                                     },
                                 },
                             ],
                         },
                         {
                             user: {
-                                role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
+                                role_IN: jwtParam.property("roles"),
                             },
                         },
                     ],
                 },
                 {
                     user: {
-                        role_IN: Cypher.coalesce(jwtParam.property("roles"), jwtDefault),
+                        role_IN: jwtParam.property("roles"),
                     },
                 },
             ],

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

@@ -39,12 +39,7 @@ export function populateWhereParams({ where, context }: { where: GraphQLWhereArg
 
                 const jwtProperty = context.authorization.jwtParam.property(...(mappedPath || path).split("."));
 
-                // coalesce jwt parameter values to be an empty map which can be evaluated in a boolean expression
-                // this is because comparing against null will always produce null, which results in errors in apoc.util.validatePredicate
-                // comparing a node property against a map will always results in false, because maps cannot be used as properties
-                const coalesce = Cypher.coalesce(jwtProperty, context.authorization.jwtDefault);
-
-                parsed[k] = coalesce;
+                parsed[k] = jwtProperty;
             } else if (v.startsWith("$context")) {
                 const path = v.substring(9);
                 const contextValueParameter = new Cypher.Param(dotProp.get(context, path));

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

@@ -74,6 +74,7 @@ export function createAuthorizationWherePredicate({
                 whereInput: populateWhereParams({ where: value, context }),
                 targetElement: target,
                 useExistExpr,
+                checkParameterExistence: true,
             });
 
             if (predicate) {

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

@@ -33,12 +33,14 @@ export function createWherePredicate({
     context,
     element,
     useExistExpr = true,
+    checkParameterExistence,
 }: {
     targetElement: Cypher.Variable;
     whereInput: GraphQLWhereArg;
     context: Context;
     element: GraphElement;
     useExistExpr?: boolean;
+    checkParameterExistence?: boolean;
 }): PredicateReturn {
     const whereFields = Object.entries(whereInput);
     const predicates: Cypher.Predicate[] = [];
@@ -51,6 +53,8 @@ export function createWherePredicate({
                 targetElement,
                 context,
                 value: asArray(value),
+                useExistExpr,
+                checkParameterExistence,
             });
             if (predicate) {
                 predicates.push(predicate);
@@ -66,6 +70,7 @@ export function createWherePredicate({
             targetElement,
             context,
             useExistExpr,
+            checkParameterExistence,
         });
         if (predicate) {
             predicates.push(predicate);
@@ -87,12 +92,16 @@ function createNestedPredicate({
     targetElement,
     context,
     value,
+    useExistExpr,
+    checkParameterExistence,
 }: {
     key: LogicalOperator;
     element: GraphElement;
     targetElement: Cypher.Variable;
     context: Context;
     value: Array<GraphQLWhereArg>;
+    useExistExpr?: boolean;
+    checkParameterExistence?: boolean;
 }): PredicateReturn {
     const nested: Cypher.Predicate[] = [];
     let subqueries: Cypher.CompositeClause | undefined;
@@ -103,6 +112,8 @@ function createNestedPredicate({
             element,
             targetElement,
             context,
+            useExistExpr,
+            checkParameterExistence,
         });
         if (predicate) {
             nested.push(predicate);

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

@@ -36,13 +36,15 @@ export function createConnectionOperation({
     parentNode,
     operator,
     useExistExpr = true,
+    checkParameterExistence,
 }: {
     connectionField: ConnectionField;
     value: any;
     context: Context;
     parentNode: Cypher.Node;
     operator: string | undefined;
     useExistExpr?: boolean;
+    checkParameterExistence?: boolean;
 }): PredicateReturn {
     let nodeEntries: Record<string, any>;
 
@@ -99,6 +101,7 @@ export function createConnectionOperation({
             whereOperator: operator as WhereOperator,
             refEdge: contextRelationship,
             useExistExpr,
+            checkParameterExistence,
         });
 
         operations.push(predicate);
@@ -119,6 +122,7 @@ export function createConnectionWherePropertyOperation({
     node,
     edge,
     useExistExpr = true,
+    checkParameterExistence,
 }: {
     whereInput: ConnectionWhereArg;
     context: Context;
@@ -127,6 +131,7 @@ export function createConnectionWherePropertyOperation({
     edgeRef: Cypher.Variable;
     targetNode: Cypher.Node;
     useExistExpr?: boolean;
+    checkParameterExistence?: boolean;
 }): PredicateReturn {
     const preComputedSubqueriesResult: (Cypher.CompositeClause | undefined)[] = [];
     const params: (Cypher.Predicate | undefined)[] = [];
@@ -142,6 +147,7 @@ export function createConnectionWherePropertyOperation({
                     node,
                     edge,
                     useExistExpr,
+                    checkParameterExistence,
                 });
                 subOperations.push(predicate);
                 if (preComputedSubqueries && !preComputedSubqueries.empty)
@@ -161,6 +167,7 @@ export function createConnectionWherePropertyOperation({
                 context,
                 element: edge,
                 useExistExpr,
+                checkParameterExistence,
             });
 
             params.push(result);
@@ -189,6 +196,7 @@ export function createConnectionWherePropertyOperation({
                 context,
                 element: node,
                 useExistExpr,
+                checkParameterExistence,
             });
 
             // NOTE: _NOT is handled by the size()=0

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

@@ -42,13 +42,15 @@ export function createPropertyWhere({
     targetElement,
     context,
     useExistExpr = true,
+    checkParameterExistence,
 }: {
     key: string;
     value: any;
     element: GraphElement;
     targetElement: Cypher.Variable;
     context: Context;
     useExistExpr?: boolean;
+    checkParameterExistence?: boolean;
 }): PredicateReturn {
     const match = whereRegEx.exec(key);
     if (!match) {
@@ -126,6 +128,7 @@ export function createPropertyWhere({
                 value,
                 isNot,
                 useExistExpr,
+                checkParameterExistence,
             });
         }
 
@@ -138,6 +141,7 @@ export function createPropertyWhere({
                 parentNode: targetElement as Cypher.Node,
                 operator,
                 useExistExpr,
+                checkParameterExistence,
             });
         }
 
@@ -157,24 +161,26 @@ export function createPropertyWhere({
         (x) => x.fieldName === fieldName && x.typeMeta.name === "Duration"
     );
 
+    const param =
+        value instanceof Cypher.Param || value instanceof Cypher.Property || value instanceof Cypher.Function
+            ? value
+            : new Cypher.Param(value);
+
     const comparisonOp = createComparisonOperation({
         propertyRefOrCoalesce: propertyRef,
         // When dealing with authorization input, references to JWT will already be a param
         // TODO: Pre-parse all where input in a manner similar to populateWhereParams, which substitutes all values for params
-        param:
-            value instanceof Cypher.Param || value instanceof Cypher.Property || value instanceof Cypher.Function
-                ? value
-                : new Cypher.Param(value),
+        param,
         operator,
         durationField,
         pointField,
         // Casting because this is definitely assigned in the wrapper
         neo4jDatabaseInfo: context.neo4jDatabaseInfo as Neo4jDatabaseInfo,
     });
-    if (isNot) {
-        return {
-            predicate: Cypher.not(comparisonOp),
-        };
-    }
-    return { predicate: comparisonOp };
+
+    const comparison = isNot ? Cypher.not(comparisonOp) : comparisonOp;
+
+    const predicate = checkParameterExistence ? Cypher.and(Cypher.isNotNull(param), comparison) : comparison;
+
+    return { predicate };
 }