Skip to content

Commit

Permalink
Merge branch 'dev' into schema-model-abstract-types-2
Browse files Browse the repository at this point in the history
  • Loading branch information
a-alle authored Sep 5, 2023
2 parents d456b84 + 6d43410 commit b1ef39b
Show file tree
Hide file tree
Showing 12 changed files with 302 additions and 26 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-bees-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@neo4j/graphql": patch
---

Fix: authorization checks are no longer added for the source nodes of connect operations, when the operation started with a create. The connect operation is likely required to complete before the authorization rules will be satisfied.
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ describe("createConnectAndParams", () => {
refNodes: [node],
parentNode: node,
callbackBucket: new CallbackBucket(context),
source: "CONNECT",
});

expect(result[0]).toMatchInlineSnapshot(`
Expand Down
16 changes: 12 additions & 4 deletions packages/graphql/src/translate/create-connect-and-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function createConnectAndParams({
parentNode,
includeRelationshipValidation,
isFirstLevel = true,
source,
}: {
withVars: string[];
value: any;
Expand All @@ -63,6 +64,7 @@ function createConnectAndParams({
parentNode: Node;
includeRelationshipValidation?: boolean;
isFirstLevel?: boolean;
source: "CREATE" | "UPDATE" | "CONNECT";
}): [string, any] {
checkAuthentication({ context, node: parentNode, targetOperations: ["CREATE_RELATIONSHIP"] });

Expand Down Expand Up @@ -168,12 +170,16 @@ function createConnectAndParams({
}
}

const authorizationNodes = [{ node: relatedNode, variable: nodeName }];
// If the source is a create operation, it is likely that authorization
// rules are not satisfied until connect operation is complete
if (source !== "CREATE") {
authorizationNodes.push({ node: parentNode, variable: parentVar });
}

const authorizationBeforeAndParams = createAuthorizationBeforeAndParams({
context,
nodes: [
{ node: parentNode, variable: parentVar },
{ node: relatedNode, variable: nodeName },
],
nodes: authorizationNodes,
operations: ["CREATE_RELATIONSHIP"],
});

Expand Down Expand Up @@ -356,6 +362,7 @@ function createConnectAndParams({
labelOverride: relField.union ? newRefNode.name : "",
includeRelationshipValidation: true,
isFirstLevel: false,
source: "CONNECT",
});
r.connects.push(recurse[0]);
r.params = { ...r.params, ...recurse[1] };
Expand Down Expand Up @@ -405,6 +412,7 @@ function createConnectAndParams({
parentNode: relatedNode,
labelOverride: relField.union ? newRefNode.name : "",
isFirstLevel: false,
source: "CONNECT",
});
r.connects.push(recurse[0]);
r.params = { ...r.params, ...recurse[1] };
Expand Down
2 changes: 2 additions & 0 deletions packages/graphql/src/translate/create-create-and-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ function createCreateAndParams({
refNodes: [refNode],
labelOverride: unionTypeName,
parentNode: node,
source: "CREATE",
});
res.creates.push(connectAndParams[0]);
res.params = { ...res.params, ...connectAndParams[1] };
Expand Down Expand Up @@ -249,6 +250,7 @@ function createCreateAndParams({
refNodes,
labelOverride: "",
parentNode: node,
source: "CREATE",
});
res.creates.push(connectAndParams[0]);
res.params = { ...res.params, ...connectAndParams[1] };
Expand Down
1 change: 1 addition & 0 deletions packages/graphql/src/translate/create-update-and-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ export default function createUpdateAndParams({
relationField,
labelOverride: relationField.union ? refNode.name : "",
parentNode: node,
source: "UPDATE",
});
subquery.push(connectAndParams[0]);
if (context.subscriptionsEnabled) {
Expand Down
2 changes: 2 additions & 0 deletions packages/graphql/src/translate/translate-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export default async function translateUpdate({
parentNode: node,
labelOverride: "",
includeRelationshipValidation: !!assumeReconnecting,
source: "UPDATE",
});
connectStrs.push(connectAndParams[0]);
cypherParams = { ...cypherParams, ...connectAndParams[1] };
Expand All @@ -255,6 +256,7 @@ export default async function translateUpdate({
withVars,
parentNode: node,
labelOverride: relationField.union ? refNode.name : "",
source: "UPDATE",
});
connectStrs.push(connectAndParams[0]);
cypherParams = { ...cypherParams, ...connectAndParams[1] };
Expand Down
139 changes: 139 additions & 0 deletions packages/graphql/tests/integration/issues/3888.int.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* 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 { Driver, Session } from "neo4j-driver";
import { graphql } from "graphql";
import Neo4j from "../neo4j";
import { Neo4jGraphQL } from "../../../src";
import { UniqueType } from "../../utils/graphql-types";
import { cleanNodes } from "../../utils/clean-nodes";
import { createBearerToken } from "../../utils/create-bearer-token";

describe("https://github.com/neo4j/graphql/issues/3888", () => {
let driver: Driver;
let neo4j: Neo4j;
let neoSchema: Neo4jGraphQL;
let session: Session;
const secret = "secret";

let Post: UniqueType;
let User: UniqueType;

beforeAll(async () => {
neo4j = new Neo4j();
driver = await neo4j.getDriver();
});

beforeEach(async () => {
session = await neo4j.getSession();

Post = new UniqueType("Post");
User = new UniqueType("User");

const typeDefs = `
type ${User} {
id: ID!
}
type ${Post} @authorization(filter: [{ where: { node: { author: { id: "$jwt.sub" } } } }]) {
title: String!
content: String!
author: ${User}! @relationship(type: "AUTHORED", direction: IN)
}
`;

neoSchema = new Neo4jGraphQL({
typeDefs,
driver,
features: {
authorization: {
key: secret,
},
},
});
});

afterEach(async () => {
await cleanNodes(session, [Post, User]);
await session.close();
});

afterAll(async () => {
await driver.close();
});

test("should not raise cardinality error when connecting on create", async () => {
const createUser = `
mutation {
${User.operations.create}(input: [{ id: "michel" }]) {
${User.plural} {
id
}
}
}
`;

const createPost = `
mutation {
${Post.operations.create}(
input: [
{ title: "Test1", content: "Test1", author: { connect: { where: { node: { id: "michel" } } } } }
]
) {
${Post.plural} {
title
author {
id
}
}
}
}
`;

const token = createBearerToken(secret, { sub: "michel" });

const createUserResult = await graphql({
schema: await neoSchema.getSchema(),
source: createUser,
contextValue: neo4j.getContextValues({ token }),
});

expect(createUserResult.errors).toBeFalsy();

const createPostResult = await graphql({
schema: await neoSchema.getSchema(),
source: createPost,
contextValue: neo4j.getContextValues({ token }),
});

expect(createPostResult.errors).toBeFalsy();
expect(createPostResult.data).toEqual({
[Post.operations.create]: {
[Post.plural]: [
{
title: "Test1",
author: {
id: "michel",
},
},
],
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ describe("Cypher Auth Allow", () => {
OPTIONAL MATCH (this_connect_posts0_node:Post)
OPTIONAL MATCH (this_connect_posts0_node)<-[:HAS_POST]-(authorization_this0:User)
WITH *, count(authorization_this0) AS creatorCount
WHERE this_connect_posts0_node.id = $this_connect_posts0_node_param0 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_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]))
WHERE this_connect_posts0_node.id = $this_connect_posts0_node_param0 AND (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND (creatorCount <> 0 AND ($jwt.sub IS NOT NULL AND authorization_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]))
CALL {
WITH *
WITH collect(this_connect_posts0_node) as connectedNodes, collect(this) as parentNodes
Expand Down
Loading

0 comments on commit b1ef39b

Please sign in to comment.