From 6ab060cdfd5e84e2ae9cb398652ff02d29c29550 Mon Sep 17 00:00:00 2001 From: a-alle Date: Wed, 24 Sep 2025 16:18:50 +0100 Subject: [PATCH 1/6] only apply auth update rules if properties values are being updated --- .../translate/translate-top-level-match.ts | 39 +- .../graphql/src/translate/translate-update.ts | 5 + .../tests/integration/issues/6620.int.test.ts | 570 ++++++++++++++++++ .../roles-deprecated.test.ts | 6 - .../arguments/allow/allow.test.ts | 24 +- .../arguments/roles-where.test.ts | 20 - .../arguments/roles/roles.test.ts | 6 - .../implementation-where.test.ts | 10 - .../arguments/where/where.test.ts | 10 - .../graphql/tests/tck/issues/5023.test.ts | 5 - .../graphql/tests/tck/issues/6620.test.ts | 273 +++++++++ 11 files changed, 878 insertions(+), 90 deletions(-) create mode 100644 packages/graphql/tests/integration/issues/6620.int.test.ts create mode 100644 packages/graphql/tests/tck/issues/6620.test.ts diff --git a/packages/graphql/src/translate/translate-top-level-match.ts b/packages/graphql/src/translate/translate-top-level-match.ts index 15253e9939..bc7b8f8112 100644 --- a/packages/graphql/src/translate/translate-top-level-match.ts +++ b/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,6 +34,7 @@ export function translateTopLevelMatch({ context, operation, where, + areNodePropertiesBeingUpdated = true, }: { matchNode: Cypher.Node; matchPattern: Cypher.Pattern; @@ -41,6 +42,7 @@ export function translateTopLevelMatch({ node: Node; operation: AuthorizationOperation; where: GraphQLWhereArg | undefined; + areNodePropertiesBeingUpdated: boolean; }): Cypher.CypherResult { const { matchClause, preComputedWhereFieldSubqueries, whereClause } = createMatchClause({ matchNode, @@ -49,6 +51,7 @@ export function translateTopLevelMatch({ context, operation, where, + areNodePropertiesBeingUpdated, }); return buildClause(Cypher.utils.concat(matchClause, preComputedWhereFieldSubqueries, whereClause), { context }); @@ -66,6 +69,7 @@ function createMatchClause({ node, context, operation, + areNodePropertiesBeingUpdated, where, }: { matchNode: Cypher.Node; @@ -74,26 +78,33 @@ function createMatchClause({ node: Node; operation: AuthorizationOperation; where: GraphQLWhereArg | undefined; + areNodePropertiesBeingUpdated: 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" && !areNodePropertiesBeingUpdated) { 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; diff --git a/packages/graphql/src/translate/translate-update.ts b/packages/graphql/src/translate/translate-update.ts index d8d1e45078..c40c48fed3 100644 --- a/packages/graphql/src/translate/translate-update.ts +++ b/packages/graphql/src/translate/translate-update.ts @@ -67,6 +67,10 @@ 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 areNodePropertiesBeingUpdated = updateInput + ? Object.keys(updateInput).filter((x) => !node.relationFields.some((field) => field.fieldName === x)).length > 0 + : false; const topLevelMatch = translateTopLevelMatch({ matchNode, matchPattern, @@ -74,6 +78,7 @@ export default async function translateUpdate({ context, operation: "UPDATE", where, + areNodePropertiesBeingUpdated, }); matchAndWhereStr = topLevelMatch.cypher; let cypherParams = topLevelMatch.params; diff --git a/packages/graphql/tests/integration/issues/6620.int.test.ts b/packages/graphql/tests/integration/issues/6620.int.test.ts new file mode 100644 index 0000000000..f1a2cc0419 --- /dev/null +++ b/packages/graphql/tests/integration/issues/6620.int.test.ts @@ -0,0 +1,570 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { GraphQLError } from "graphql"; +import type { UniqueType } from "../../utils/graphql-types"; +import { TestHelper } from "../../utils/tests-helper"; + +describe("https://github.com/neo4j/graphql/issues/6620", () => { + let carType: UniqueType; + let carManufacturerType: UniqueType; + + const testHelper = new TestHelper(); + const secret = "secret"; + + beforeAll(async () => { + carType = testHelper.createUniqueType("Car"); + carManufacturerType = testHelper.createUniqueType("CarManufacturer"); + + const typeDefs = ` + type ${carType} + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_CREATE_RELATIONSHIP" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE_RELATIONSHIP" } } } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + producedBy: [${carManufacturerType}!]! + @relationship(type: "CAR_IS_PRODUCED_BY_CARMANUFACTURER", direction: IN, queryDirection: DIRECTED) + } + + type ${carManufacturerType} + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP" + } + } + } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_DELETE_RELATIONSHIP" + } + } + } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (car:${carType}{name:"x1", accessibleBy:"test"}) + CREATE (m:${carManufacturerType}{name:"BMW", accessibleBy:"test"}) + `); + }); + + afterAll(async () => { + await testHelper.close(); + }); + + test("should allow connect without property updates", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors).toBeFalsy(); + expect(updateResult.data).toEqual({ + [carType.operations.update]: { + info: { relationshipsCreated: 1 }, + }, + }); + }); + + test("should not allow connect with property updates without update permission", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { + name:{set:"x2"}, + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors).toBeFalsy(); + expect(updateResult.data).toEqual({ + [carType.operations.update]: { + info: { relationshipsCreated: 0 }, + }, + }); + }); + + test("should allow connect with property updates with update permission", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { + name:{set:"x2"}, + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + permission_Car_node_UPDATE: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors).toBeFalsy(); + expect(updateResult.data).toEqual({ + [carType.operations.update]: { + info: { relationshipsCreated: 1 }, + }, + }); + }); +}); + +describe("https://github.com/neo4j/graphql/issues/6620 validate", () => { + let carType: UniqueType; + let carManufacturerType: UniqueType; + + const testHelper = new TestHelper(); + const secret = "secret"; + + beforeAll(async () => { + carType = testHelper.createUniqueType("Car"); + carManufacturerType = testHelper.createUniqueType("CarManufacturer"); + + const typeDefs = ` + type ${carType} + @authorization( + validate: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_CREATE_RELATIONSHIP" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE_RELATIONSHIP" } } } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + producedBy: [${carManufacturerType}!]! + @relationship(type: "CAR_IS_PRODUCED_BY_CARMANUFACTURER", direction: IN, queryDirection: DIRECTED) + } + + type ${carManufacturerType} + @authorization( + validate: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP" + } + } + } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_DELETE_RELATIONSHIP" + } + } + } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (car:${carType}{name:"x1", accessibleBy:"test"}) + CREATE (m:${carManufacturerType}{name:"BMW", accessibleBy:"test"}) + `); + }); + + afterAll(async () => { + await testHelper.close(); + }); + + test("should allow connect without property updates", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors).toBeFalsy(); + expect(updateResult.data).toEqual({ + [carType.operations.update]: { + info: { relationshipsCreated: 1 }, + }, + }); + }); + + test("should not allow connect with property updates without update permission", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { + name:{set:"x2"}, + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors?.[0]?.message).toBe("Forbidden"); + expect(updateResult.errors?.[0]).toBeInstanceOf(GraphQLError); + }); + + test("should allow connect with property updates with update permission", async () => { + const updateMutation = ` + mutation { + ${carType.operations.update}( + where: { name: { eq: "x1" } } + update: { + name:{set:"x2"}, + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const token = testHelper.createBearerToken(secret, { + permission_Car_node_CREATE_RELATIONSHIP: "test", + permission_CarManufacturer_node_CREATE_RELATIONSHIP: "test", + permission_Car_node_UPDATE: "test", + }); + + const updateResult = await testHelper.executeGraphQLWithToken(updateMutation, token); + expect(updateResult.errors).toBeFalsy(); + expect(updateResult.data).toEqual({ + [carType.operations.update]: { + info: { relationshipsCreated: 1 }, + }, + }); + }); +}); diff --git a/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts b/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts index 8b0490e3c5..b87c691eee 100644 --- a/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts +++ b/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts @@ -447,8 +447,6 @@ describe("Cypher Auth Roles - deprecated", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -483,7 +481,6 @@ describe("Cypher Auth Roles - deprecated", () => { \\"sub\\": \\"super_admin\\" }, \\"update_param2\\": \\"admin\\", - \\"param2\\": \\"admin\\", \\"authorization__before_param2\\": \\"super-admin\\", \\"authorization__before_param3\\": \\"admin\\", \\"authorization__after_param2\\": \\"admin\\", @@ -581,8 +578,6 @@ describe("Cypher Auth Roles - deprecated", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this @@ -614,7 +609,6 @@ describe("Cypher Auth Roles - deprecated", () => { \\"sub\\": \\"super_admin\\" }, \\"update_param2\\": \\"admin\\", - \\"param2\\": \\"admin\\", \\"authorization__before_param2\\": \\"admin\\", \\"authorization__before_param3\\": \\"super-admin\\", \\"authorization__after_param2\\": \\"admin\\", diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/allow/allow.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/allow/allow.test.ts index 3510d8069b..88faa0cce9 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/allow/allow.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/allow/allow.test.ts @@ -452,11 +452,7 @@ describe("Cypher Auth Allow", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:Post) - WITH * - WHERE (this.id = $param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this)<-[:HAS_POST]-(this0:User) - WHERE ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + WHERE this.id = $param0 WITH this CALL(*) { WITH this @@ -511,11 +507,7 @@ describe("Cypher Auth Allow", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:Post) - WITH * - WHERE (this.id = $param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this)<-[:HAS_POST]-(this0:User) - WHERE ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + WHERE this.id = $param0 WITH this CALL(*) { WITH this @@ -664,8 +656,7 @@ describe("Cypher Auth Allow", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE (this.id = $param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + WHERE this.id = $param0 WITH this CALL(*) { WITH this @@ -749,11 +740,7 @@ describe("Cypher Auth Allow", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:Comment) - WITH * - WHERE (this.id = $param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this)<-[:HAS_COMMENT]-(this0:User) - WHERE ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + WHERE this.id = $param0 WITH this CALL(*) { WITH this @@ -859,8 +846,7 @@ describe("Cypher Auth Allow", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE (this.id = $param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + WHERE this.id = $param0 WITH * CALL(*) { WITH this diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts index e32de6c103..ac8d66c892 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts @@ -710,8 +710,6 @@ describe("Cypher Auth Where with Roles", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this @@ -759,8 +757,6 @@ describe("Cypher Auth Where with Roles", () => { \\"update_param3\\": \\"admin\\", \\"update_param4\\": \\"user\\", \\"update_param5\\": \\"admin\\", - \\"param2\\": \\"user\\", - \\"param3\\": \\"admin\\", \\"authorization_updatebefore_param2\\": \\"user\\", \\"authorization_updatebefore_param3\\": \\"admin\\", \\"this_update_posts0_id_SET\\": \\"new-id\\", @@ -1039,8 +1035,6 @@ describe("Cypher Auth Where with Roles", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -1082,8 +1076,6 @@ describe("Cypher Auth Where with Roles", () => { }, \\"update_param2\\": \\"user\\", \\"update_param3\\": \\"admin\\", - \\"param2\\": \\"user\\", - \\"param3\\": \\"admin\\", \\"authorization__before_param2\\": \\"user\\", \\"authorization__before_param3\\": \\"admin\\", \\"authorization__before_param4\\": \\"user\\", @@ -1117,8 +1109,6 @@ describe("Cypher Auth Where with Roles", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -1160,8 +1150,6 @@ describe("Cypher Auth Where with Roles", () => { }, \\"update_param2\\": \\"user\\", \\"update_param3\\": \\"admin\\", - \\"param2\\": \\"user\\", - \\"param3\\": \\"admin\\", \\"this_posts0_connect0_node_param0\\": \\"new-id\\", \\"authorization__before_param2\\": \\"user\\", \\"authorization__before_param3\\": \\"admin\\", @@ -1195,8 +1183,6 @@ describe("Cypher Auth Where with Roles", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this @@ -1235,8 +1221,6 @@ describe("Cypher Auth Where with Roles", () => { }, \\"update_param2\\": \\"user\\", \\"update_param3\\": \\"admin\\", - \\"param2\\": \\"user\\", - \\"param3\\": \\"admin\\", \\"authorization__before_param2\\": \\"user\\", \\"authorization__before_param3\\": \\"admin\\", \\"authorization__before_param4\\": \\"user\\", @@ -1269,8 +1253,6 @@ describe("Cypher Auth Where with Roles", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this @@ -1309,8 +1291,6 @@ describe("Cypher Auth Where with Roles", () => { }, \\"update_param2\\": \\"user\\", \\"update_param3\\": \\"admin\\", - \\"param2\\": \\"user\\", - \\"param3\\": \\"admin\\", \\"updateUsers_args_update_posts0_disconnect0_where_Post_this_posts0_disconnect0param0\\": \\"new-id\\", \\"authorization__before_param2\\": \\"user\\", \\"authorization__before_param3\\": \\"admin\\", diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts index b965d8e1c7..90417e35b2 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts @@ -450,8 +450,6 @@ describe("Cypher Auth Roles", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -486,7 +484,6 @@ describe("Cypher Auth Roles", () => { \\"sub\\": \\"super_admin\\" }, \\"update_param2\\": \\"admin\\", - \\"param2\\": \\"admin\\", \\"authorization__before_param2\\": \\"super-admin\\", \\"authorization__before_param3\\": \\"admin\\", \\"authorization__after_param2\\": \\"admin\\", @@ -586,8 +583,6 @@ describe("Cypher Auth Roles", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this @@ -619,7 +614,6 @@ describe("Cypher Auth Roles", () => { \\"sub\\": \\"super_admin\\" }, \\"update_param2\\": \\"admin\\", - \\"param2\\": \\"admin\\", \\"authorization__before_param2\\": \\"admin\\", \\"authorization__before_param3\\": \\"super-admin\\", \\"authorization__after_param2\\": \\"admin\\", diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/where/interface-relationships/implementation-where.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/where/interface-relationships/implementation-where.test.ts index 5051719d03..d2a8b25535 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/where/interface-relationships/implementation-where.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/where/interface-relationships/implementation-where.test.ts @@ -469,8 +469,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL (this) { WITH this @@ -796,8 +794,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL (this) { WITH * @@ -876,8 +872,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL (this) { WITH * @@ -957,8 +951,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL (this) { WITH this @@ -1029,8 +1021,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL (this) { WITH this diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/where/where.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/where/where.test.ts index ea421d289d..fe9eccf873 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/where/where.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/where/where.test.ts @@ -645,8 +645,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL(*) { WITH this @@ -956,8 +954,6 @@ describe("Cypher Auth Where", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -1015,8 +1011,6 @@ describe("Cypher Auth Where", () => { "CYPHER 5 MATCH (this:User) WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) - WITH * CALL(*) { WITH this OPTIONAL MATCH (this_posts0_connect0_node:Post) @@ -1074,8 +1068,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL(*) { WITH this @@ -1129,8 +1121,6 @@ describe("Cypher Auth Where", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:User) - WITH * - WHERE ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) WITH this CALL(*) { WITH this diff --git a/packages/graphql/tests/tck/issues/5023.test.ts b/packages/graphql/tests/tck/issues/5023.test.ts index 13859bccd7..a4597027f3 100644 --- a/packages/graphql/tests/tck/issues/5023.test.ts +++ b/packages/graphql/tests/tck/issues/5023.test.ts @@ -141,11 +141,6 @@ describe("https://github.com/neo4j/graphql/issues/5023", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "CYPHER 5 MATCH (this:Tenant) - WITH * - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this)<-[:ADMIN_IN]-(this0:User) - WHERE ($jwt.id IS NOT NULL AND this0.userId = $jwt.id) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH this CALL(*) { WITH this diff --git a/packages/graphql/tests/tck/issues/6620.test.ts b/packages/graphql/tests/tck/issues/6620.test.ts new file mode 100644 index 0000000000..421d384aaf --- /dev/null +++ b/packages/graphql/tests/tck/issues/6620.test.ts @@ -0,0 +1,273 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Neo4jGraphQL } from "../../../src"; +import { formatCypher, translateQuery } from "../utils/tck-test-utils"; + +describe("https://github.com/neo4j/graphql/issues/6620", () => { + let typeDefs: string; + let neoSchema: Neo4jGraphQL; + + beforeAll(() => { + typeDefs = /* GraphQL */ ` + type Car + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_CREATE_RELATIONSHIP" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE_RELATIONSHIP" } } } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + producedBy: [CarManufacturer!]! + @relationship(type: "CAR_IS_PRODUCED_BY_CARMANUFACTURER", direction: IN, queryDirection: DIRECTED) + } + + type CarManufacturer + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP" + } + } + } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_DELETE_RELATIONSHIP" + } + } + } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + } + `; + + neoSchema = new Neo4jGraphQL({ + typeDefs, + }); + }); + + test("Should only apply connection auth rule", async () => { + const query = /* GraphQL */ ` + mutation { + updateCars( + where: { name: { eq: "x1" } } + update: { producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CYPHER 5 + MATCH (this:Car) + WHERE this.name = $param0 + WITH * + CALL(*) { + WITH this + OPTIONAL MATCH (this_producedBy0_connect0_node:CarManufacturer) + WHERE this_producedBy0_connect0_node.name = $this_producedBy0_connect0_node_param0 AND ((this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)) AND (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP))) + CALL(*) { + WITH collect(this_producedBy0_connect0_node) as connectedNodes, collect(this) as parentNodes + CALL(connectedNodes, parentNodes) { + UNWIND parentNodes as this + UNWIND connectedNodes as this_producedBy0_connect0_node + CREATE (this)<-[:CAR_IS_PRODUCED_BY_CARMANUFACTURER]-(this_producedBy0_connect0_node) + } + } + WITH this, this_producedBy0_connect0_node + RETURN count(*) AS connect_this_producedBy0_connect_CarManufacturer0 + } + RETURN \\"Query cannot conclude with CALL\\"" + `); + }); + + test("Should apply both update and connection auth rule", async () => { + const query = /* GraphQL */ ` + mutation { + updateCars( + where: { name: { eq: "x1" } } + update: { + name: { set: "x2" } + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] + } + ) { + info { + relationshipsCreated + } + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CYPHER 5 + MATCH (this:Car) + WITH * + WHERE (this.name = $param0 AND (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_UPDATE IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_UPDATE))) + SET this.name = $this_update_name.set + WITH * + CALL(*) { + WITH this + OPTIONAL MATCH (this_producedBy0_connect0_node:CarManufacturer) + WHERE this_producedBy0_connect0_node.name = $this_producedBy0_connect0_node_param0 AND ((this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)) AND (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP))) + CALL(*) { + WITH collect(this_producedBy0_connect0_node) as connectedNodes, collect(this) as parentNodes + CALL(connectedNodes, parentNodes) { + UNWIND parentNodes as this + UNWIND connectedNodes as this_producedBy0_connect0_node + CREATE (this)<-[:CAR_IS_PRODUCED_BY_CARMANUFACTURER]-(this_producedBy0_connect0_node) + } + } + WITH this, this_producedBy0_connect0_node + RETURN count(*) AS connect_this_producedBy0_connect_CarManufacturer0 + } + RETURN \\"Query cannot conclude with CALL\\"" + `); + }); +}); From 1e7f7bd4245d9cdb20c37f9e0d239e66c4585341 Mon Sep 17 00:00:00 2001 From: a-alle Date: Wed, 24 Sep 2025 16:31:36 +0100 Subject: [PATCH 2/6] review changes --- .../src/translate/translate-top-level-match.ts | 12 ++++++------ packages/graphql/src/translate/translate-update.ts | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/graphql/src/translate/translate-top-level-match.ts b/packages/graphql/src/translate/translate-top-level-match.ts index bc7b8f8112..da07993b24 100644 --- a/packages/graphql/src/translate/translate-top-level-match.ts +++ b/packages/graphql/src/translate/translate-top-level-match.ts @@ -34,7 +34,7 @@ export function translateTopLevelMatch({ context, operation, where, - areNodePropertiesBeingUpdated = true, + ignoreOperationAuthorization = true, }: { matchNode: Cypher.Node; matchPattern: Cypher.Pattern; @@ -42,7 +42,7 @@ export function translateTopLevelMatch({ node: Node; operation: AuthorizationOperation; where: GraphQLWhereArg | undefined; - areNodePropertiesBeingUpdated: boolean; + ignoreOperationAuthorization: boolean; }): Cypher.CypherResult { const { matchClause, preComputedWhereFieldSubqueries, whereClause } = createMatchClause({ matchNode, @@ -51,7 +51,7 @@ export function translateTopLevelMatch({ context, operation, where, - areNodePropertiesBeingUpdated, + ignoreOperationAuthorization, }); return buildClause(Cypher.utils.concat(matchClause, preComputedWhereFieldSubqueries, whereClause), { context }); @@ -69,7 +69,7 @@ function createMatchClause({ node, context, operation, - areNodePropertiesBeingUpdated, + ignoreOperationAuthorization, where, }: { matchNode: Cypher.Node; @@ -78,7 +78,7 @@ function createMatchClause({ node: Node; operation: AuthorizationOperation; where: GraphQLWhereArg | undefined; - areNodePropertiesBeingUpdated: boolean; + ignoreOperationAuthorization: boolean; }): CreateMatchClauseReturn { const matchClause: Cypher.Match | Cypher.Yield = new Cypher.Match(matchPattern); const whereOperators: Cypher.Predicate[] = []; @@ -86,7 +86,7 @@ function createMatchClause({ let whereClause: Cypher.Match | Cypher.Yield | Cypher.With | undefined; let authorizationPredicateReturn: PredicateReturn | undefined; - if (operation === "UPDATE" && !areNodePropertiesBeingUpdated) { + if (operation === "UPDATE" && !ignoreOperationAuthorization) { whereClause = matchClause; authorizationPredicateReturn = undefined; } else { diff --git a/packages/graphql/src/translate/translate-update.ts b/packages/graphql/src/translate/translate-update.ts index c40c48fed3..b8006cd9d0 100644 --- a/packages/graphql/src/translate/translate-update.ts +++ b/packages/graphql/src/translate/translate-update.ts @@ -68,8 +68,8 @@ export default async function translateUpdate({ 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 areNodePropertiesBeingUpdated = updateInput - ? Object.keys(updateInput).filter((x) => !node.relationFields.some((field) => field.fieldName === x)).length > 0 + const ignoreOperationAuthorization = updateInput + ? Object.keys(updateInput).some((x) => !node.relationFields.some((field) => field.fieldName === x)) : false; const topLevelMatch = translateTopLevelMatch({ matchNode, @@ -78,7 +78,7 @@ export default async function translateUpdate({ context, operation: "UPDATE", where, - areNodePropertiesBeingUpdated, + ignoreOperationAuthorization, }); matchAndWhereStr = topLevelMatch.cypher; let cypherParams = topLevelMatch.params; From 4ab311bed859ef4b048ac274965d2fcc13217961 Mon Sep 17 00:00:00 2001 From: a-alle Date: Wed, 24 Sep 2025 16:35:58 +0100 Subject: [PATCH 3/6] add changeset --- .changeset/modern-vans-care.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/modern-vans-care.md diff --git a/.changeset/modern-vans-care.md b/.changeset/modern-vans-care.md new file mode 100644 index 0000000000..a848b8d5e9 --- /dev/null +++ b/.changeset/modern-vans-care.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graphql": patch +--- + +Fix update rules incorrectly applied on relationship creation and deletion From ec470098f745114731031dbd9a5d119660764e88 Mon Sep 17 00:00:00 2001 From: a-alle Date: Wed, 24 Sep 2025 17:22:06 +0100 Subject: [PATCH 4/6] fix validate rules as well --- .../create-authorization-after-and-params.ts | 2 +- .../src/translate/create-update-and-params.ts | 35 ++- .../graphql/src/translate/translate-update.ts | 1 + .../directives/authorization/bind.int.test.ts | 1 + .../tests/integration/issues/6620.int.test.ts | 15 + .../roles-deprecated.test.ts | 4 - .../arguments/roles-where.test.ts | 17 -- .../arguments/roles/roles.test.ts | 4 - .../implementation-bind.test.ts | 18 -- .../arguments/validate/validate.test.ts | 11 - .../graphql/tests/tck/issues/5023.test.ts | 13 - .../graphql/tests/tck/issues/6620.test.ts | 258 ++++++++++++++++++ 12 files changed, 298 insertions(+), 81 deletions(-) diff --git a/packages/graphql/src/translate/authorization/compatibility/create-authorization-after-and-params.ts b/packages/graphql/src/translate/authorization/compatibility/create-authorization-after-and-params.ts index 594c1edcaf..b684001eb4 100644 --- a/packages/graphql/src/translate/authorization/compatibility/create-authorization-after-and-params.ts +++ b/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; subqueries?: string; diff --git a/packages/graphql/src/translate/create-update-and-params.ts b/packages/graphql/src/translate/create-update-and-params.ts index ee68ddd4ea..94adb4bbcc 100644 --- a/packages/graphql/src/translate/create-update-and-params.ts +++ b/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 }; + } } } diff --git a/packages/graphql/src/translate/translate-update.ts b/packages/graphql/src/translate/translate-update.ts index b8006cd9d0..8c929bc44a 100644 --- a/packages/graphql/src/translate/translate-update.ts +++ b/packages/graphql/src/translate/translate-update.ts @@ -178,6 +178,7 @@ export default async function translateUpdate({ parentVar: varName, withVars, parameterPrefix: `${resolveTree.name}.args.update`, + ignoreOperationAuthorization, }); [updateStr] = updateAndParams; cypherParams = { diff --git a/packages/graphql/tests/integration/directives/authorization/bind.int.test.ts b/packages/graphql/tests/integration/directives/authorization/bind.int.test.ts index 7b73079dac..01f1494cc9 100644 --- a/packages/graphql/tests/integration/directives/authorization/bind.int.test.ts +++ b/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}" } }, diff --git a/packages/graphql/tests/integration/issues/6620.int.test.ts b/packages/graphql/tests/integration/issues/6620.int.test.ts index f1a2cc0419..0874392a5e 100644 --- a/packages/graphql/tests/integration/issues/6620.int.test.ts +++ b/packages/graphql/tests/integration/issues/6620.int.test.ts @@ -309,6 +309,21 @@ describe("https://github.com/neo4j/graphql/issues/6620 validate", () => { carManufacturerType = testHelper.createUniqueType("CarManufacturer"); const typeDefs = ` + type JWT @jwt { + permission_Car_node_READ: [String!]! + permission_Car_node_AGGREGATE: [String!]! + permission_Car_node_UPDATE: [String!]! + permission_Car_node_DELETE: [String!]! + permission_Car_node_CREATE_RELATIONSHIP: [String!]! + permission_Car_node_DELETE_RELATIONSHIP: [String!]! + permission_CarManufacturer_node_READ: [String!]! + permission_CarManufacturer_node_AGGREGATE: [String!]! + permission_CarManufacturer_node_UPDATE: [String!]! + permission_CarManufacturer_node_DELETE: [String!]! + permission_CarManufacturer_node_CREATE_RELATIONSHIP: [String!]! + permission_CarManufacturer_node_DELETE_RELATIONSHIP: [String!]! + } + type ${carType} @authorization( validate: [ diff --git a/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts b/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts index b87c691eee..ab52599c0d 100644 --- a/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts +++ b/packages/graphql/tests/tck/deprecated/generic-filtering/roles-deprecated.test.ts @@ -464,8 +464,6 @@ describe("Cypher Auth Roles - deprecated", () => { WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS connect_this_posts0_connect_Post0 } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" @@ -592,8 +590,6 @@ describe("Cypher Auth Roles - deprecated", () => { WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS disconnect_this_posts0_disconnect_Post } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts index ac8d66c892..12997355b3 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/roles-where.test.ts @@ -719,15 +719,8 @@ describe("Cypher Auth Where with Roles", () => { WHERE ($jwt.sub IS NOT NULL AND authorization_updatebefore_this0.id = $jwt.sub) } AND ($jwt.roles IS NOT NULL AND $authorization_updatebefore_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization_updatebefore_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) SET this_posts0.id = $this_update_posts0_id_SET - WITH this, this_posts0 - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND EXISTS { - MATCH (this_posts0)<-[:HAS_POST]-(authorization__after_this0:User) - WHERE ($jwt.sub IS NOT NULL AND authorization__after_this0.id = $jwt.sub) - } AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_posts0 } - WITH this - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) CALL (this) { @@ -760,8 +753,6 @@ describe("Cypher Auth Where with Roles", () => { \\"authorization_updatebefore_param2\\": \\"user\\", \\"authorization_updatebefore_param3\\": \\"admin\\", \\"this_update_posts0_id_SET\\": \\"new-id\\", - \\"authorization__after_param2\\": \\"user\\", - \\"authorization__after_param3\\": \\"admin\\", \\"resolvedCallbacks\\": {} }" `); @@ -1058,8 +1049,6 @@ describe("Cypher Auth Where with Roles", () => { } AND ($jwt.roles IS NOT NULL AND $authorization__after_param4 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param5 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS connect_this_posts0_connect_Post0 } - WITH this - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" @@ -1132,8 +1121,6 @@ describe("Cypher Auth Where with Roles", () => { } AND ($jwt.roles IS NOT NULL AND $authorization__after_param4 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param5 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS connect_this_posts0_connect_Post0 } - WITH this - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" @@ -1203,8 +1190,6 @@ describe("Cypher Auth Where with Roles", () => { } AND ($jwt.roles IS NOT NULL AND $authorization__after_param4 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param5 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS disconnect_this_posts0_disconnect_Post } - WITH this - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" @@ -1273,8 +1258,6 @@ describe("Cypher Auth Where with Roles", () => { } AND ($jwt.roles IS NOT NULL AND $authorization__after_param4 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param5 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS disconnect_this_posts0_disconnect_Post } - WITH this - WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts index 90417e35b2..7b4dd804ae 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/roles/roles.test.ts @@ -467,8 +467,6 @@ describe("Cypher Auth Roles", () => { WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS connect_this_posts0_connect_Post0 } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" @@ -597,8 +595,6 @@ describe("Cypher Auth Roles", () => { WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param3 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) RETURN count(*) AS disconnect_this_posts0_disconnect_Post } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization__after_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $update_param2 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/validate/interface-relationships/implementation-bind.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/validate/interface-relationships/implementation-bind.test.ts index 66d37b4a70..4c7e17a289 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/validate/interface-relationships/implementation-bind.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/validate/interface-relationships/implementation-bind.test.ts @@ -292,8 +292,6 @@ describe("Cypher Auth Allow", () => { WITH this, this_content0 MATCH (this_content0)<-[this_content0_has_content0_relationship:HAS_CONTENT]-(this_content0_creator0:User) SET this_content0_creator0.id = $this_update_content0_creator0_id_SET - WITH this, this_content0, this_content0_creator0 - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this_content0_creator0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_content0_creator0 } RETURN count(*) AS update_this_content0 @@ -311,21 +309,12 @@ describe("Cypher Auth Allow", () => { WITH this, this_content0 MATCH (this_content0)<-[this_content0_has_content0_relationship:HAS_CONTENT]-(this_content0_creator0:User) SET this_content0_creator0.id = $this_update_content0_creator0_id_SET - WITH this, this_content0, this_content0_creator0 - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this_content0_creator0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_content0_creator0 } - WITH this, this_content0 - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this_content0)<-[:HAS_CONTENT]-(authorization__after_this0:User) - WHERE ($jwt.sub IS NOT NULL AND authorization__after_this0.id = $jwt.sub) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_content0 } RETURN count(*) AS update_this_Post } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" `); @@ -334,13 +323,6 @@ describe("Cypher Auth Allow", () => { \\"param0\\": \\"id-01\\", \\"updateUsers_args_update_content0_where_this_content0param0\\": \\"post-id\\", \\"this_update_content0_creator0_id_SET\\": \\"not bound\\", - \\"isAuthenticated\\": true, - \\"jwt\\": { - \\"roles\\": [ - \\"admin\\" - ], - \\"sub\\": \\"id-01\\" - }, \\"updateUsers\\": { \\"args\\": { \\"update\\": { diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/validate/validate.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/validate/validate.test.ts index 33ca1f8c9e..828b223146 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/validate/validate.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/validate/validate.test.ts @@ -307,14 +307,10 @@ describe("Cypher Auth Allow", () => { WITH this, this_posts0 MATCH (this_posts0)<-[this_posts0_has_post0_relationship:HAS_POST]-(this_posts0_creator0:User) SET this_posts0_creator0.id = $this_update_posts0_creator0_id_SET - WITH this, this_posts0, this_posts0_creator0 - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this_posts0_creator0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_posts0_creator0 } RETURN count(*) AS update_this_posts0 } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN collect(DISTINCT this { .id }) AS data" `); @@ -323,13 +319,6 @@ describe("Cypher Auth Allow", () => { \\"param0\\": \\"id-01\\", \\"updateUsers_args_update_posts0_where_this_posts0param0\\": \\"post-id\\", \\"this_update_posts0_creator0_id_SET\\": \\"not bound\\", - \\"isAuthenticated\\": true, - \\"jwt\\": { - \\"roles\\": [ - \\"admin\\" - ], - \\"sub\\": \\"id-01\\" - }, \\"updateUsers\\": { \\"args\\": { \\"update\\": { diff --git a/packages/graphql/tests/tck/issues/5023.test.ts b/packages/graphql/tests/tck/issues/5023.test.ts index a4597027f3..48170bb70f 100644 --- a/packages/graphql/tests/tck/issues/5023.test.ts +++ b/packages/graphql/tests/tck/issues/5023.test.ts @@ -171,21 +171,8 @@ describe("https://github.com/neo4j/graphql/issues/5023", () => { DETACH DELETE x } } - WITH this, this_settings0 - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this_settings0)<-[:HAS_SETTINGS]-(authorization__after_this0:Tenant) - WHERE EXISTS { - MATCH (authorization__after_this0)<-[:ADMIN_IN]-(authorization__after_this1:User) - WHERE ($jwt.id IS NOT NULL AND authorization__after_this1.userId = $jwt.id) - } - }), \\"@neo4j/graphql/FORBIDDEN\\", [0]) RETURN count(*) AS update_this_settings0 } - WITH this - WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND EXISTS { - MATCH (this)<-[:ADMIN_IN]-(authorization__after_this0:User) - WHERE ($jwt.id IS NOT NULL AND authorization__after_this0.userId = $jwt.id) - }), \\"@neo4j/graphql/FORBIDDEN\\", [0]) WITH * CALL apoc.util.validate(NOT ($isAuthenticated = true AND EXISTS { MATCH (this)<-[:ADMIN_IN]-(update_this0:User) diff --git a/packages/graphql/tests/tck/issues/6620.test.ts b/packages/graphql/tests/tck/issues/6620.test.ts index 421d384aaf..bec5e23b5f 100644 --- a/packages/graphql/tests/tck/issues/6620.test.ts +++ b/packages/graphql/tests/tck/issues/6620.test.ts @@ -271,3 +271,261 @@ describe("https://github.com/neo4j/graphql/issues/6620", () => { `); }); }); + +describe("https://github.com/neo4j/graphql/issues/6620 validate", () => { + let typeDefs: string; + let neoSchema: Neo4jGraphQL; + + beforeAll(() => { + typeDefs = /* GraphQL */ ` + type Car + @authorization( + validate: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_CREATE_RELATIONSHIP" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_Car_node_DELETE_RELATIONSHIP" } } } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + producedBy: [CarManufacturer!]! + @relationship(type: "CAR_IS_PRODUCED_BY_CARMANUFACTURER", direction: IN, queryDirection: DIRECTED) + } + + type CarManufacturer + @authorization( + validate: [ + { + requireAuthentication: false + operations: [READ] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_READ" } } } + ] + } + } + { + requireAuthentication: false + operations: [AGGREGATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_AGGREGATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [UPDATE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_UPDATE" } } } + ] + } + } + { + requireAuthentication: false + operations: [DELETE] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { node: { accessibleBy: { in: "$jwt.permission_CarManufacturer_node_DELETE" } } } + ] + } + } + { + requireAuthentication: false + operations: [CREATE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP" + } + } + } + ] + } + } + { + requireAuthentication: false + operations: [DELETE_RELATIONSHIP] + where: { + OR: [ + { node: { accessibleBy: { eq: null } } } + { + node: { + accessibleBy: { + in: "$jwt.permission_CarManufacturer_node_DELETE_RELATIONSHIP" + } + } + } + ] + } + } + ] + ) + @node { + name: String + accessibleBy: String + } + `; + + neoSchema = new Neo4jGraphQL({ + typeDefs, + }); + }); + + test("Should only apply connection auth rule", async () => { + const query = /* GraphQL */ ` + mutation { + updateCars( + where: { name: { eq: "x1" } } + update: { producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] } + ) { + info { + relationshipsCreated + } + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CYPHER 5 + MATCH (this:Car) + WHERE this.name = $param0 + WITH * + CALL(*) { + WITH this + OPTIONAL MATCH (this_producedBy0_connect0_node:CarManufacturer) + WHERE this_producedBy0_connect0_node.name = $this_producedBy0_connect0_node_param0 AND (apoc.util.validatePredicate(NOT (this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + CALL(*) { + WITH collect(this_producedBy0_connect0_node) as connectedNodes, collect(this) as parentNodes + CALL(connectedNodes, parentNodes) { + UNWIND parentNodes as this + UNWIND connectedNodes as this_producedBy0_connect0_node + CREATE (this)<-[:CAR_IS_PRODUCED_BY_CARMANUFACTURER]-(this_producedBy0_connect0_node) + } + } + WITH this, this_producedBy0_connect0_node + WITH this, this_producedBy0_connect0_node + WHERE (apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT (this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + RETURN count(*) AS connect_this_producedBy0_connect_CarManufacturer0 + } + RETURN \\"Query cannot conclude with CALL\\"" + `); + }); + + test("Should apply both update and connection auth rule", async () => { + const query = /* GraphQL */ ` + mutation { + updateCars( + where: { name: { eq: "x1" } } + update: { + name: { set: "x2" } + producedBy: [{ connect: { where: { node: { name: { eq: "BMW" } } } } }] + } + ) { + info { + relationshipsCreated + } + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CYPHER 5 + MATCH (this:Car) + WITH * + WHERE (this.name = $param0 AND apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_UPDATE IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_UPDATE)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + SET this.name = $this_update_name.set + WITH * + CALL(*) { + WITH this + OPTIONAL MATCH (this_producedBy0_connect0_node:CarManufacturer) + WHERE this_producedBy0_connect0_node.name = $this_producedBy0_connect0_node_param0 AND (apoc.util.validatePredicate(NOT (this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + CALL(*) { + WITH collect(this_producedBy0_connect0_node) as connectedNodes, collect(this) as parentNodes + CALL(connectedNodes, parentNodes) { + UNWIND parentNodes as this + UNWIND connectedNodes as this_producedBy0_connect0_node + CREATE (this)<-[:CAR_IS_PRODUCED_BY_CARMANUFACTURER]-(this_producedBy0_connect0_node) + } + } + WITH this, this_producedBy0_connect0_node + WITH this, this_producedBy0_connect0_node + WHERE (apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_CREATE_RELATIONSHIP IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT (this_producedBy0_connect0_node.accessibleBy IS NULL OR ($jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP IS NOT NULL AND this_producedBy0_connect0_node.accessibleBy IN $jwt.permission_CarManufacturer_node_CREATE_RELATIONSHIP)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) + RETURN count(*) AS connect_this_producedBy0_connect_CarManufacturer0 + } + WITH this + WHERE apoc.util.validatePredicate(NOT (this.accessibleBy IS NULL OR ($jwt.permission_Car_node_UPDATE IS NOT NULL AND this.accessibleBy IN $jwt.permission_Car_node_UPDATE)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) + RETURN \\"Query cannot conclude with CALL\\"" + `); + }); +}); From 6057b2eb84cbe60eeeb7df4c1ea5fb28b46d3b99 Mon Sep 17 00:00:00 2001 From: a-alle Date: Wed, 24 Sep 2025 17:25:45 +0100 Subject: [PATCH 5/6] change default for ignoreOperationAuthorization flag --- packages/graphql/src/translate/create-update-and-params.ts | 2 +- packages/graphql/src/translate/translate-top-level-match.ts | 4 ++-- packages/graphql/src/translate/translate-update.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/graphql/src/translate/create-update-and-params.ts b/packages/graphql/src/translate/create-update-and-params.ts index 94adb4bbcc..c0ed5e91c0 100644 --- a/packages/graphql/src/translate/create-update-and-params.ts +++ b/packages/graphql/src/translate/create-update-and-params.ts @@ -605,7 +605,7 @@ export default function createUpdateAndParams({ const withStr = `WITH ${withVars.join(", ")}`; let authorizationAfterAndParams: AuthorizationAfterAndParams | undefined; - if (!ignoreOperationAuthorization) { + if (ignoreOperationAuthorization) { authorizationAfterAndParams = undefined; } else { authorizationAfterAndParams = createAuthorizationAfterAndParams({ diff --git a/packages/graphql/src/translate/translate-top-level-match.ts b/packages/graphql/src/translate/translate-top-level-match.ts index da07993b24..dda78c710f 100644 --- a/packages/graphql/src/translate/translate-top-level-match.ts +++ b/packages/graphql/src/translate/translate-top-level-match.ts @@ -34,7 +34,7 @@ export function translateTopLevelMatch({ context, operation, where, - ignoreOperationAuthorization = true, + ignoreOperationAuthorization = false, }: { matchNode: Cypher.Node; matchPattern: Cypher.Pattern; @@ -86,7 +86,7 @@ function createMatchClause({ let whereClause: Cypher.Match | Cypher.Yield | Cypher.With | undefined; let authorizationPredicateReturn: PredicateReturn | undefined; - if (operation === "UPDATE" && !ignoreOperationAuthorization) { + if (operation === "UPDATE" && ignoreOperationAuthorization) { whereClause = matchClause; authorizationPredicateReturn = undefined; } else { diff --git a/packages/graphql/src/translate/translate-update.ts b/packages/graphql/src/translate/translate-update.ts index 8c929bc44a..59bf1936ea 100644 --- a/packages/graphql/src/translate/translate-update.ts +++ b/packages/graphql/src/translate/translate-update.ts @@ -69,7 +69,7 @@ export default async function translateUpdate({ 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).some((x) => !node.relationFields.some((field) => field.fieldName === x)) + ? !Object.keys(updateInput).some((x) => !node.relationFields.some((field) => field.fieldName === x)) : false; const topLevelMatch = translateTopLevelMatch({ matchNode, From 1a9d48c163c4165bfb8350f90ba836ac7de08428 Mon Sep 17 00:00:00 2001 From: a-alle Date: Thu, 25 Sep 2025 10:23:54 +0100 Subject: [PATCH 6/6] remove double negation --- packages/graphql/src/translate/create-update-and-params.test.ts | 1 + packages/graphql/src/translate/translate-update.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/graphql/src/translate/create-update-and-params.test.ts b/packages/graphql/src/translate/create-update-and-params.test.ts index aaa2ee079b..9f56f9b71c 100644 --- a/packages/graphql/src/translate/create-update-and-params.test.ts +++ b/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( diff --git a/packages/graphql/src/translate/translate-update.ts b/packages/graphql/src/translate/translate-update.ts index 59bf1936ea..1dec53b0a9 100644 --- a/packages/graphql/src/translate/translate-update.ts +++ b/packages/graphql/src/translate/translate-update.ts @@ -69,7 +69,7 @@ export default async function translateUpdate({ 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).some((x) => !node.relationFields.some((field) => field.fieldName === x)) + ? Object.keys(updateInput).every((x) => node.relationFields.some((field) => field.fieldName === x)) : false; const topLevelMatch = translateTopLevelMatch({ matchNode,