From 5d0db540344c48b85156dda9ba7a4c769079ae04 Mon Sep 17 00:00:00 2001 From: angrykoala Date: Mon, 2 Sep 2024 17:23:33 +0100 Subject: [PATCH] Fix #5515 --- .changeset/lazy-brooms-explain.md | 5 + .../ast/operations/DeleteOperation.ts | 9 +- .../tests/integration/issues/5515.int.test.ts | 117 ++++++++++++++++++ .../graphql/tests/tck/issues/5515.test.ts | 105 ++++++++++++++++ 4 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 .changeset/lazy-brooms-explain.md create mode 100644 packages/graphql/tests/integration/issues/5515.int.test.ts create mode 100644 packages/graphql/tests/tck/issues/5515.test.ts diff --git a/.changeset/lazy-brooms-explain.md b/.changeset/lazy-brooms-explain.md new file mode 100644 index 0000000000..892e672436 --- /dev/null +++ b/.changeset/lazy-brooms-explain.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graphql": patch +--- + +Fix authorization filtering in delete operations diff --git a/packages/graphql/src/translate/queryAST/ast/operations/DeleteOperation.ts b/packages/graphql/src/translate/queryAST/ast/operations/DeleteOperation.ts index 42771629fa..66a14aed88 100644 --- a/packages/graphql/src/translate/queryAST/ast/operations/DeleteOperation.ts +++ b/packages/graphql/src/translate/queryAST/ast/operations/DeleteOperation.ts @@ -78,7 +78,9 @@ export class DeleteOperation extends MutationOperation { ): OperationTranspileResult { this.validateSelection(selection); const filterSubqueries = wrapSubqueriesInCypherCalls(context, this.filters, [context.target]); - const authBeforeSubqueries = this.getAuthFilterSubqueries(context); + const authBeforeSubqueries = this.getAuthFilterSubqueries(context).map((c) => { + return new Cypher.Call(c).importWith(context.target); + }); const predicate = this.getPredicate(context); const extraSelections = this.getExtraSelections(context); @@ -105,7 +107,10 @@ export class DeleteOperation extends MutationOperation { throw new Error("Transpile error: No relationship found!"); } const filterSubqueries = wrapSubqueriesInCypherCalls(context, this.filters, [context.target]); - const authBeforeSubqueries = this.getAuthFilterSubqueries(context); + + const authBeforeSubqueries = this.getAuthFilterSubqueries(context).map((c) => { + return new Cypher.Call(c).importWith(context.target); + }); const predicate = this.getPredicate(context); const extraSelections = this.getExtraSelections(context); const collect = Cypher.collect(context.target).distinct(); diff --git a/packages/graphql/tests/integration/issues/5515.int.test.ts b/packages/graphql/tests/integration/issues/5515.int.test.ts new file mode 100644 index 0000000000..6ad882ce3c --- /dev/null +++ b/packages/graphql/tests/integration/issues/5515.int.test.ts @@ -0,0 +1,117 @@ +/* + * 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 { createBearerToken } from "../../utils/create-bearer-token"; +import type { UniqueType } from "../../utils/graphql-types"; +import { TestHelper } from "../../utils/tests-helper"; + +describe("https://github.com/neo4j/graphql/issues/5515", () => { + const testHelper = new TestHelper(); + + const secret = "dontpanic"; + + 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, + features: { + authorization: { + key: secret, + }, + }, + }); + }); + + afterAll(async () => { + await testHelper.close(); + }); + + test("should delete categories with auth filters", async () => { + await testHelper.executeCypher(` + CREATE (:${User} {id: "1234"})-[:HAS_CABINET]->(:${Cabinet})-[:HAS_CATEGORY]->(:${Category} {id: "category-video"}) + CREATE (:${User} {id: "another"})-[:HAS_CABINET]->(:${Cabinet})-[:HAS_CATEGORY]->(:${Category} {id: "category-video"}) + `); + + const query = /* GraphQL */ ` + mutation { + ${Category.operations.delete}(where: { id: "category-video" }) { + __typename + nodesDeleted + relationshipsDeleted + } + } + `; + + const token = createBearerToken(secret, { sub: "1234" }); + + const response = await testHelper.executeGraphQLWithToken(query, token); + + expect(response.errors).toBeFalsy(); + expect(response.data).toEqual({ + [Category.operations.delete]: { + __typename: "DeleteInfo", + nodesDeleted: 1, + relationshipsDeleted: 1, + }, + }); + }); +}); diff --git a/packages/graphql/tests/tck/issues/5515.test.ts b/packages/graphql/tests/tck/issues/5515.test.ts new file mode 100644 index 0000000000..ce0ea4dfde --- /dev/null +++ b/packages/graphql/tests/tck/issues/5515.test.ts @@ -0,0 +1,105 @@ +/* + * 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/5515", () => { + 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 delete categories with auth filters", async () => { + const query = /* GraphQL */ ` + mutation { + deleteCategories(where: { id: "category-video" }) { + __typename + nodesDeleted + relationshipsDeleted + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "MATCH (this:Category) + CALL { + WITH this + MATCH (this)<-[:HAS_CATEGORY]-(this0:Cabinet) + OPTIONAL MATCH (this0)<-[:HAS_CABINET]-(this1:User) + WITH *, count(this1) AS userCount + WITH * + WHERE (userCount <> 0 AND ($jwt.sub IS NOT NULL AND this1.id = $jwt.sub)) + RETURN count(this0) = 1 AS var2 + } + WITH * + WHERE (this.id = $param1 AND ($isAuthenticated = true AND var2 = true)) + DETACH DELETE this" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"jwt\\": {}, + \\"param1\\": \\"category-video\\", + \\"isAuthenticated\\": false + }" + `); + }); +});