Only apply auth update rules if properties values are being updated (#6696)

* only apply auth update rules if properties values are being updated

* review changes

* add changeset

* fix validate rules as well

* change default for ignoreOperationAuthorization flag

* remove double negation

Author: a-alle

.changeset/modern-vans-care.md

@@ -0,0 +1,5 @@
+---
+"@neo4j/graphql": patch
+---
+
+Fix update rules incorrectly applied on relationship creation and deletion

packages/graphql/src/translate/authorization/compatibility/create-authorization-after-and-params.ts

@@ -28,7 +28,7 @@ import {
 import type { NodeMap } from "../types/node-map";
 import { compilePredicateReturn } from "./compile-predicate-return";
 
-type AuthorizationAfterAndParams = {
+export type AuthorizationAfterAndParams = {
     cypher: string;
     params: Record<string, any>;
     subqueries?: string;

packages/graphql/src/translate/create-update-and-params.test.ts

@@ -104,6 +104,7 @@ describe("createUpdateAndParams", () => {
             withVars: ["this"],
             parameterPrefix: "this",
             callbackBucket: new CallbackBucketDeprecated(context),
+            ignoreOperationAuthorization: false,
         });
 
         expect(trimmer(result[0])).toEqual(

packages/graphql/src/translate/create-update-and-params.ts

@@ -27,6 +27,7 @@ import { caseWhere } from "../utils/case-where";
 import { getRelationshipType } from "../utils/get-relationship-type";
 import { wrapStringInApostrophes } from "../utils/wrap-string-in-apostrophes";
 import { checkAuthentication } from "./authorization/check-authentication";
+import type { AuthorizationAfterAndParams } from "./authorization/compatibility/create-authorization-after-and-params";
 import {
     createAuthorizationAfterAndParams,
     createAuthorizationAfterAndParamsField,
@@ -74,6 +75,7 @@ export default function createUpdateAndParams({
     context,
     callbackBucket,
     parameterPrefix,
+    ignoreOperationAuthorization,
 }: {
     parentVar: string;
     updateInput: any;
@@ -84,6 +86,7 @@ export default function createUpdateAndParams({
     context: Neo4jGraphQLTranslationContext;
     callbackBucket: CallbackBucketDeprecated;
     parameterPrefix: string;
+    ignoreOperationAuthorization: boolean;
 }): [string, any] {
     let hasAppliedTimeStamps = false;
 
@@ -312,6 +315,7 @@ export default function createUpdateAndParams({
                                 parameterPrefix: `${parameterPrefix}.${key}${
                                     relationField.union ? `.${refNode.name}` : ""
                                 }${relationField.typeMeta.array ? `[${index}]` : ``}.update.node`,
+                                ignoreOperationAuthorization,
                             });
                             res.params = { ...res.params, ...updateAndParams[1] };
                             innerUpdate.push(updateAndParams[0]);
@@ -599,23 +603,28 @@ export default function createUpdateAndParams({
     const authorizationAfterSubqueries = meta.authorizationAfterSubqueries;
 
     const withStr = `WITH ${withVars.join(", ")}`;
+    let authorizationAfterAndParams: AuthorizationAfterAndParams | undefined;
 
-    const authorizationAfterAndParams = createAuthorizationAfterAndParams({
-        context,
-        nodes: [{ node, variable: varName }],
-        operations: ["UPDATE"],
-    });
+    if (ignoreOperationAuthorization) {
+        authorizationAfterAndParams = undefined;
+    } else {
+        authorizationAfterAndParams = createAuthorizationAfterAndParams({
+            context,
+            nodes: [{ node, variable: varName }],
+            operations: ["UPDATE"],
+        });
 
-    if (authorizationAfterAndParams) {
-        const { cypher, params: authWhereParams, subqueries } = authorizationAfterAndParams;
+        if (authorizationAfterAndParams) {
+            const { cypher, params: authWhereParams, subqueries } = authorizationAfterAndParams;
 
-        if (cypher) {
-            if (subqueries) {
-                authorizationAfterSubqueries.push(subqueries);
-            }
+            if (cypher) {
+                if (subqueries) {
+                    authorizationAfterSubqueries.push(subqueries);
+                }
 
-            authorizationAfterStrs.push(cypher);
-            params = { ...params, ...authWhereParams };
+                authorizationAfterStrs.push(cypher);
+                params = { ...params, ...authWhereParams };
+            }
         }
     }
 

packages/graphql/src/translate/translate-top-level-match.ts

@@ -20,7 +20,7 @@
 import Cypher from "@neo4j/cypher-builder";
 import type { Node } from "../classes";
 import type { AuthorizationOperation } from "../schema-model/annotation/AuthorizationAnnotation";
-import type { GraphQLWhereArg } from "../types";
+import type { GraphQLWhereArg, PredicateReturn } from "../types";
 import type { Neo4jGraphQLTranslationContext } from "../types/neo4j-graphql-translation-context";
 import { getEntityAdapterFromNode } from "../utils/get-entity-adapter-from-node";
 import { createAuthorizationBeforePredicate } from "./authorization/create-authorization-before-predicate";
@@ -34,13 +34,15 @@ export function translateTopLevelMatch({
     context,
     operation,
     where,
+    ignoreOperationAuthorization = false,
 }: {
     matchNode: Cypher.Node;
     matchPattern: Cypher.Pattern;
     context: Neo4jGraphQLTranslationContext;
     node: Node;
     operation: AuthorizationOperation;
     where: GraphQLWhereArg | undefined;
+    ignoreOperationAuthorization: boolean;
 }): Cypher.CypherResult {
     const { matchClause, preComputedWhereFieldSubqueries, whereClause } = createMatchClause({
         matchNode,
@@ -49,6 +51,7 @@ export function translateTopLevelMatch({
         context,
         operation,
         where,
+        ignoreOperationAuthorization,
     });
 
     return buildClause(Cypher.utils.concat(matchClause, preComputedWhereFieldSubqueries, whereClause), { context });
@@ -66,6 +69,7 @@ function createMatchClause({
     node,
     context,
     operation,
+    ignoreOperationAuthorization,
     where,
 }: {
     matchNode: Cypher.Node;
@@ -74,26 +78,33 @@ function createMatchClause({
     node: Node;
     operation: AuthorizationOperation;
     where: GraphQLWhereArg | undefined;
+    ignoreOperationAuthorization: boolean;
 }): CreateMatchClauseReturn {
     const matchClause: Cypher.Match | Cypher.Yield = new Cypher.Match(matchPattern);
     const whereOperators: Cypher.Predicate[] = [];
 
     let whereClause: Cypher.Match | Cypher.Yield | Cypher.With | undefined;
 
-    const authorizationPredicateReturn = createAuthorizationBeforePredicate({
-        context,
-        nodes: [
-            {
-                variable: matchNode,
-                node,
-            },
-        ],
-        operations: [operation],
-    });
-    if (authorizationPredicateReturn?.predicate) {
-        whereClause = new Cypher.With("*");
-    } else {
+    let authorizationPredicateReturn: PredicateReturn | undefined;
+    if (operation === "UPDATE" && ignoreOperationAuthorization) {
         whereClause = matchClause;
+        authorizationPredicateReturn = undefined;
+    } else {
+        authorizationPredicateReturn = createAuthorizationBeforePredicate({
+            context,
+            nodes: [
+                {
+                    variable: matchNode,
+                    node,
+                },
+            ],
+            operations: [operation],
+        });
+        if (authorizationPredicateReturn?.predicate) {
+            whereClause = new Cypher.With("*");
+        } else {
+            whereClause = matchClause;
+        }
     }
 
     let preComputedWhereFieldSubqueries: Cypher.CompositeClause | undefined;

packages/graphql/src/translate/translate-update.ts

@@ -67,13 +67,18 @@ export default async function translateUpdate({
     const matchNode = new Cypher.NamedNode(varName);
     const where = resolveTree.args.where as GraphQLWhereArg | undefined;
     const matchPattern = new Cypher.Pattern(matchNode, { labels: node.getLabels(context) });
+    // bypass auth rules if there's no actual update of properties (connect and disconnect have their own auth rules)
+    const ignoreOperationAuthorization = updateInput
+        ? Object.keys(updateInput).every((x) => node.relationFields.some((field) => field.fieldName === x))
+        : false;
     const topLevelMatch = translateTopLevelMatch({
         matchNode,
         matchPattern,
         node,
         context,
         operation: "UPDATE",
         where,
+        ignoreOperationAuthorization,
     });
     matchAndWhereStr = topLevelMatch.cypher;
     let cypherParams = topLevelMatch.params;
@@ -173,6 +178,7 @@ export default async function translateUpdate({
             parentVar: varName,
             withVars,
             parameterPrefix: `${resolveTree.name}.args.update`,
+            ignoreOperationAuthorization,
         });
         [updateStr] = updateAndParams;
         cypherParams = {

packages/graphql/tests/integration/directives/authorization/bind.int.test.ts

@@ -497,6 +497,7 @@ describe("auth/bind", () => {
                     ${User.operations.update}(
                         where: { id_EQ: "${userId}" },
                         update: {
+                            id_SET: "${userId}",
                             posts: {
                                 update: {
                                     where: { node: { id_EQ: "${postId}" } },