Skip to content

Commit

Permalink
Fix 5497
Browse files Browse the repository at this point in the history
  • Loading branch information
angrykoala committed Aug 29, 2024
1 parent b4daa27 commit 8377972
Show file tree
Hide file tree
Showing 8 changed files with 313 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/stupid-geese-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@neo4j/graphql": patch
---

Fix generated cypher for disconnect operations with filters #5497
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ function createDisconnectAndParams({

if (subqueries) {
subquery.push(subqueries);
if (whereStrs.length) {
subquery.push("WITH *");
}
}
}

Expand Down
113 changes: 113 additions & 0 deletions packages/graphql/tests/integration/issues/5497.int.test.ts
Original file line number Diff line number Diff line change
@@ -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,
},
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,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
Expand Down
183 changes: 183 additions & 0 deletions packages/graphql/tests/tck/issues/5497.test.ts
Original file line number Diff line number Diff line change
@@ -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\\": {}
}"
`);
});
});

0 comments on commit 8377972

Please sign in to comment.