Skip to content

Commit

Permalink
Merge pull request #4233 from a-alle/fix/4223
Browse files Browse the repository at this point in the history
Fix authorization variable prefix
  • Loading branch information
a-alle authored Nov 2, 2023
2 parents a14d465 + abe11cb commit 86d0f59
Show file tree
Hide file tree
Showing 10 changed files with 595 additions and 57 deletions.
5 changes: 5 additions & 0 deletions .changeset/lovely-items-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@neo4j/graphql": patch
---

Improve authorization variable prefix on create operations
34 changes: 24 additions & 10 deletions packages/graphql/src/translate/create-create-and-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ function createCreateAndParams({
withVars,
includeRelationshipValidation,
topLevelNodeVariable,
idx = 0, // used to build authorization variable in auth subqueries
refNodeIndex = 0, // used to build authorization variable in auth subqueries
authorizationPrefix = {
inputIndex: 0,
reducerIndex: 0,
createIndex: 0,
refNodeIndex: 0,
},
}: {
input: any;
varName: string;
Expand All @@ -71,8 +75,13 @@ function createCreateAndParams({
withVars: string[];
includeRelationshipValidation?: boolean;
topLevelNodeVariable?: string;
idx?: number;
refNodeIndex?: number;
//used to build authorization variable in auth subqueries
authorizationPrefix?: {
inputIndex: number; // index of the input
reducerIndex: number; // index of the reducer in the context of the input
createIndex: number; // index of the create in the context of a run of the reducer
refNodeIndex: number; // when a relationship is to an abstract type, this is the index of the refNode in the context of a run of the reducer
};
}): CreateAndParams {
const conflictingProperties = findConflictingProperties({ node, input });
if (conflictingProperties.length > 0) {
Expand All @@ -84,7 +93,7 @@ function createCreateAndParams({
}
checkAuthentication({ context, node, targetOperations: ["CREATE"] });

function reducer(res: Res, [key, value]: [string, any]): Res {
function reducer(res: Res, [key, value]: [string, any], reducerIndex): Res {
const varNameKey = `${varName}_${key}`;
const relationField = node.relationFields.find((x) => key === x.fieldName);
const primitiveField = node.primitiveFields.find((x) => key === x.fieldName);
Expand Down Expand Up @@ -130,7 +139,7 @@ function createCreateAndParams({
}

const creates = relationField.typeMeta.array ? v.create : [v.create];
creates.forEach((create, index) => {
creates.forEach((create, createIndex) => {
if (relationField.interface && !create.node[refNode.name]) {
return;
}
Expand All @@ -139,7 +148,7 @@ function createCreateAndParams({
res.creates.push(`\nWITH *`);
}

const baseName = `${varNameKey}${relationField.union ? "_" : ""}${unionTypeName}${index}`;
const baseName = `${varNameKey}${relationField.union ? "_" : ""}${unionTypeName}${createIndex}`;
const nodeName = `${baseName}_node`;
const propertiesName = `${baseName}_relationship`;

Expand All @@ -157,8 +166,12 @@ function createCreateAndParams({
withVars: [...withVars, nodeName],
includeRelationshipValidation: false,
topLevelNodeVariable,
idx: idx + index + 1,
refNodeIndex: refNodeIndex,
authorizationPrefix: {
inputIndex: authorizationPrefix.inputIndex + 1,
reducerIndex,
createIndex,
refNodeIndex,
},
});
res.creates.push(nestedCreate);
res.params = { ...res.params, ...params };
Expand Down Expand Up @@ -357,6 +370,7 @@ function createCreateAndParams({
}

const { authorizationPredicates, authorizationSubqueries } = meta;
const authorizationVariablesPrefix = `${authorizationPrefix.inputIndex}_${authorizationPrefix.reducerIndex}_${authorizationPrefix.refNodeIndex}_${authorizationPrefix.createIndex}_`;
const authorizationAndParams = createAuthorizationAfterAndParams({
context,
nodes: [
Expand All @@ -366,7 +380,7 @@ function createCreateAndParams({
},
],
operations: ["CREATE"],
indexPrefix: `${refNodeIndex}_${idx}_`,
indexPrefix: authorizationVariablesPrefix,
});

if (authorizationAndParams) {
Expand Down
232 changes: 232 additions & 0 deletions packages/graphql/tests/integration/issues/4223.int.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
/*
* 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 } from "neo4j-driver";
import Neo4j from "../neo4j";
import { Neo4jGraphQL } from "../../../src/classes";
import { graphql } from "graphql";
import { UniqueType } from "../../utils/graphql-types";
import gql from "graphql-tag";

describe("https://github.com/neo4j/graphql/issues/4223", () => {
let driver: Driver;
let neo4j: Neo4j;

const User = new UniqueType("User");
const Tenant = new UniqueType("Tenant");
const Settings = new UniqueType("Settings");
const OpeningDay = new UniqueType("OpeningDay");
const OpeningHoursInterval = new UniqueType("OpeningHoursInterval");
const MyWorkspace = new UniqueType("MyWorkspace");

const typeDefs = gql`
type JWT @jwt {
id: String
roles: [String]
}
type ${User.name} @authorization(validate: [{ where: { node: { userId: "$jwt.id" } }, operations: [READ] }]) {
userId: String! @unique
adminAccess: [${Tenant.name}!]! @relationship(type: "ADMIN_IN", direction: OUT)
}
type ${Tenant.name} @authorization(validate: [{ where: { node: { admins: { userId: "$jwt.id" } } } }]) {
id: ID! @id
settings: ${Settings.name}! @relationship(type: "VEHICLECARD_OWNER", direction: IN)
admins: [${User.name}!]! @relationship(type: "ADMIN_IN", direction: IN)
}
type ${Settings.name} @authorization(validate: [{ where: { node: { tenant: { admins: { userId: "$jwt.id" } } } } }]) {
id: ID! @id
openingDays: [${OpeningDay.name}!]! @relationship(type: "VALID_GARAGES", direction: OUT)
myWorkspace: ${MyWorkspace.name}! @relationship(type: "HAS_WORKSPACE_SETTINGS", direction: OUT)
tenant: ${Tenant.name}! @relationship(type: "VEHICLECARD_OWNER", direction: OUT)
}
type ${OpeningDay.name}
@authorization(
validate: [{ where: { node: { settings: { tenant: { admins: { userId: "$jwt.id" } } } } } }]
) {
id: ID! @id
settings: ${Settings.name} @relationship(type: "VALID_GARAGES", direction: IN)
open: [${OpeningHoursInterval.name}!]! @relationship(type: "HAS_OPEN_INTERVALS", direction: OUT)
}
type ${OpeningHoursInterval.name}
@authorization(
validate: [
{ where: { node: { openingDay: { settings: { tenant: { admins: { userId: "$jwt.id" } } } } } } }
]
) {
name: String
openingDay: ${OpeningDay.name}! @relationship(type: "HAS_OPEN_INTERVALS", direction: IN)
}
type ${MyWorkspace.name}
@authorization(
validate: [
{
where: {
node: {
settings: { tenant: { admins: { userId: "$jwt.id" } } }
}
}
}
]
) {
settings: ${Settings.name}!
@relationship(type: "HAS_WORKSPACE_SETTINGS", direction: IN)
workspace: String
updatedBy: String
@populatedBy(
callback: "getUserIDFromContext"
operations: [CREATE, UPDATE]
)
}
`;

const ADD_TENANT = `
mutation addTenant($input: [${Tenant.name}CreateInput!]!) {
${Tenant.operations.create}(input: $input) {
${Tenant.plural} {
id
admins {
userId
}
settings {
openingDays {
open {
name
}
}
myWorkspace {
workspace
}
}
}
}
}
`;

let tenantVariables: Record<string, any>;
let myUserId: string;

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

beforeEach(() => {
myUserId = Math.random().toString(36).slice(2, 7);
tenantVariables = {
input: {
admins: {
create: [
{
node: { userId: myUserId },
},
],
},
settings: {
create: {
node: {
openingDays: {
create: {
node: {
open: {
create: {
node: {
name: "lambo",
},
},
},
},
},
},
myWorkspace: {
create: {
node: {
workspace: "myWorkspace",
},
},
},
},
},
},
},
};
});

afterEach(async () => {
const session = driver.session();
await session.run(` match (n) detach delete n`);
await session.close();
});

afterAll(async () => {
await driver.close();
});
test("create tenant with nested openingDays and openHoursInterval - subscriptions disabled", async () => {
const neo4jGraphql = new Neo4jGraphQL({
typeDefs,
driver,
features: {
populatedBy: {
callbacks: {
getUserIDFromContext: () => "hi",
},
},
},
});
const schema = await neo4jGraphql.getSchema();

const addTenantResponse = await graphql({
schema,
source: ADD_TENANT,
variableValues: tenantVariables,
contextValue: neo4j.getContextValues({ jwt: { id: myUserId } }),
});

expect(addTenantResponse.errors).toBeUndefined();
});

test("create tenant with nested openingDays and openHoursInterval - subscriptions enabled", async () => {
const neo4jGraphql = new Neo4jGraphQL({
typeDefs,
driver,
features: {
subscriptions: true,
populatedBy: {
callbacks: {
getUserIDFromContext: () => "hi",
},
},
},
});
const schema = await neo4jGraphql.getSchema();

const addTenantResponse = await graphql({
schema,
source: ADD_TENANT,
variableValues: tenantVariables,
contextValue: neo4j.getContextValues({ jwt: { id: myUserId } }),
});

expect(addTenantResponse.errors).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ describe("Cypher Auth Allow", () => {
RETURN c AS this0_contentPost0_node_creator_User_unique_ignored
}
WITH *
OPTIONAL MATCH (this0_contentPost0_node)<-[:HAS_CONTENT]-(authorization_1_1_this0:User)
WITH *, count(authorization_1_1_this0) AS creatorCount
OPTIONAL MATCH (this0_contentPost0_node)<-[:HAS_CONTENT]-(authorization_1_2_1_0_this0:User)
WITH *, count(authorization_1_2_1_0_this0) AS creatorCount
WITH *
WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this0_contentPost0_node_creator0_node.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_1_1_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0])
WHERE apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this0_contentPost0_node_creator0_node.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_1_2_1_0_this0.id = $jwt.sub))), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0])
RETURN this0
}
CALL {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ describe("Cypher Auth Where with Roles", () => {
RETURN count(*) AS connect_this0_posts_connect_Post0
}
WITH *
WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND (($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization_0_0_param2 IN $jwt.roles))) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization_0_0_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])
WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND (($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization_0_0_0_0_param2 IN $jwt.roles))) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization_0_0_0_0_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])
RETURN this0
}
CALL {
Expand All @@ -863,8 +863,8 @@ describe("Cypher Auth Where with Roles", () => {
\\"authorization_param3\\": \\"admin\\",
\\"authorization_param4\\": \\"user\\",
\\"authorization_param5\\": \\"admin\\",
\\"authorization_0_0_param2\\": \\"user\\",
\\"authorization_0_0_param3\\": \\"admin\\",
\\"authorization_0_0_0_0_param2\\": \\"user\\",
\\"authorization_0_0_0_0_param3\\": \\"admin\\",
\\"resolvedCallbacks\\": {}
}"
`);
Expand Down Expand Up @@ -922,7 +922,7 @@ describe("Cypher Auth Where with Roles", () => {
RETURN count(*) AS connect_this0_posts_connect_Post0
}
WITH *
WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND (($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization_0_0_param2 IN $jwt.roles))) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization_0_0_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])
WHERE apoc.util.validatePredicate(NOT (($isAuthenticated = true AND (($jwt.sub IS NOT NULL AND this0.id = $jwt.sub) AND ($jwt.roles IS NOT NULL AND $authorization_0_0_0_0_param2 IN $jwt.roles))) OR ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $authorization_0_0_0_0_param3 IN $jwt.roles))), \\"@neo4j/graphql/FORBIDDEN\\", [0])
RETURN this0
}
CALL {
Expand All @@ -949,8 +949,8 @@ describe("Cypher Auth Where with Roles", () => {
\\"authorization_param3\\": \\"admin\\",
\\"authorization_param4\\": \\"user\\",
\\"authorization_param5\\": \\"admin\\",
\\"authorization_0_0_param2\\": \\"user\\",
\\"authorization_0_0_param3\\": \\"admin\\",
\\"authorization_0_0_0_0_param2\\": \\"user\\",
\\"authorization_0_0_0_0_param3\\": \\"admin\\",
\\"resolvedCallbacks\\": {}
}"
`);
Expand Down
Loading

0 comments on commit 86d0f59

Please sign in to comment.