diff --git a/.changeset/stupid-geese-marry.md b/.changeset/stupid-geese-marry.md new file mode 100644 index 0000000000..251d7175eb --- /dev/null +++ b/.changeset/stupid-geese-marry.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graphql": patch +--- + +Fix generated cypher for disconnect operations with filters #5497 diff --git a/packages/graphql/src/translate/create-disconnect-and-params.ts b/packages/graphql/src/translate/create-disconnect-and-params.ts index 3334239d8b..5f19675185 100644 --- a/packages/graphql/src/translate/create-disconnect-and-params.ts +++ b/packages/graphql/src/translate/create-disconnect-and-params.ts @@ -133,6 +133,9 @@ function createDisconnectAndParams({ if (subqueries) { subquery.push(subqueries); + if (whereStrs.length) { + subquery.push("WITH *"); + } } } diff --git a/packages/graphql/tests/integration/issues/5497.int.test.ts b/packages/graphql/tests/integration/issues/5497.int.test.ts new file mode 100644 index 0000000000..3e8bb58ff8 --- /dev/null +++ b/packages/graphql/tests/integration/issues/5497.int.test.ts @@ -0,0 +1,113 @@ +/* + * 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 type { UniqueType } from "../../utils/graphql-types"; +import { TestHelper } from "../../utils/tests-helper"; + +describe("https://github.com/neo4j/graphql/issues/5467", () => { + const testHelper = new TestHelper(); + + let User: UniqueType; + let Cabinet: UniqueType; + let Category: UniqueType; + let File: UniqueType; + + beforeAll(async () => { + User = testHelper.createUniqueType("User"); + Cabinet = testHelper.createUniqueType("Cabinet"); + Category = testHelper.createUniqueType("Category"); + File = testHelper.createUniqueType("File"); + + const typeDefs = /* GraphQL */ ` + type JWT @jwt { + roles: [String!]! + } + + type ${User} + @authorization( + validate: [ + { operations: [CREATE, DELETE], where: { jwt: { roles_INCLUDES: "admin" } } } + { operations: [READ, UPDATE], where: { node: { id: "$jwt.sub" } } } + ] + filter: [{ where: { node: { id: "$jwt.sub" } } }] + ) { + id: ID! + cabinets: [${Cabinet}!]! @relationship(type: "HAS_CABINET", direction: OUT) + } + + type ${Cabinet} @authorization(filter: [{ where: { node: { user: { id: "$jwt.sub" } } } }]) { + id: ID! @id + categories: [${Category}!]! @relationship(type: "HAS_CATEGORY", direction: OUT) + user: ${User}! @relationship(type: "HAS_CABINET", direction: IN) + } + + type ${Category} @authorization(filter: [{ where: { node: { cabinet: { user: { id: "$jwt.sub" } } } } }]) { + id: ID! @id + files: [${File}!]! @relationship(type: "HAS_FILE", direction: OUT) + cabinet: ${Cabinet}! @relationship(type: "HAS_CATEGORY", direction: IN) + } + + type ${File} { + id: ID! @unique + category: ${Category} @relationship(type: "HAS_FILE", direction: IN) + } + `; + await testHelper.initNeo4jGraphQL({ + typeDefs, + }); + }); + + afterAll(async () => { + await testHelper.close(); + }); + + test("should properly add where filters for auth", async () => { + const query = /* GraphQL */ ` + mutation ($fileId: ID!, $newCategoryId: ID) { + ${File.operations.update}( + where: { id: $fileId } + disconnect: { category: { where: { node: { NOT: { id: $newCategoryId } } } } } + connect: { category: { where: { node: { id: $newCategoryId } } } } + ) { + info { + relationshipsDeleted + relationshipsCreated + } + } + } + `; + + const response = await testHelper.executeGraphQL(query, { + variableValues: { + fileId: "old-id", + newCategoryId: "new-id", + }, + }); + + expect(response.errors).toBeFalsy(); + expect(response.data).toEqual({ + [File.operations.update]: { + info: { + relationshipsDeleted: 0, + relationshipsCreated: 0, + }, + }, + }); + }); +}); 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 24d081010b..cce9af0dc3 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 @@ -647,6 +647,7 @@ describe("Cypher Auth Allow", () => { OPTIONAL MATCH (this)-[this_disconnect_posts0_rel:HAS_POST]->(this_disconnect_posts0:Post) OPTIONAL MATCH (this_disconnect_posts0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_disconnect_posts0.id = $updateUsers_args_disconnect_posts0_where_Post_this_disconnect_posts0param0 AND (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) CALL { WITH this_disconnect_posts0, this_disconnect_posts0_rel, this @@ -728,6 +729,7 @@ describe("Cypher Auth Allow", () => { WITH *, count(authorization__before_this0) AS creatorCount OPTIONAL MATCH (this_post0_disconnect0)<-[:HAS_POST]-(authorization__before_this1:User) WITH *, count(authorization__before_this1) AS creatorCount + WITH * WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this1.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0])) CALL { WITH this_post0_disconnect0, this_post0_disconnect0_rel, this @@ -740,6 +742,7 @@ describe("Cypher Auth Allow", () => { OPTIONAL MATCH (this_post0_disconnect0)<-[this_post0_disconnect0_creator0_rel:HAS_POST]-(this_post0_disconnect0_creator0:User) OPTIONAL MATCH (this_post0_disconnect0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_post0_disconnect0_creator0.id = $updateComments_args_update_post_disconnect_disconnect_creator_where_User_this_post0_disconnect0_creator0param0 AND (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this_post0_disconnect0_creator0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) CALL { WITH this_post0_disconnect0_creator0, this_post0_disconnect0_creator0_rel, this_post0_disconnect0 diff --git a/packages/graphql/tests/tck/directives/authorization/arguments/allow/interface-relationships/implementation-allow.test.ts b/packages/graphql/tests/tck/directives/authorization/arguments/allow/interface-relationships/implementation-allow.test.ts index de5b4053a3..8d4bca2639 100644 --- a/packages/graphql/tests/tck/directives/authorization/arguments/allow/interface-relationships/implementation-allow.test.ts +++ b/packages/graphql/tests/tck/directives/authorization/arguments/allow/interface-relationships/implementation-allow.test.ts @@ -412,6 +412,7 @@ describe("@auth allow on specific interface implementation", () => { OPTIONAL MATCH (this)-[this_disconnect_content0_rel:HAS_CONTENT]->(this_disconnect_content0:Post) OPTIONAL MATCH (this_disconnect_content0)<-[:HAS_CONTENT]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_disconnect_content0.id = $updateUsers_args_disconnect_content0_where_Post_this_disconnect_content0param0 AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) CALL { WITH this_disconnect_content0, this_disconnect_content0_rel, this 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 95752823f4..371ad6b3f2 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 @@ -1200,6 +1200,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_content0_disconnect0_rel:HAS_CONTENT]->(this_content0_disconnect0:Post) OPTIONAL MATCH (this_content0_disconnect0)<-[:HAS_CONTENT]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_content0_disconnect0, this_content0_disconnect0_rel, this @@ -1274,6 +1275,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_content0_disconnect0_rel:HAS_CONTENT]->(this_content0_disconnect0:Post) OPTIONAL MATCH (this_content0_disconnect0)<-[:HAS_CONTENT]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_content0_disconnect0.id = $updateUsers_args_update_content0_disconnect0_where_Post_this_content0_disconnect0param0 AND (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_content0_disconnect0, this_content0_disconnect0_rel, this @@ -1361,6 +1363,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_disconnect_content0_rel:HAS_CONTENT]->(this_disconnect_content0:Post) OPTIONAL MATCH (this_disconnect_content0)<-[:HAS_CONTENT]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_disconnect_content0, this_disconnect_content0_rel, this @@ -1437,6 +1440,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_disconnect_content0_rel:HAS_CONTENT]->(this_disconnect_content0:Post) OPTIONAL MATCH (this_disconnect_content0)<-[:HAS_CONTENT]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_disconnect_content0.id = $updateUsers_args_disconnect_content0_where_Post_this_disconnect_content0param0 AND (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_disconnect_content0, this_disconnect_content0_rel, 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 4a08813b3e..5a3403ed96 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 @@ -1215,6 +1215,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_posts0_disconnect0_rel:HAS_POST]->(this_posts0_disconnect0:Post) OPTIONAL MATCH (this_posts0_disconnect0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_posts0_disconnect0, this_posts0_disconnect0_rel, this @@ -1269,6 +1270,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_posts0_disconnect0_rel:HAS_POST]->(this_posts0_disconnect0:Post) OPTIONAL MATCH (this_posts0_disconnect0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_posts0_disconnect0.id = $updateUsers_args_update_posts0_disconnect0_where_Post_this_posts0_disconnect0param0 AND (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_posts0_disconnect0, this_posts0_disconnect0_rel, this @@ -1343,6 +1345,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_disconnect_posts0_rel:HAS_POST]->(this_disconnect_posts0:Post) OPTIONAL MATCH (this_disconnect_posts0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_disconnect_posts0, this_disconnect_posts0_rel, this @@ -1409,6 +1412,7 @@ describe("Cypher Auth Where", () => { OPTIONAL MATCH (this)-[this_disconnect_posts0_rel:HAS_POST]->(this_disconnect_posts0:Post) OPTIONAL MATCH (this_disconnect_posts0)<-[:HAS_POST]-(authorization__before_this0:User) WITH *, count(authorization__before_this0) AS creatorCount + WITH * WHERE this_disconnect_posts0.id = $updateUsers_args_disconnect_posts0_where_Post_this_disconnect_posts0param0 AND (($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)) AND ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this0.id = $jwt.sub)))) CALL { WITH this_disconnect_posts0, this_disconnect_posts0_rel, this diff --git a/packages/graphql/tests/tck/issues/5497.test.ts b/packages/graphql/tests/tck/issues/5497.test.ts new file mode 100644 index 0000000000..8817d20b97 --- /dev/null +++ b/packages/graphql/tests/tck/issues/5497.test.ts @@ -0,0 +1,183 @@ +/* + * 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, formatParams, translateQuery } from "../utils/tck-test-utils"; + +describe("https://github.com/neo4j/graphql/issues/5497", () => { + let typeDefs: string; + let neoSchema: Neo4jGraphQL; + + beforeAll(() => { + typeDefs = /* GraphQL */ ` + type JWT @jwt { + roles: [String!]! + } + + type User + @authorization( + validate: [ + { operations: [CREATE, DELETE], where: { jwt: { roles_INCLUDES: "admin" } } } + { operations: [READ, UPDATE], where: { node: { id: "$jwt.sub" } } } + ] + filter: [{ where: { node: { id: "$jwt.sub" } } }] + ) { + id: ID! + cabinets: [Cabinet!]! @relationship(type: "HAS_CABINET", direction: OUT) + } + + type Cabinet @authorization(filter: [{ where: { node: { user: { id: "$jwt.sub" } } } }]) { + id: ID! @id + categories: [Category!]! @relationship(type: "HAS_CATEGORY", direction: OUT) + user: User! @relationship(type: "HAS_CABINET", direction: IN) + } + + type Category @authorization(filter: [{ where: { node: { cabinet: { user: { id: "$jwt.sub" } } } } }]) { + id: ID! @id + files: [File!]! @relationship(type: "HAS_FILE", direction: OUT) + cabinet: Cabinet! @relationship(type: "HAS_CATEGORY", direction: IN) + } + + type File { + id: ID! @unique + category: Category @relationship(type: "HAS_FILE", direction: IN) + } + `; + + neoSchema = new Neo4jGraphQL({ + typeDefs, + }); + }); + + test("should properly add where filters for auth", async () => { + const query = /* GraphQL */ ` + mutation ($fileId: ID!, $newCategoryId: ID) { + updateFiles( + where: { id: $fileId } + disconnect: { category: { where: { node: { NOT: { id: $newCategoryId } } } } } + connect: { category: { where: { node: { id: $newCategoryId } } } } + ) { + info { + relationshipsDeleted + relationshipsCreated + } + } + } + `; + + const result = await translateQuery(neoSchema, query, { + variableValues: { + fileId: "old-id", + newCategoryId: "new-id", + }, + }); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "MATCH (this:File) + WHERE this.id = $param0 + WITH this + CALL { + WITH this + OPTIONAL MATCH (this)<-[this_disconnect_category0_rel:HAS_FILE]-(this_disconnect_category0:Category) + CALL { + WITH this_disconnect_category0 + MATCH (this_disconnect_category0)<-[:HAS_CATEGORY]-(authorization__before_this1:Cabinet) + OPTIONAL MATCH (authorization__before_this1)<-[:HAS_CABINET]-(authorization__before_this2:User) + WITH *, count(authorization__before_this2) AS userCount + WITH * + WHERE (userCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this2.id = $jwt.sub)) + RETURN count(authorization__before_this1) = 1 AS authorization__before_var0 + } + WITH * + WHERE NOT (this_disconnect_category0.id = $updateFiles_args_disconnect_category_where_Category_this_disconnect_category0param0) AND ($isAuthenticated = true AND authorization__before_var0 = true) + CALL { + WITH this_disconnect_category0, this_disconnect_category0_rel, this + WITH collect(this_disconnect_category0) as this_disconnect_category0, this_disconnect_category0_rel, this + UNWIND this_disconnect_category0 as x + DELETE this_disconnect_category0_rel + } + RETURN count(*) AS disconnect_this_disconnect_category_Category + } + WITH * + CALL { + WITH this + OPTIONAL MATCH (this_connect_category0_node:Category) + CALL { + WITH this_connect_category0_node + MATCH (this_connect_category0_node)<-[:HAS_CATEGORY]-(authorization__before_this1:Cabinet) + OPTIONAL MATCH (authorization__before_this1)<-[:HAS_CABINET]-(authorization__before_this2:User) + WITH *, count(authorization__before_this2) AS userCount + WITH * + WHERE (userCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization__before_this2.id = $jwt.sub)) + RETURN count(authorization__before_this1) = 1 AS authorization__before_var0 + } + WITH * + WHERE this_connect_category0_node.id = $this_connect_category0_node_param0 AND ($isAuthenticated = true AND authorization__before_var0 = true) + CALL { + WITH * + WITH collect(this_connect_category0_node) as connectedNodes, collect(this) as parentNodes + CALL { + WITH connectedNodes, parentNodes + UNWIND parentNodes as this + UNWIND connectedNodes as this_connect_category0_node + MERGE (this)<-[:HAS_FILE]-(this_connect_category0_node) + } + } + WITH this, this_connect_category0_node + RETURN count(*) AS connect_this_connect_category_Category0 + } + WITH * + WITH * + CALL { + WITH this + MATCH (this)<-[this_category_Category_unique:HAS_FILE]-(:Category) + WITH count(this_category_Category_unique) as c + WHERE apoc.util.validatePredicate(NOT (c <= 1), '@neo4j/graphql/RELATIONSHIP-REQUIREDFile.category must be less than or equal to one', [0]) + RETURN c AS this_category_Category_unique_ignored + } + RETURN \\"Query cannot conclude with CALL\\"" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"param0\\": \\"old-id\\", + \\"updateFiles_args_disconnect_category_where_Category_this_disconnect_category0param0\\": \\"new-id\\", + \\"isAuthenticated\\": false, + \\"jwt\\": {}, + \\"this_connect_category0_node_param0\\": \\"new-id\\", + \\"updateFiles\\": { + \\"args\\": { + \\"disconnect\\": { + \\"category\\": { + \\"where\\": { + \\"node\\": { + \\"NOT\\": { + \\"id\\": \\"new-id\\" + } + } + } + } + } + } + }, + \\"resolvedCallbacks\\": {} + }" + `); + }); +});