Fix connect disconnect

packages/graphql/src/translate/queryAST/ast/operations/ConnectOperation.ts

@@ -168,7 +168,7 @@ export class ConnectOperation extends MutationOperation {
 
         const relVar = new Cypher.Relationship();
 
-        const relDirection = this.relationship.getCypherDirection();
+        const relDirection = this.relationship.cypherDirectionFromRelDirection();
 
         const connectPattern = new Cypher.Pattern(context.target)
             .related(relVar, { direction: relDirection, type: this.relationship.type })
@@ -205,6 +205,9 @@ export class ConnectOperation extends MutationOperation {
         }
 
         const clauses = Cypher.utils.concat(
+            // required in: packages/graphql/tests/integration/directives/authorization/roles.int.test.ts
+            // without when: AFTER adjustment failing in: packages/graphql/tests/integration/issues/3929.int.test.ts
+
             matchClause,
             ...bothAuthClausesBefore, // THESE ARE "BEFORE" AUTH
             ...mutationSubqueries,

packages/graphql/src/translate/queryAST/ast/operations/UpdateOperation.ts

@@ -33,7 +33,6 @@ import type { RelationshipAdapter } from "../../../../schema-model/relationship/
 import { wrapSubqueriesInCypherCalls } from "../../utils/wrap-subquery-in-calls";
 import type { Filter } from "../filters/Filter";
 import { ParamInputField } from "../input-fields/ParamInputField";
-
 /**
  * This is currently just a dummy tree node,
  * The whole mutation part is still implemented in the old way, the current scope of this node is just to contains the nested fields.
@@ -126,10 +125,13 @@ export class UpdateOperation extends Operation {
             .flatMap((input) => {
                 const subqueries = input.getSubqueries(nestedContext);
                 const authSubqueries = input.getAuthorizationSubqueries(nestedContext);
-                if (authSubqueries.length > 0 || subqueries.length > 0) {
-                    return Cypher.utils.concat(...subqueries, ...authSubqueries);
+                if (!authSubqueries.length && !subqueries.length) {
+                    return undefined;
+                }
+                if (authSubqueries.length > 0) {
+                    return Cypher.utils.concat(...subqueries, new Cypher.With("*"), ...authSubqueries);
                 }
-                return undefined;
+                return Cypher.utils.concat(...subqueries);
             })
             .filter((s) => s !== undefined);
 

packages/graphql/src/translate/queryAST/factory/Operations/ConnectFactory.ts

@@ -141,13 +141,14 @@ export class ConnectFactory {
             context,
             operation: connect,
         });
-        if (isConcreteEntity(relationship.source)) {
-            this.addSourceEntityAuthorization({
-                entity: relationship.source,
-                context,
-                operation: connect,
-            });
-        }
+        // this wasn't in the original code - but should it be?
+        // if (isConcreteEntity(relationship.source)) {
+        //     this.addSourceEntityAuthorization({
+        //         entity: relationship.source,
+        //         context,
+        //         operation: connect,
+        //     });
+        // }
 
         asArray(input).forEach((inputItem) => {
             const { whereArg, connectArg } = this.parseConnectArgs(inputItem);

packages/graphql/src/translate/queryAST/factory/Operations/DisconnectFactory.ts

@@ -25,6 +25,7 @@ import type { Neo4jGraphQLTranslationContext } from "../../../../types/neo4j-gra
 import { asArray } from "../../../../utils/utils";
 import type { Filter } from "../../ast/filters/Filter";
 import { MutationOperationField } from "../../ast/input-fields/MutationOperationField";
+import { ParamInputField } from "../../ast/input-fields/ParamInputField";
 import { DisconnectOperation } from "../../ast/operations/DisconnectOperation";
 import { CompositeDisconnectOperation } from "../../ast/operations/composite/CompositeDisconnectOperation";
 import { CompositeDisconnectPartial } from "../../ast/operations/composite/CompositeDisconnectPartial";
@@ -33,6 +34,7 @@ import type { CallbackBucket } from "../../utils/callback-bucket";
 import { isConcreteEntity } from "../../utils/is-concrete-entity";
 import { isInterfaceEntity } from "../../utils/is-interface-entity";
 import { isUnionEntity } from "../../utils/is-union-entity";
+import { raiseAttributeAmbiguity } from "../../utils/raise-attribute-ambiguity";
 import type { QueryASTFactory } from "../QueryASTFactory";
 
 export class DisconnectFactory {
@@ -153,6 +155,7 @@ export class DisconnectFactory {
         asArray(input).forEach((inputItem) => {
             const { whereArg, disconnectArg } = this.parseDisconnectArgs(inputItem);
             const nodeFilters: Filter[] = [];
+            const edgeFilters: Filter[] = [];
             if (whereArg.node) {
                 if (isConcreteEntity(relationship.target) || isUnionEntity(relationship.target)) {
                     nodeFilters.push(...this.queryASTFactory.filterFactory.createNodeFilters(target, whereArg.node));
@@ -167,8 +170,11 @@ export class DisconnectFactory {
                     );
                 }
             }
+            if (whereArg.edge) {
+                edgeFilters.push(...this.queryASTFactory.filterFactory.createEdgeFilters(relationship, whereArg.edge));
+            }
 
-            disconnect.addFilters(...nodeFilters);
+            disconnect.addFilters(...nodeFilters, ...edgeFilters);
 
             asArray(disconnectArg).forEach((nestedDisconnectInputFields) => {
                 Object.entries(nestedDisconnectInputFields).forEach(([key, value]) => {
@@ -195,23 +201,23 @@ export class DisconnectFactory {
                 });
             });
 
-            // const targetInputEdge = this.getInputEdge(inputItem, relationship);
+            const targetInputEdge = this.getInputEdge(inputItem, relationship);
 
             /* Create the attributes for the edge */
-            // raiseAttributeAmbiguity(Object.keys(targetInputEdge), relationship);
-            // for (const key of Object.keys(targetInputEdge)) {
-            //     const attribute = relationship.attributes.get(key);
-            //     if (attribute) {
-            //         const attachedTo = "relationship";
+            raiseAttributeAmbiguity(Object.keys(targetInputEdge), relationship);
+            for (const key of Object.keys(targetInputEdge)) {
+                const attribute = relationship.attributes.get(key);
+                if (attribute) {
+                    const attachedTo = "relationship";
 
-            //         const paramInputField = new ParamInputField({
-            //             attachedTo,
-            //             attribute,
-            //             inputValue: targetInputEdge[key],
-            //         });
-            //         disconnect.addField(paramInputField, attachedTo);
-            //     }
-            // }
+                    const paramInputField = new ParamInputField({
+                        attachedTo,
+                        attribute,
+                        inputValue: targetInputEdge[key],
+                    });
+                    disconnect.addField(paramInputField, attachedTo);
+                }
+            }
         });
     }
 
@@ -269,7 +275,7 @@ export class DisconnectFactory {
     } {
         const rawWhere = args.where ?? {};
 
-        const whereArg = { node: rawWhere.node, edge: {} };
+        const whereArg = { node: rawWhere.node, edge: rawWhere.edge };
         const disconnectArg = args.disconnect ?? {};
         return { whereArg, disconnectArg };
     }

packages/graphql/src/translate/queryAST/factory/Operations/UpdateFactory.ts

@@ -154,7 +154,9 @@ export class UpdateFactory {
             });
         }
 
-        this.addEntityAuthorization({ entity: target, context, operation: update });
+        if (this.shouldApplyUpdateAuthorization(input, target, isNested)) {
+            this.addEntityAuthorization({ entity: target, context, operation: update });
+        }
         asArray(input).forEach((inputItem) => {
             const targetInput = this.getInputNode(inputItem, isNested);
             raiseAttributeAmbiguityForUpdate(Object.keys(targetInput), target);
@@ -166,13 +168,15 @@ export class UpdateFactory {
             //     operation: update,
             // });
 
-            const filters = this.queryASTFactory.filterFactory.createConnectionPredicates({
-                rel: relationship,
-                entity: target,
-                where: whereArgs,
-            });
-            // const filters = this.queryASTFactory.filterFactory.createNodeFilters(target, whereArgs.node);
-            update.addFilters(...filters);
+            if (whereArgs) {
+                const filters = this.queryASTFactory.filterFactory.createConnectionPredicates({
+                    rel: relationship,
+                    entity: target,
+                    where: whereArgs,
+                });
+                // const filters = this.queryASTFactory.filterFactory.createNodeFilters(target, whereArgs.node);
+                update.addFilters(...filters);
+            }
             for (const key of Object.keys(targetInput)) {
                 const { fieldName, operator } = parseMutationField(key);
                 const nestedRelationship = target.relationships.get(fieldName);
@@ -582,6 +586,35 @@ export class UpdateFactory {
         }
     }
 
+    // returns true only if actual attributes are modified
+    // UPDATE rules should not be applied for (dis)connections
+    private shouldApplyUpdateAuthorization(
+        input: Record<string, any>,
+        entity: ConcreteEntityAdapter,
+        isNested: boolean
+    ): boolean {
+        const actualInput = this.getInputNode(input, isNested);
+        const affectedKeys = Object.keys(actualInput).map((key) => {
+            // old version compatibility (eg id_SET)
+            const { fieldName } = parseMutationField(key);
+            return fieldName;
+        });
+        const areAttributesAffected = affectedKeys.filter((k) => entity.attributes.has(k)).length > 0;
+        // TODO: not sure I agree with this but 'tis the case nonetheless
+        // In the example below Post authorization UPDATE rules should not be applied (in my opinion)
+        // because only the relationship field is in the input (creator)
+        // User(update: { posts: { update: { node: { creator: { update: { node: { id_SET: "" } } }}} }}
+        const isRelationshipUpdated =
+            isNested &&
+            affectedKeys
+                .filter((k) => entity.relationships.has(k))
+                .some((k) => {
+                    return asArray(actualInput[k]).filter((inputItem) => inputItem.update);
+                });
+
+        return areAttributesAffected || isRelationshipUpdated;
+    }
+
     private getInputNode(inputItem: Record<string, any>, isNested: boolean): Record<string, any> {
         if (isNested) {
             return inputItem.node ?? {};

packages/graphql/tests/integration/issues/3929.int.test.ts

@@ -53,7 +53,7 @@ describe("https://github.com/neo4j/graphql/issues/3929", () => {
                 creator: [${User}!]! @relationship(type: "CREATOR_OF", direction: IN, nestedOperations: [CONNECT])
             }
 
-            type ${Person} @authorization(validate: [{ where: { node: { creator_SINGLE: { id_EQ: "$jwt.uid" }}}}]) @node {
+            type ${Person} @authorization(validate: [{  where: { node: { creator_SINGLE: { id_EQ: "$jwt.uid" }}}}]) @node {
                 id: ID! @id
                 name: String!
                 creator: [${User}!]! @relationship(type: "CREATOR_OF", direction: IN)