From 16d4100e3b1521252a72907838e5ad9439f5604e Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 16 Jan 2025 16:42:26 +0000 Subject: [PATCH 01/13] Authorization for simple relationship filter --- .../ast/filters/RelationshipFilter.ts | 236 ++++++++++-------- .../filters/aggregation/AggregationFilter.ts | 16 ++ .../AuthRelationshipFilter.ts | 6 +- .../queryAST/factory/AuthFilterFactory.ts | 6 +- .../queryAST/factory/FilterFactory.ts | 192 ++++++++++---- .../queryAST/factory/OperationFactory.ts | 3 +- .../factory/Operations/AggregateFactory.ts | 8 +- .../factory/Operations/ConnectionFactory.ts | 1 + .../factory/Operations/DeleteFactory.ts | 7 +- .../translate/where/create-where-predicate.ts | 17 +- .../tests/tck/advanced-filtering.test.ts | 2 +- .../graphql/tests/tck/issues/5534.test.ts | 113 +++++++++ 12 files changed, 443 insertions(+), 164 deletions(-) create mode 100644 packages/graphql/tests/tck/issues/5534.test.ts diff --git a/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts index d89e82a48a..93898ac773 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts @@ -29,6 +29,7 @@ import type { QueryASTContext } from "../QueryASTContext"; import type { QueryASTNode } from "../QueryASTNode"; import type { RelationshipWhereOperator } from "./Filter"; import { Filter } from "./Filter"; +import type { AuthorizationFilters } from "./authorization-filters/AuthorizationFilters"; export class RelationshipFilter extends Filter { protected targetNodeFilters: Filter[] = []; @@ -42,6 +43,8 @@ export class RelationshipFilter extends Filter { /** Variable to be used if relationship need to get the count (i.e. 1-1 relationships) */ protected countVariable = new Cypher.Variable(); + protected authFilters: AuthorizationFilters[] = []; + constructor({ relationship, operator, @@ -57,10 +60,63 @@ export class RelationshipFilter extends Filter { this.target = target; } + public getPredicate(queryASTContext: QueryASTContext): Cypher.Predicate | undefined { + if (this.subqueryPredicate) { + return this.subqueryPredicate; + } + const nestedContext = this.getNestedContext(queryASTContext); + + const pattern = new Cypher.Pattern(nestedContext.source as Cypher.Node) + .related({ + type: this.relationship.type, + direction: this.relationship.getCypherDirection(), + }) + .to(nestedContext.target, { + labels: getEntityLabels(this.target, nestedContext.neo4jGraphQLContext), + }); + + const predicate = this.createRelationshipOperation(pattern, nestedContext); + return predicate; + } + + public getSubqueries(context: QueryASTContext): Cypher.Clause[] { + // NOTE: not using getNestedContext because this should not be memoized in ALL operations + const target = new Cypher.Node(); + const relationship = new Cypher.Relationship(); + const nestedContext = context.push({ + target, + relationship, + }); + + const subqueries: Cypher.Clause[] = []; + + const nestedSubqueries = this.targetNodeFilters.flatMap((f) => f.getSubqueries(nestedContext)); + const nestedSelection = this.getNestedSelectionSubqueries(nestedContext); + const authFilterSubqueries = this.getAuthFilterSubqueries(nestedContext); + + if (nestedSubqueries.length > 0) { + subqueries.push(...this.getNestedSubqueries(nestedContext)); + } + + if (nestedSelection.length > 0) { + subqueries.push(...nestedSelection); + } + + if (authFilterSubqueries.length > 0) { + subqueries.push(...authFilterSubqueries); + } + + return subqueries; + } + public getChildren(): QueryASTNode[] { return this.targetNodeFilters; } + public addAuthFilters(...filter: AuthorizationFilters[]) { + this.authFilters.push(...filter); + } + public addTargetNodeFilter(...filter: Filter[]): void { this.targetNodeFilters.push(...filter); } @@ -81,7 +137,78 @@ export class RelationshipFilter extends Filter { return nestedContext; } - protected getNestedSelectionSubqueries(context: QueryASTContext): Cypher.Clause[] { + protected getSingleRelationshipOperation({ + pattern, + context, + innerPredicate, + }: { + pattern: Cypher.Pattern; + context: QueryASTContext; + innerPredicate: Cypher.Predicate; + }): Cypher.Predicate { + if (!context.hasTarget()) { + throw new Error("No parent node found!"); + } + const patternComprehension = new Cypher.PatternComprehension(pattern) + .map(new Cypher.Literal(1)) + .where(innerPredicate); + return Cypher.single(context.target, patternComprehension, new Cypher.Literal(true)); + } + + protected createRelationshipOperation( + pattern: Cypher.Pattern, + context: QueryASTContext + ): Cypher.Predicate | undefined { + const predicates = this.targetNodeFilters.map((c) => c.getPredicate(context)); + const authPredicates = this.getAuthFilterPredicate(context); + const innerPredicate = Cypher.and(...authPredicates, ...predicates); + + switch (this.operator) { + case "ALL": { + if (!innerPredicate) { + return; + } + const match = new Cypher.Match(pattern).where(innerPredicate); + const negativeMatch = new Cypher.Match(pattern).where(Cypher.not(innerPredicate)); + // Testing "ALL" requires testing that at least one element exists and that no elements not matching the filter exists + return Cypher.and(new Cypher.Exists(match), Cypher.not(new Cypher.Exists(negativeMatch))); + } + case "SINGLE": { + if (!innerPredicate) { + return; + } + + return this.getSingleRelationshipOperation({ + pattern, + context, + innerPredicate, + }); + } + case "NONE": + case "SOME": { + const match = new Cypher.Match(pattern); + if (innerPredicate) { + match.where(innerPredicate); + } + + const exists = new Cypher.Exists(match); + if (this.operator === "NONE") { + return Cypher.not(exists); + } + return exists; + } + } + } + + private getAuthFilterSubqueries(context: QueryASTContext): Cypher.Clause[] { + return this.authFilters.flatMap((f) => f.getSubqueries(context)); + } + + private getAuthFilterPredicate(context: QueryASTContext): Cypher.Predicate[] { + return filterTruthy(this.authFilters.map((f) => f.getPredicate(context))); + } + + private getNestedSelectionSubqueries(context: QueryASTContext): Cypher.Clause[] { const returnVars: Cypher.Variable[] = []; const nestedSelection = filterTruthy( @@ -132,31 +259,7 @@ export class RelationshipFilter extends Filter { return nestedSelection; } - public getSubqueries(context: QueryASTContext): Cypher.Clause[] { - // NOTE: not using getNestedContext because this should not be memoized in ALL operations - const target = new Cypher.Node(); - const relationship = new Cypher.Relationship(); - const nestedContext = context.push({ - target, - relationship, - }); - - const subqueries: Cypher.Clause[] = []; - - const nestedSubqueries = this.targetNodeFilters.flatMap((f) => f.getSubqueries(nestedContext)); - const nestedSelection = this.getNestedSelectionSubqueries(nestedContext); - - if (nestedSubqueries.length > 0) { - subqueries.push(...this.getNestedSubqueries(nestedContext)); - } - - if (nestedSelection.length > 0) { - subqueries.push(...nestedSelection); - } - return subqueries; - } - - protected getNestedSubqueries(context: QueryASTContext): Cypher.Clause[] { + private getNestedSubqueries(context: QueryASTContext): Cypher.Clause[] { const pattern = new Cypher.Pattern(context.source) .related({ direction: this.relationship.getCypherDirection(), @@ -266,85 +369,4 @@ export class RelationshipFilter extends Filter { throw new Error("Not supported"); } } - - public getPredicate(queryASTContext: QueryASTContext): Cypher.Predicate | undefined { - if (this.subqueryPredicate) { - return this.subqueryPredicate; - } - const nestedContext = this.getNestedContext(queryASTContext); - - const pattern = new Cypher.Pattern(nestedContext.source as Cypher.Node) - .related({ - type: this.relationship.type, - direction: this.relationship.getCypherDirection(), - }) - .to(nestedContext.target, { - labels: getEntityLabels(this.target, nestedContext.neo4jGraphQLContext), - }); - - const predicate = this.createRelationshipOperation(pattern, nestedContext); - return predicate; - } - - protected getSingleRelationshipOperation({ - pattern, - queryASTContext, - innerPredicate, - }: { - pattern: Cypher.Pattern; - queryASTContext: QueryASTContext; - innerPredicate: Cypher.Predicate; - }): Cypher.Predicate { - if (!queryASTContext.hasTarget()) { - throw new Error("No parent node found!"); - } - const patternComprehension = new Cypher.PatternComprehension(pattern) - .map(new Cypher.Literal(1)) - .where(innerPredicate); - return Cypher.single(queryASTContext.target, patternComprehension, new Cypher.Literal(true)); - } - - protected createRelationshipOperation( - pattern: Cypher.Pattern, - queryASTContext: QueryASTContext - ): Cypher.Predicate | undefined { - const predicates = this.targetNodeFilters.map((c) => c.getPredicate(queryASTContext)); - const innerPredicate = Cypher.and(...predicates); - - switch (this.operator) { - case "ALL": { - if (!innerPredicate) { - return; - } - const match = new Cypher.Match(pattern).where(innerPredicate); - const negativeMatch = new Cypher.Match(pattern).where(Cypher.not(innerPredicate)); - // Testing "ALL" requires testing that at least one element exists and that no elements not matching the filter exists - return Cypher.and(new Cypher.Exists(match), Cypher.not(new Cypher.Exists(negativeMatch))); - } - case "SINGLE": { - if (!innerPredicate) { - return; - } - - return this.getSingleRelationshipOperation({ - pattern, - queryASTContext, - innerPredicate, - }); - } - case "NONE": - case "SOME": { - const match = new Cypher.Match(pattern); - if (innerPredicate) { - match.where(innerPredicate); - } - - const exists = new Cypher.Exists(match); - if (this.operator === "NONE") { - return Cypher.not(exists); - } - return exists; - } - } - } } diff --git a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts index f1d7ff974e..5ac0cfbb7c 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts @@ -20,10 +20,12 @@ import Cypher from "@neo4j/cypher-builder"; import { InterfaceEntityAdapter } from "../../../../../schema-model/entity/model-adapters/InterfaceEntityAdapter"; import type { RelationshipAdapter } from "../../../../../schema-model/relationship/model-adapters/RelationshipAdapter"; +import { filterTruthy } from "../../../../../utils/utils"; import { hasTarget } from "../../../utils/context-has-target"; import { getEntityLabels } from "../../../utils/create-node-from-entity"; import type { QueryASTContext } from "../../QueryASTContext"; import type { QueryASTNode } from "../../QueryASTNode"; +import type { AuthorizationFilters } from "../authorization-filters/AuthorizationFilters"; import { Filter } from "../Filter"; import type { LogicalFilter } from "../LogicalFilter"; import type { AggregationPropertyFilter } from "./AggregationPropertyFilter"; @@ -34,6 +36,8 @@ export class AggregationFilter extends Filter { private filters: Array = []; + private authFilters: AuthorizationFilters[] = []; + private subqueryReturnVariable: Cypher.Variable | undefined; constructor(relationship: RelationshipAdapter) { @@ -45,6 +49,10 @@ export class AggregationFilter extends Filter { this.filters.push(...filter); } + public addAuthFilters(...filter: AuthorizationFilters[]) { + this.authFilters.push(...filter); + } + public getChildren(): QueryASTNode[] { return [...this.filters]; } @@ -102,4 +110,12 @@ export class AggregationFilter extends Filter { if (!this.subqueryReturnVariable) return undefined; return Cypher.eq(this.subqueryReturnVariable, Cypher.true); } + + private getAuthFilterSubqueries(context: QueryASTContext): Cypher.Clause[] { + return this.authFilters.flatMap((f) => f.getSubqueries(context)); + } + + private getAuthFilterPredicate(context: QueryASTContext): Cypher.Predicate[] { + return filterTruthy(this.authFilters.map((f) => f.getPredicate(context))); + } } diff --git a/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthRelationshipFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthRelationshipFilter.ts index 8c5ed6f6c7..afdd750e1c 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthRelationshipFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthRelationshipFilter.ts @@ -45,9 +45,9 @@ export class AuthRelationshipFilter extends RelationshipFilter { protected createRelationshipOperation( pattern: Cypher.Pattern, - queryASTContext: QueryASTContext + context: QueryASTContext ): Cypher.Predicate | undefined { - const predicates = this.targetNodeFilters.map((c) => c.getPredicate(queryASTContext)); + const predicates = this.targetNodeFilters.map((c) => c.getPredicate(context)); const innerPredicate = Cypher.and(...predicates); if (!innerPredicate) { return; @@ -62,7 +62,7 @@ export class AuthRelationshipFilter extends RelationshipFilter { case "SINGLE": { return this.getSingleRelationshipOperation({ pattern, - queryASTContext, + context, innerPredicate, }); } diff --git a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts index a376d13764..7be3725408 100644 --- a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts @@ -77,7 +77,7 @@ export class AuthFilterFactory extends FilterFactory { } if (key === "node") { - return this.createNodeFilters(entity, value); + return this.createNodeFilters(entity, value, context); } else if (key === "jwt") { return this.createJWTFilters(context.authorization.jwtParam, value, context); } @@ -147,12 +147,14 @@ export class AuthFilterFactory extends FilterFactory { operator, attachedTo, relationship, + context, }: { attribute: AttributeAdapter; comparisonValue: unknown; operator: FilterOperator | undefined; attachedTo?: "node" | "relationship"; relationship?: RelationshipAdapter; + context: Neo4jGraphQLTranslationContext; }): Filter { const isCypherVariable = comparisonValue instanceof Cypher.Variable || @@ -182,6 +184,7 @@ export class AuthFilterFactory extends FilterFactory { target: entityAdapter, operator: legacyOperator, attribute, + context, }); }); return new LogicalFilter({ @@ -198,6 +201,7 @@ export class AuthFilterFactory extends FilterFactory { target: entityAdapter, operator, attribute, + context, }), }); } diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index bc7194abc0..ddde33e065 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -25,6 +25,7 @@ import type { UnionEntityAdapter } from "../../../schema-model/entity/model-adap import { RelationshipAdapter } from "../../../schema-model/relationship/model-adapters/RelationshipAdapter"; import { getEntityAdapter } from "../../../schema-model/utils/get-entity-adapter"; import type { ConnectionWhereArg, GraphQLWhereArg } from "../../../types"; +import type { Neo4jGraphQLTranslationContext } from "../../../types/neo4j-graphql-translation-context"; import { fromGlobalId } from "../../../utils/global-ids"; import { asArray, filterTruthy } from "../../../utils/utils"; import { isLogicalOperator } from "../../utils/logical-operators"; @@ -77,8 +78,8 @@ export class FilterFactory { private createConnectionFilter( relationship: RelationshipAdapter, where: ConnectionWhereArg, - - operator: RelationshipWhereOperator + operator: RelationshipWhereOperator, + context: Neo4jGraphQLTranslationContext ): Filter[] { if ( isInterfaceEntity(relationship.target) && @@ -89,7 +90,12 @@ export class FilterFactory { target: relationship.target, operator, }); - const filters = this.createConnectionPredicates({ rel: relationship, entity: relationship.target, where }); + const filters = this.createConnectionPredicates({ + rel: relationship, + entity: relationship.target, + where, + context, + }); connectionFilter.addFilters(filters); return asArray(connectionFilter); } @@ -113,6 +119,7 @@ export class FilterFactory { entity: concreteEntity, where, partialOf, + context, }); connectionFilter.addFilters(filters); connectionFilters.push(connectionFilter); @@ -126,11 +133,13 @@ export class FilterFactory { entity, where, partialOf, + context, }: { rel?: RelationshipAdapter; entity: EntityAdapter; where: GraphQLWhereArg | GraphQLWhereArg[]; partialOf?: InterfaceEntityAdapter | UnionEntityAdapter; + context: Neo4jGraphQLTranslationContext; }): Filter[] { let entityWhere = where; if (rel && isUnionEntity(rel.target) && where[entity.name]) { @@ -139,7 +148,13 @@ export class FilterFactory { const filters = asArray(entityWhere).flatMap((nestedWhere) => { return Object.entries(nestedWhere).flatMap(([key, value]: [string, GraphQLWhereArg]) => { if (isLogicalOperator(key)) { - const nestedFilters = this.createConnectionPredicates({ rel, entity, where: value, partialOf }); + const nestedFilters = this.createConnectionPredicates({ + rel, + entity, + where: value, + partialOf, + context, + }); return [ new LogicalFilter({ operation: key, @@ -149,7 +164,7 @@ export class FilterFactory { } if (rel && key === "edge") { - return this.createEdgeFilters(rel, value); + return this.createEdgeFilters(rel, value, context); } if (key === "node") { @@ -158,15 +173,17 @@ export class FilterFactory { entity: partialOf, targetEntity: entity, whereFields: value, + context, }); } else if (isInterfaceEntity(entity)) { return this.createInterfaceNodeFilters({ entity, whereFields: value, relationship: rel, + context, }); } - return this.createNodeFilters(entity, value); + return this.createNodeFilters(entity, value, context); } }); }); @@ -177,10 +194,12 @@ export class FilterFactory { attribute, comparisonValue, operator, + context, }: { attribute: AttributeAdapter; comparisonValue: GraphQLWhereArg; operator: FilterOperator | undefined; + context: Neo4jGraphQLTranslationContext; }): Filter | Filter[] { const selection = new CustomCypherSelection({ operationField: attribute, @@ -204,6 +223,7 @@ export class FilterFactory { target: entityAdapter, operator: legacyOperator, attribute, + context, }); }); return this.wrapMultipleFiltersInLogical(genericFilters); @@ -215,6 +235,7 @@ export class FilterFactory { target: entityAdapter, operator: operator ?? "SOME", attribute, + context, }); } @@ -234,18 +255,21 @@ export class FilterFactory { comparisonValue, operator, attachedTo, + context, }: { attribute: AttributeAdapter; relationship?: RelationshipAdapter; comparisonValue: GraphQLWhereArg; operator: FilterOperator | undefined; attachedTo?: "node" | "relationship"; + context: Neo4jGraphQLTranslationContext; }): Filter | Filter[] { if (attribute.annotations.cypher) { return this.createCypherFilter({ attribute, comparisonValue, operator, + context, }); } // Implicit _EQ filters are removed but the argument "operator" can still be undefined in some cases, for instance: @@ -281,7 +305,8 @@ export class FilterFactory { private createRelationshipFilter( relationship: RelationshipAdapter, where: GraphQLWhereArg, - operator: RelationshipWhereOperator | undefined + operator: RelationshipWhereOperator | undefined, + context: Neo4jGraphQLTranslationContext ): Filter[] { /** * The logic below can be confusing, but it's to handle the following cases: @@ -302,9 +327,18 @@ export class FilterFactory { operator: operator ?? "SOME", }); + const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ + entity: concreteEntity, + operations: ["READ"], + // attributes: this.getSelectedAttributes(entity, projectionFields), + context, + }); + + relationshipFilter.addAuthFilters(...authFilters); + if (!isNull) { const entityWhere = where[concreteEntity.name] ?? where; - const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere); + const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere, context); relationshipFilter.addTargetNodeFilter(...targetNodeFilters); } @@ -320,12 +354,14 @@ export class FilterFactory { where, attribute, operator, + context, }: { selection: CustomCypherSelection; target: EntityAdapter; where: GraphQLWhereArg; operator: RelationshipWhereOperator | undefined; attribute: AttributeAdapter; + context: Neo4jGraphQLTranslationContext; }): Filter[] { /** * The logic below can be confusing, but it's to handle the following cases: @@ -356,7 +392,7 @@ export class FilterFactory { if (!isNull) { const entityWhere = where[concreteEntity.name] ?? where; - const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere); + const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere, context); filter.addTargetNodeFilter(...targetNodeFilters); } @@ -411,15 +447,17 @@ export class FilterFactory { targetEntity, whereFields, relationship, + context, }: { entity: InterfaceEntityAdapter; targetEntity?: ConcreteEntityAdapter; whereFields: Record; relationship?: RelationshipAdapter; + context: Neo4jGraphQLTranslationContext; }): Filter[] { const filters = filterTruthy( Object.entries(whereFields).flatMap(([key, value]): Filter | Filter[] | undefined => { - return this.parseEntryFilter({ entity, key, value, targetEntity, relationship }); + return this.parseEntryFilter({ entity, key, value, targetEntity, relationship, context }); }) ); return this.wrapMultipleFiltersInLogical(filters); @@ -427,14 +465,15 @@ export class FilterFactory { public createNodeFilters( entity: ConcreteEntityAdapter | UnionEntityAdapter, - whereFields: Record + whereFields: Record, + context: Neo4jGraphQLTranslationContext ): Filter[] { if (isUnionEntity(entity)) { return []; } const filters = filterTruthy( Object.entries(whereFields).flatMap(([key, value]): Filter | Filter[] | undefined => { - return this.parseEntryFilter({ entity, key, value }); + return this.parseEntryFilter({ entity, key, value, context }); }) ); return this.wrapMultipleFiltersInLogical(filters); @@ -446,12 +485,14 @@ export class FilterFactory { value, targetEntity, relationship, + context, }: { entity: ConcreteEntityAdapter | InterfaceEntityAdapter; key: string; value: any; targetEntity?: ConcreteEntityAdapter; relationship?: RelationshipAdapter; + context: Neo4jGraphQLTranslationContext; }): Filter | Filter[] { const valueAsArray = asArray(value); if (isLogicalOperator(key)) { @@ -464,6 +505,7 @@ export class FilterFactory { value: nestedValue, targetEntity, relationship, + context, }) ); }); @@ -486,6 +528,7 @@ export class FilterFactory { operator, isConnection, isAggregate, + context, }); } } else { @@ -503,6 +546,7 @@ export class FilterFactory { operator, isConnection, isAggregate, + context, }); } if (key === "typename") { @@ -517,7 +561,7 @@ export class FilterFactory { if (!isInterfaceEntity(entity) && !attribute) { if (fieldName === "id" && entity.globalIdField) { - return this.createRelayIdPropertyFilter(entity, operator, value); + return this.createRelayIdPropertyFilter(entity, operator, value, context); } } if (!attribute) { @@ -526,7 +570,7 @@ export class FilterFactory { // This is a bit hacky, basically skipping cypher fields and federation strings being passed to filterFactory if (!operator && !attribute.annotations.cypher?.targetEntity && typeof value === "object") { - return this.parseGenericFilters(entity, fieldName, value, relationship); + return this.parseGenericFilters({ entity, fieldName, value, relationship, context }); } return this.createPropertyFilter({ @@ -534,31 +578,46 @@ export class FilterFactory { comparisonValue: value, operator: operator, relationship, + context, }); } - private parseGenericFilters( - entity: ConcreteEntityAdapter | RelationshipAdapter | InterfaceEntityAdapter, - fieldName: string, - value: Record, - relationship?: RelationshipAdapter - ): Filter | Filter[] { + private parseGenericFilters({ + entity, + fieldName, + value, + relationship, + context, + }: { + entity: ConcreteEntityAdapter | RelationshipAdapter | InterfaceEntityAdapter; + fieldName: string; + value: Record; + relationship?: RelationshipAdapter; + context: Neo4jGraphQLTranslationContext; + }): Filter | Filter[] { const genericFilters = Object.entries(value).flatMap((filterInput) => { - return this.parseGenericFilter(entity, fieldName, filterInput, relationship); + return this.parseGenericFilter({ entity, fieldName, filterInput, relationship, context }); }); return this.wrapMultipleFiltersInLogical(genericFilters); } - private parseGenericFilter( - entity: ConcreteEntityAdapter | RelationshipAdapter | InterfaceEntityAdapter, - fieldName: string, - filterInput: [string, any], - relationship?: RelationshipAdapter - ): Filter | Filter[] { + private parseGenericFilter({ + entity, + fieldName, + filterInput, + relationship, + context, + }: { + entity: ConcreteEntityAdapter | RelationshipAdapter | InterfaceEntityAdapter; + fieldName: string; + filterInput: [string, any]; + relationship?: RelationshipAdapter; + context: Neo4jGraphQLTranslationContext; + }): Filter | Filter[] { const [rawOperator, value] = filterInput; if (isLogicalOperator(rawOperator)) { const nestedFilters = asArray(value).flatMap((nestedWhere) => { - return this.parseGenericFilter(entity, fieldName, nestedWhere, relationship); + return this.parseGenericFilter({ entity, fieldName, filterInput: nestedWhere, relationship, context }); }); return new LogicalFilter({ operation: rawOperator, @@ -569,7 +628,7 @@ export class FilterFactory { if (rawOperator === "distance") { // Converts new distance filter into the old one to be parsed the same as deprecated syntax const desugaredInput = this.desugarGenericDistanceOperations(value); - return this.parseGenericFilters(entity, fieldName, desugaredInput, relationship); + return this.parseGenericFilters({ entity, fieldName, value: desugaredInput, relationship, context }); } const operator = this.parseGenericOperator(rawOperator); @@ -581,7 +640,7 @@ export class FilterFactory { throw new Error("Transpilation error: Expected concrete entity"); } if (fieldName === "id" && entity.globalIdField) { - return this.createRelayIdPropertyFilter(entity, operator, value); + return this.createRelayIdPropertyFilter(entity, operator, value, context); } throw new Error(`Attribute ${fieldName} not found`); } @@ -592,6 +651,7 @@ export class FilterFactory { operator, attachedTo, relationship, + context, }); return this.wrapMultipleFiltersInLogical(asArray(filters)); } @@ -665,19 +725,19 @@ export class FilterFactory { relationship, value, operator, - isConnection, isAggregate, + context, }: { relationship: RelationshipAdapter; value: Record; operator: FilterOperator | undefined; - isConnection: boolean; isAggregate: boolean; + context: Neo4jGraphQLTranslationContext; }): Filter | Filter[] { if (isAggregate) { - return this.createAggregationFilter(relationship, value as AggregateWhereInput); + return this.createAggregationFilter(relationship, value as AggregateWhereInput, context); } if (!operator) { const genericFilters = Object.entries(value).flatMap(([quantifier, predicate]) => { @@ -688,6 +748,7 @@ export class FilterFactory { operator: legacyOperator, isConnection, isAggregate, + context, }); }); return this.wrapMultipleFiltersInLogical(genericFilters); @@ -697,9 +758,9 @@ export class FilterFactory { throw new Error(`Invalid operator ${operator} for relationship`); } if (isConnection) { - return this.createConnectionFilter(relationship, value as ConnectionWhereArg, operator); + return this.createConnectionFilter(relationship, value as ConnectionWhereArg, operator, context); } - return this.createRelationshipFilter(relationship, value as GraphQLWhereArg, operator); + return this.createRelationshipFilter(relationship, value as GraphQLWhereArg, operator, context); } private getLogicalOperatorForRelatedNodeFilters( @@ -719,9 +780,9 @@ export class FilterFactory { private createRelayIdPropertyFilter( entity: ConcreteEntityAdapter, - operator: FilterOperator | undefined = "EQ", - value: string + value: string, + context: Neo4jGraphQLTranslationContext ): Filter | Filter[] { const relayIdData = fromGlobalId(value); const { typeName, field } = relayIdData; @@ -746,14 +807,19 @@ export class FilterFactory { attribute: idAttribute, comparisonValue: id as unknown as GraphQLWhereArg, operator, + context, }); } - public createEdgeFilters(relationship: RelationshipAdapter, where: GraphQLWhereArg): Filter[] { + public createEdgeFilters( + relationship: RelationshipAdapter, + where: GraphQLWhereArg, + context: Neo4jGraphQLTranslationContext + ): Filter[] { const filterASTs = Object.entries(where).flatMap(([key, value]): Filter | Filter[] | undefined => { if (isLogicalOperator(key)) { const nestedFilters = asArray(value).flatMap((nestedWhere) => { - return this.createEdgeFilters(relationship, nestedWhere); + return this.createEdgeFilters(relationship, nestedWhere, context); }); return new LogicalFilter({ operation: key, @@ -766,12 +832,12 @@ export class FilterFactory { if (!attribute) { // @declareRelationship path. if (fieldName === relationship.propertiesTypeName) { - return this.createEdgeFilters(relationship, value); + return this.createEdgeFilters(relationship, value, context); } return; } if (!operator) { - return this.parseGenericFilters(relationship, fieldName, value); + return this.parseGenericFilters({ entity: relationship, fieldName, value, context }); } return this.createPropertyFilter({ @@ -779,6 +845,7 @@ export class FilterFactory { comparisonValue: value, operator, attachedTo: "relationship", + context, }); }); @@ -842,9 +909,50 @@ export class FilterFactory { return this.wrapMultipleFiltersInLogical(nestedFilters); } - private createAggregationFilter(relationship: RelationshipAdapter, where: AggregateWhereInput): AggregationFilter { + private createAggregationFilter( + relationship: RelationshipAdapter, + where: AggregateWhereInput, + context: Neo4jGraphQLTranslationContext + ): AggregationFilter { + const filteredEntities = getConcreteEntities(relationship.target, where); + const relationshipFilters: RelationshipFilter[] = []; + for (const concreteEntity of filteredEntities) { + const relationshipFilter = this.createRelationshipFilterTreeNode({ + relationship, + target: concreteEntity, + operator: operator ?? "SOME", + }); + + const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ + entity: concreteEntity, + operations: ["READ"], + // attributes: this.getSelectedAttributes(entity, projectionFields), + context, + }); + + relationshipFilter.addAuthFilters(...authFilters); + + if (!isNull) { + const entityWhere = where[concreteEntity.name] ?? where; + const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere, context); + relationshipFilter.addTargetNodeFilter(...targetNodeFilters); + } + + relationshipFilters.push(relationshipFilter); + } + const aggregationFilter = new AggregationFilter(relationship); const nestedFilters = this.getAggregationNestedFilters(where, relationship); + + const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ + entity: concreteEntity, + operations: ["READ"], + // attributes: this.getSelectedAttributes(entity, projectionFields), + context, + }); + + aggregationFilter.addAuthFilters(...authFilters); + aggregationFilter.addFilters(...nestedFilters); return aggregationFilter; diff --git a/packages/graphql/src/translate/queryAST/factory/OperationFactory.ts b/packages/graphql/src/translate/queryAST/factory/OperationFactory.ts index 560f51846a..ebeef19bc4 100644 --- a/packages/graphql/src/translate/queryAST/factory/OperationFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/OperationFactory.ts @@ -318,10 +318,11 @@ export class OperationsFactory { entity: partialOf, targetEntity: entity, whereFields: whereArgs, + context, }); operation.addFilters(...filters); } else { - const filters = this.filterFactory.createNodeFilters(entity, whereArgs); + const filters = this.filterFactory.createNodeFilters(entity, whereArgs, context); operation.addFilters(...filters); } diff --git a/packages/graphql/src/translate/queryAST/factory/Operations/AggregateFactory.ts b/packages/graphql/src/translate/queryAST/factory/Operations/AggregateFactory.ts index e8fade40ca..3fa6605e01 100644 --- a/packages/graphql/src/translate/queryAST/factory/Operations/AggregateFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/Operations/AggregateFactory.ts @@ -170,7 +170,7 @@ export class AggregateFactory { context, }); - const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs); // Aggregation filters only apply to target node + const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs, context); // Aggregation filters only apply to target node operation.addFilters(...filters); operation.addAuthFilters(...authFilters); @@ -274,10 +274,11 @@ export class AggregateFactory { const filters = this.queryASTFactory.filterFactory.createInterfaceNodeFilters({ entity, whereFields: whereArgs, + context, }); operation.addFilters(...filters); } else { - const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs); // Aggregation filters only apply to target node + const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs, context); // Aggregation filters only apply to target node operation.addFilters(...filters); const attributes = this.queryASTFactory.operationsFactory.getSelectedAttributes(entity, nodeRawFields); @@ -306,10 +307,11 @@ export class AggregateFactory { const filters = this.queryASTFactory.filterFactory.createInterfaceNodeFilters({ entity, whereFields: whereArgs, + context, }); operation.addFilters(...filters); } else { - const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs); // Aggregation filters only apply to target node + const filters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArgs, context); // Aggregation filters only apply to target node operation.addFilters(...filters); const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ entity, diff --git a/packages/graphql/src/translate/queryAST/factory/Operations/ConnectionFactory.ts b/packages/graphql/src/translate/queryAST/factory/Operations/ConnectionFactory.ts index d2cf75d839..7123f75984 100644 --- a/packages/graphql/src/translate/queryAST/factory/Operations/ConnectionFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/Operations/ConnectionFactory.ts @@ -377,6 +377,7 @@ export class ConnectionFactory { rel: relationship, entity: target, where: whereArgs, + context, }); operation.setNodeFields(nodeFields); diff --git a/packages/graphql/src/translate/queryAST/factory/Operations/DeleteFactory.ts b/packages/graphql/src/translate/queryAST/factory/Operations/DeleteFactory.ts index 11b02d2a1b..fabc6086bc 100644 --- a/packages/graphql/src/translate/queryAST/factory/Operations/DeleteFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/Operations/DeleteFactory.ts @@ -79,7 +79,7 @@ export class DeleteFactory { target: entity, alias: varName, }); - const nodeFilters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArg.node); + const nodeFilters = this.queryASTFactory.filterFactory.createNodeFilters(entity, whereArg.node, context); const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ entity, operations: ["DELETE"], @@ -215,11 +215,12 @@ export class DeleteFactory { entity: partialOf, targetEntity: target, whereFields: whereArg.node, + context, }); } else { - nodeFilters = this.queryASTFactory.filterFactory.createNodeFilters(target, whereArg.node); + nodeFilters = this.queryASTFactory.filterFactory.createNodeFilters(target, whereArg.node, context); } - const edgeFilters = this.queryASTFactory.filterFactory.createEdgeFilters(relationship, whereArg.edge); + const edgeFilters = this.queryASTFactory.filterFactory.createEdgeFilters(relationship, whereArg.edge, context); const filters = [...nodeFilters, ...edgeFilters]; diff --git a/packages/graphql/src/translate/where/create-where-predicate.ts b/packages/graphql/src/translate/where/create-where-predicate.ts index 6b5879c43e..9401b7704d 100644 --- a/packages/graphql/src/translate/where/create-where-predicate.ts +++ b/packages/graphql/src/translate/where/create-where-predicate.ts @@ -36,6 +36,7 @@ function createWherePredicate({ whereInput, targetElement, targetEntity, + context, }: { factory: QueryASTFactory; queryASTContext: QueryASTContext; @@ -43,23 +44,25 @@ function createWherePredicate({ whereInput: GraphQLWhereArg; targetElement: Cypher.Node | Cypher.Relationship; targetEntity?: ConcreteEntityAdapter; // It's required for interface entities to be passed in + context: Neo4jGraphQLTranslationContext; }): { predicate: Cypher.Predicate | undefined; preComputedSubqueries?: Cypher.CompositeClause | undefined; } { const filters: Filter[] = []; if (entityOrRel instanceof RelationshipAdapter) { - filters.push(...factory.filterFactory.createEdgeFilters(entityOrRel, whereInput)); + filters.push(...factory.filterFactory.createEdgeFilters(entityOrRel, whereInput, context)); } else if (isInterfaceEntity(entityOrRel)) { filters.push( ...factory.filterFactory.createInterfaceNodeFilters({ entity: entityOrRel, targetEntity, whereFields: whereInput, + context, }) ); } else { - filters.push(...factory.filterFactory.createNodeFilters(entityOrRel, whereInput)); + filters.push(...factory.filterFactory.createNodeFilters(entityOrRel, whereInput, context)); } const subqueries = wrapSubqueriesInCypherCalls(queryASTContext, filters, [targetElement]); @@ -105,6 +108,7 @@ export function createWhereNodePredicate({ whereInput, targetElement, targetEntity, + context, }); } @@ -134,5 +138,12 @@ export function createWhereEdgePredicate({ neo4jGraphQLContext: context, }); - return createWherePredicate({ factory, queryASTContext, entityOrRel: relationship, whereInput, targetElement }); + return createWherePredicate({ + factory, + queryASTContext, + entityOrRel: relationship, + whereInput, + targetElement, + context, + }); } diff --git a/packages/graphql/tests/tck/advanced-filtering.test.ts b/packages/graphql/tests/tck/advanced-filtering.test.ts index 05b8cc7e64..10fc5fcdb6 100644 --- a/packages/graphql/tests/tck/advanced-filtering.test.ts +++ b/packages/graphql/tests/tck/advanced-filtering.test.ts @@ -35,7 +35,7 @@ describe("Cypher Advanced Filtering", () => { genres: [Genre!]! @relationship(type: "IN_GENRE", direction: OUT) } - type Genre @node { + type Genre @node @authorization(filter: [{ where: { node: { name: { eq: "some genre" } } } }]) { name: String movies: [Movie!]! @relationship(type: "IN_GENRE", direction: IN) } diff --git a/packages/graphql/tests/tck/issues/5534.test.ts b/packages/graphql/tests/tck/issues/5534.test.ts new file mode 100644 index 0000000000..a8d81d8228 --- /dev/null +++ b/packages/graphql/tests/tck/issues/5534.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 { Neo4jGraphQL } from "../../../src"; +import { formatCypher, formatParams, translateQuery } from "../utils/tck-test-utils"; + +describe("https://github.com/neo4j/graphql/issues/5534", () => { + let typeDefs: string; + let neoSchema: Neo4jGraphQL; + + beforeAll(() => { + typeDefs = /* GraphQL */ ` + type Product + @node + @mutation(operations: []) + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ, AGGREGATE] + where: { AND: [{ node: { isPublic: { eq: true } } }, { node: { isEmpty: { eq: true } } }] } + } + ] + ) + @subscription(events: []) { + """ + Unique Identifier of this product + """ + productId: Int! + isEmpty: Boolean! @default(value: false) + isPublic: Boolean! @default(value: false) + """ + The product variants belonging to this product + """ + variants: [Product!]! + @relationship(type: "PRODUCT_HAS_FAMILY_PRODUCT", direction: IN, nestedOperations: []) + @settable(onCreate: false, onUpdate: false) + } + `; + + neoSchema = new Neo4jGraphQL({ + typeDefs, + }); + }); + + test("should generate authorization for filter", async () => { + const query = /* GraphQL */ ` + query { + products(limit: 1, where: { variantsAggregate: { count: { eq: 1 } } }) { + productId + variantsAggregate { + count + } + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "MATCH (this:Product) + CALL { + WITH this + MATCH (this)<-[this0:PRODUCT_HAS_FAMILY_PRODUCT]-(this1:Product|Banana) + RETURN count(this1) = $param0 AS var2 + } + WITH * + WHERE (var2 = true AND (($param1 IS NOT NULL AND this.isPublic = $param1) AND ($param2 IS NOT NULL AND this.isEmpty = $param2))) + WITH * + LIMIT $param3 + CALL { + WITH this + MATCH (this)<-[this3:PRODUCT_HAS_FAMILY_PRODUCT]-(this4:Product) + WHERE (($param4 IS NOT NULL AND this4.isPublic = $param4) AND ($param5 IS NOT NULL AND this4.isEmpty = $param5)) + RETURN count(this4) AS var5 + } + RETURN this { .productId, variantsAggregate: { count: var5 } } AS this" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"param0\\": { + \\"low\\": 1, + \\"high\\": 0 + }, + \\"param1\\": true, + \\"param2\\": true, + \\"param3\\": { + \\"low\\": 1, + \\"high\\": 0 + }, + \\"param4\\": true, + \\"param5\\": true + }" + `); + }); +}); From c2f08ca8a41337fd0fa4178996535e8a077cb119 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Wed, 22 Jan 2025 16:51:11 +0000 Subject: [PATCH 02/13] Update tests to modern syntax --- .../aggregations/where/edge/interfaces.int.test.ts | 6 +++--- .../where/edge/interface-relationship.test.ts | 14 +++++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts b/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts index 4b47dca55f..a6bc182ebf 100644 --- a/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts +++ b/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts @@ -87,8 +87,8 @@ describe("aggregations-where-edge-string interface relationships of interface ty where: { productionsAggregate: { edge: { - AppearedIn: { role_SHORTEST_LENGTH_LT: 3 } - ActedIn: { role_AVERAGE_LENGTH_LT: 5 } + AppearedIn: { role: { shortestLength: { lt: 3 } } } + ActedIn: { role: { averageLength: { lt: 5 } } } } } } @@ -132,7 +132,7 @@ describe("aggregations-where-edge-string interface relationships of interface ty const query = /* GraphQL */ ` query People { ${Person.plural}( - where: { productionsAggregate: { edge: { ActedIn: { role_AVERAGE_LENGTH_LT: 5 } }, count_LT: 3 } } + where: { productionsAggregate: { edge: { ActedIn: { role: { averageLength: { lt:5 } } } }, count_LT: 3 } } ) { name } diff --git a/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts b/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts index a1adb49279..309e47d6c7 100644 --- a/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts +++ b/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts @@ -70,7 +70,7 @@ describe("Cypher Aggregations where edge with String", () => { test("should count number of interface relationships", async () => { const query = /* GraphQL */ ` query ActorsAggregate { - actors(where: { productionsAggregate: { count_LT: 3 } }) { + actors(where: { productionsAggregate: { count: { lt: 3 } } }) { name } } @@ -107,7 +107,10 @@ describe("Cypher Aggregations where edge with String", () => { people( where: { productionsAggregate: { - edge: { AppearedIn: { role_SHORTEST_LENGTH_LT: 3 }, ActedIn: { role_AVERAGE_LENGTH_LT: 5 } } + edge: { + AppearedIn: { role: { shortestLength: { lt: 3 } } } + ActedIn: { role: { averageLength: { lt: 5 } } } + } } } ) { @@ -163,7 +166,12 @@ describe("Cypher Aggregations where edge with String", () => { const query = /* GraphQL */ ` query People { people( - where: { productionsAggregate: { edge: { ActedIn: { role_AVERAGE_LENGTH_LT: 5 } }, count_LTE: 10 } } + where: { + productionsAggregate: { + edge: { ActedIn: { role: { averageLength: { lt: 5 } } } } + count: { lte: 10 } + } + } ) { name } From 3ee0ffc020f67b6678cd06df47ad46852defe69b Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Mon, 27 Jan 2025 14:49:46 +0000 Subject: [PATCH 03/13] Authorization in aggregation filters --- .../filters/aggregation/AggregationFilter.ts | 42 +++++--- .../queryAST/factory/FilterFactory.ts | 31 +----- .../tests/tck/advanced-filtering.test.ts | 32 +++--- .../directives/node/node-with-auth.test.ts | 4 +- .../graphql/tests/tck/issues/2396.test.ts | 98 +++++++++---------- packages/graphql/tests/tck/issues/505.test.ts | 42 +++++--- .../graphql/tests/tck/issues/5534.test.ts | 25 ++--- 7 files changed, 141 insertions(+), 133 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts index 5ac0cfbb7c..485e49540e 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts @@ -36,7 +36,7 @@ export class AggregationFilter extends Filter { private filters: Array = []; - private authFilters: AuthorizationFilters[] = []; + private authFilters: Record = {}; private subqueryReturnVariable: Cypher.Variable | undefined; @@ -49,8 +49,8 @@ export class AggregationFilter extends Filter { this.filters.push(...filter); } - public addAuthFilters(...filter: AuthorizationFilters[]) { - this.authFilters.push(...filter); + public addAuthFilters(name: string, ...filter: AuthorizationFilters[]) { + this.authFilters[name] = filter; } public getChildren(): QueryASTNode[] { @@ -65,13 +65,19 @@ export class AggregationFilter extends Filter { const relatedNode: Cypher.Node = new Cypher.Node(); let relatedNodeLabels: string[] = []; let labelsFilter: Cypher.Predicate | undefined; + let concreteAuthFilter: Cypher.Predicate | undefined; if (relatedEntity instanceof InterfaceEntityAdapter) { - const labelsForImplementations = relatedEntity.concreteEntities.map((e) => - relatedNode.hasLabels(...e.getLabels()) - ); + const labelsForImplementations = relatedEntity.concreteEntities.map((e) => { + const authFilterPredicate = this.getAuthFilterPredicate(e.name, context); + return Cypher.and(relatedNode.hasLabels(...e.getLabels()), ...authFilterPredicate); + }); labelsFilter = Cypher.or(...labelsForImplementations); } else { + const authFilterPredicate = this.getAuthFilterPredicate(relatedEntity.name, context); + if (authFilterPredicate.length) { + concreteAuthFilter = Cypher.and(...authFilterPredicate); + } relatedNodeLabels = getEntityLabels(relatedEntity, context.neo4jGraphQLContext); } const relationshipTarget = new Cypher.Relationship(); @@ -98,11 +104,12 @@ export class AggregationFilter extends Filter { if (returnColumns.length === 0) return []; // Maybe throw? - const subquery = labelsFilter - ? new Cypher.Match(pattern).where(labelsFilter).return(...returnColumns) - : new Cypher.Match(pattern).return(...returnColumns); - - return [subquery]; + if (labelsFilter) { + return [new Cypher.Match(pattern).where(labelsFilter).return(...returnColumns)]; + } else if (concreteAuthFilter) { + return [new Cypher.Match(pattern).where(concreteAuthFilter).return(...returnColumns)]; + } + return [new Cypher.Match(pattern).return(...returnColumns)]; } public getPredicate(_queryASTContext: QueryASTContext): Cypher.Predicate | undefined { @@ -111,11 +118,14 @@ export class AggregationFilter extends Filter { return Cypher.eq(this.subqueryReturnVariable, Cypher.true); } - private getAuthFilterSubqueries(context: QueryASTContext): Cypher.Clause[] { - return this.authFilters.flatMap((f) => f.getSubqueries(context)); - } + // private getAuthFilterSubqueries(context: QueryASTContext): Cypher.Clause[] { + // return this.authFilters.flatMap((f) => f.getSubqueries(context)); + // } + + private getAuthFilterPredicate(name: string, context: QueryASTContext): Cypher.Predicate[] { + const authFilters = this.authFilters[name]; + if (!authFilters) return []; - private getAuthFilterPredicate(context: QueryASTContext): Cypher.Predicate[] { - return filterTruthy(this.authFilters.map((f) => f.getPredicate(context))); + return filterTruthy(authFilters.map((f) => f.getPredicate(context))); } } diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index 54a3496acd..67466bc9a6 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -946,45 +946,18 @@ export class FilterFactory { where: AggregateWhereInput, context: Neo4jGraphQLTranslationContext ): AggregationFilter { + const aggregationFilter = new AggregationFilter(relationship); const filteredEntities = getConcreteEntities(relationship.target, where); - const relationshipFilters: RelationshipFilter[] = []; for (const concreteEntity of filteredEntities) { - const relationshipFilter = this.createRelationshipFilterTreeNode({ - relationship, - target: concreteEntity, - operator: operator ?? "SOME", - }); - const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ entity: concreteEntity, operations: ["READ"], - // attributes: this.getSelectedAttributes(entity, projectionFields), context, }); - - relationshipFilter.addAuthFilters(...authFilters); - - if (!isNull) { - const entityWhere = where[concreteEntity.name] ?? where; - const targetNodeFilters = this.createNodeFilters(concreteEntity, entityWhere, context); - relationshipFilter.addTargetNodeFilter(...targetNodeFilters); - } - - relationshipFilters.push(relationshipFilter); + aggregationFilter.addAuthFilters(concreteEntity.name, ...authFilters); } - const aggregationFilter = new AggregationFilter(relationship); const nestedFilters = this.getAggregationNestedFilters(where, relationship); - - const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ - entity: concreteEntity, - operations: ["READ"], - // attributes: this.getSelectedAttributes(entity, projectionFields), - context, - }); - - aggregationFilter.addAuthFilters(...authFilters); - aggregationFilter.addFilters(...nestedFilters); return aggregationFilter; diff --git a/packages/graphql/tests/tck/advanced-filtering.test.ts b/packages/graphql/tests/tck/advanced-filtering.test.ts index 10fc5fcdb6..8a896aa5cf 100644 --- a/packages/graphql/tests/tck/advanced-filtering.test.ts +++ b/packages/graphql/tests/tck/advanced-filtering.test.ts @@ -554,14 +554,16 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE EXISTS { MATCH (this)-[:IN_GENRE]->(this0:Genre) - WHERE this0.name = $param0 + WHERE (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) } RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"isAuthenticated\\": false, + \\"param1\\": \\"some genre\\", + \\"param2\\": \\"some genre\\" }" `); }); @@ -586,16 +588,18 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE (EXISTS { MATCH (this)-[:IN_GENRE]->(this0:Genre) - WHERE this0.name = $param0 + WHERE (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) } AND NOT (EXISTS { MATCH (this)-[:IN_GENRE]->(this0:Genre) - WHERE NOT (this0.name = $param0) + WHERE NOT (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) })) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"isAuthenticated\\": false, + \\"param1\\": \\"some genre\\", + \\"param2\\": \\"some genre\\" }" `); }); @@ -609,13 +613,15 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE NOT (EXISTS { MATCH (this)-[:IN_GENRE]->(this0:Genre) - WHERE this0.name = $param0 + WHERE (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) }) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"isAuthenticated\\": false, + \\"param1\\": \\"some genre\\", + \\"param2\\": \\"some genre\\" }" `); }); @@ -627,12 +633,14 @@ describe("Cypher Advanced Filtering", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "MATCH (this:Movie) - WHERE single(this0 IN [(this)-[:IN_GENRE]->(this0:Genre) WHERE this0.name = $param0 | 1] WHERE true) + WHERE single(this0 IN [(this)-[:IN_GENRE]->(this0:Genre) WHERE (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) | 1] WHERE true) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"isAuthenticated\\": false, + \\"param1\\": \\"some genre\\", + \\"param2\\": \\"some genre\\" }" `); }); @@ -645,13 +653,15 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE EXISTS { MATCH (this)-[:IN_GENRE]->(this0:Genre) - WHERE this0.name = $param0 + WHERE (($isAuthenticated = true AND ($param1 IS NOT NULL AND this0.name = $param1)) AND this0.name = $param2) } RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"isAuthenticated\\": false, + \\"param1\\": \\"some genre\\", + \\"param2\\": \\"some genre\\" }" `); }); diff --git a/packages/graphql/tests/tck/directives/node/node-with-auth.test.ts b/packages/graphql/tests/tck/directives/node/node-with-auth.test.ts index 7c3d45060f..62cbf381ae 100644 --- a/packages/graphql/tests/tck/directives/node/node-with-auth.test.ts +++ b/packages/graphql/tests/tck/directives/node/node-with-auth.test.ts @@ -115,14 +115,13 @@ describe("Node Directive", () => { "MATCH (this:Comment) WHERE (EXISTS { MATCH (this)<-[:HAS_POST]-(this0:Person) - WHERE this0.id = $param0 + WHERE (apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.sub IS NOT NULL AND this0.id = $jwt.sub)), \\"@neo4j/graphql/FORBIDDEN\\", [0]) AND this0.id = $param2) } AND apoc.util.validatePredicate(NOT ($isAuthenticated = true AND ($jwt.roles IS NOT NULL AND $param3 IN $jwt.roles)), \\"@neo4j/graphql/FORBIDDEN\\", [0])) DETACH DELETE this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"123\\", \\"isAuthenticated\\": true, \\"jwt\\": { \\"roles\\": [ @@ -130,6 +129,7 @@ describe("Node Directive", () => { ], \\"sub\\": \\"id-01\\" }, + \\"param2\\": \\"123\\", \\"param3\\": \\"admin\\" }" `); diff --git a/packages/graphql/tests/tck/issues/2396.test.ts b/packages/graphql/tests/tck/issues/2396.test.ts index aa9db71aad..5bc08b61ad 100644 --- a/packages/graphql/tests/tck/issues/2396.test.ts +++ b/packages/graphql/tests/tck/issues/2396.test.ts @@ -160,10 +160,10 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { WITH * WHERE (EXISTS { MATCH (this)-[:HAS_VALUATION]->(this0:Valuation) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this0.archivedAt IS NULL) AND EXISTS { MATCH (this0)-[:VALUATION_FOR]->(this1:Estate) - WHERE this1.floor >= $param0 - } + WHERE (($isAuthenticated = true AND this1.archivedAt IS NULL) AND this1.floor >= $param1) + }) } AND ($isAuthenticated = true AND this.archivedAt IS NULL)) CALL { WITH this @@ -188,11 +188,11 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": { + \\"isAuthenticated\\": true, + \\"param1\\": { \\"low\\": 0, \\"high\\": 0 - }, - \\"isAuthenticated\\": true + } }" `); }); @@ -238,10 +238,10 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { WITH * WHERE ((this.price >= $param0 AND EXISTS { MATCH (this)-[:HAS_VALUATION]->(this0:Valuation) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this0.archivedAt IS NULL) AND EXISTS { MATCH (this0)-[:VALUATION_FOR]->(this1:Estate) - WHERE this1.floor >= $param1 - } + WHERE (($isAuthenticated = true AND this1.archivedAt IS NULL) AND this1.floor >= $param2) + }) }) AND ($isAuthenticated = true AND this.archivedAt IS NULL)) CALL { WITH this @@ -267,11 +267,11 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ \\"param0\\": 0, - \\"param1\\": { + \\"isAuthenticated\\": true, + \\"param2\\": { \\"low\\": 0, \\"high\\": 0 - }, - \\"isAuthenticated\\": true + } }" `); }); @@ -328,16 +328,16 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { WITH * WHERE ((this.price >= $param0 AND EXISTS { MATCH (this)-[:HAS_VALUATION]->(this0:Valuation) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this0.archivedAt IS NULL) AND EXISTS { MATCH (this0)-[:VALUATION_FOR]->(this1:Estate) - WHERE (this1.estateType IN $param1 AND this1.area >= $param2 AND this1.floor >= $param3 AND EXISTS { + WHERE (($isAuthenticated = true AND this1.archivedAt IS NULL) AND (this1.estateType IN $param2 AND this1.area >= $param3 AND this1.floor >= $param4 AND EXISTS { MATCH (this1)-[:HAS_ADDRESS]->(this2:Address) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this2.archivedAt IS NULL) AND EXISTS { MATCH (this2)-[:HAS_POSTAL_CODE]->(this3:PostalCode) - WHERE this3.number IN $param4 - } - }) - } + WHERE (($isAuthenticated = true AND this3.archivedAt IS NULL) AND this3.number IN $param5) + }) + })) + }) }) AND ($isAuthenticated = true AND this.archivedAt IS NULL)) CALL { WITH this @@ -363,18 +363,18 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ \\"param0\\": 0, - \\"param1\\": [ + \\"isAuthenticated\\": true, + \\"param2\\": [ \\"APARTMENT\\" ], - \\"param2\\": 0, - \\"param3\\": { + \\"param3\\": 0, + \\"param4\\": { \\"low\\": 0, \\"high\\": 0 }, - \\"param4\\": [ + \\"param5\\": [ \\"13001\\" - ], - \\"isAuthenticated\\": true + ] }" `); }); @@ -431,16 +431,16 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { WITH * WHERE ((this.price >= $param0 AND EXISTS { MATCH (this)-[:HAS_VALUATION]->(this0:Valuation) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this0.archivedAt IS NULL) AND EXISTS { MATCH (this0)-[:VALUATION_FOR]->(this1:Estate) - WHERE (this1.estateType IN $param1 AND this1.area >= $param2 AND this1.floor >= $param3 AND EXISTS { + WHERE (($isAuthenticated = true AND this1.archivedAt IS NULL) AND (this1.estateType IN $param2 AND this1.area >= $param3 AND this1.floor >= $param4 AND EXISTS { MATCH (this1)-[:HAS_ADDRESS]->(this2:Address) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this2.archivedAt IS NULL) AND EXISTS { MATCH (this2)-[:HAS_POSTAL_CODE]->(this3:PostalCode) - WHERE this3.number IN $param4 - } - }) - } + WHERE (($isAuthenticated = true AND this3.archivedAt IS NULL) AND this3.number IN $param5) + }) + })) + }) }) AND ($isAuthenticated = true AND this.archivedAt IS NULL)) WITH * SKIP $param6 @@ -469,18 +469,18 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ \\"param0\\": 0, - \\"param1\\": [ + \\"isAuthenticated\\": true, + \\"param2\\": [ \\"APARTMENT\\" ], - \\"param2\\": 0, - \\"param3\\": { + \\"param3\\": 0, + \\"param4\\": { \\"low\\": 0, \\"high\\": 0 }, - \\"param4\\": [ + \\"param5\\": [ \\"13001\\" ], - \\"isAuthenticated\\": true, \\"param6\\": { \\"low\\": 0, \\"high\\": 0 @@ -545,16 +545,16 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { WITH * WHERE ((this.price >= $param0 AND EXISTS { MATCH (this)-[:HAS_VALUATION]->(this0:Valuation) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this0.archivedAt IS NULL) AND EXISTS { MATCH (this0)-[:VALUATION_FOR]->(this1:Estate) - WHERE (this1.estateType IN $param1 AND this1.area >= $param2 AND this1.floor >= $param3 AND EXISTS { + WHERE (($isAuthenticated = true AND this1.archivedAt IS NULL) AND (this1.estateType IN $param2 AND this1.area >= $param3 AND this1.floor >= $param4 AND EXISTS { MATCH (this1)-[:HAS_ADDRESS]->(this2:Address) - WHERE EXISTS { + WHERE (($isAuthenticated = true AND this2.archivedAt IS NULL) AND EXISTS { MATCH (this2)-[:HAS_POSTAL_CODE]->(this3:PostalCode) - WHERE this3.number IN $param4 - } - }) - } + WHERE (($isAuthenticated = true AND this3.archivedAt IS NULL) AND this3.number IN $param5) + }) + })) + }) }) AND ($isAuthenticated = true AND this.archivedAt IS NULL)) WITH * SKIP $param6 @@ -583,18 +583,18 @@ describe("https://github.com/neo4j/graphql/issues/2396", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ \\"param0\\": 0, - \\"param1\\": [ + \\"isAuthenticated\\": true, + \\"param2\\": [ \\"APARTMENT\\" ], - \\"param2\\": 0, - \\"param3\\": { + \\"param3\\": 0, + \\"param4\\": { \\"low\\": 0, \\"high\\": 0 }, - \\"param4\\": [ + \\"param5\\": [ \\"13001\\" ], - \\"isAuthenticated\\": true, \\"param6\\": { \\"low\\": 20, \\"high\\": 0 diff --git a/packages/graphql/tests/tck/issues/505.test.ts b/packages/graphql/tests/tck/issues/505.test.ts index bb82a31b02..f2db9e4f2e 100644 --- a/packages/graphql/tests/tck/issues/505.test.ts +++ b/packages/graphql/tests/tck/issues/505.test.ts @@ -248,30 +248,42 @@ describe("https://github.com/neo4j/graphql/issues/505", () => { WITH * WHERE ((EXISTS { MATCH (this)<-[:HAS_PAGE]-(this0:Workspace) - WHERE this0.id = $param0 + WHERE (($isAuthenticated = true AND (EXISTS { + MATCH (this0)<-[:MEMBER_OF]-(this1:User) + WHERE ($jwt.sub IS NOT NULL AND this1.authId = $jwt.sub) + } OR EXISTS { + MATCH (this0)-[:HAS_ADMIN]->(this2:User) + WHERE ($jwt.sub IS NOT NULL AND this2.authId = $jwt.sub) + })) AND this0.id = $param2) } AND NOT (EXISTS { MATCH (this)<-[:HAS_PAGE]-(this0:Workspace) - WHERE NOT (this0.id = $param0) + WHERE NOT (($isAuthenticated = true AND (EXISTS { + MATCH (this0)<-[:MEMBER_OF]-(this1:User) + WHERE ($jwt.sub IS NOT NULL AND this1.authId = $jwt.sub) + } OR EXISTS { + MATCH (this0)-[:HAS_ADMIN]->(this2:User) + WHERE ($jwt.sub IS NOT NULL AND this2.authId = $jwt.sub) + })) AND this0.id = $param2) })) AND ($isAuthenticated = true AND (EXISTS { - MATCH (this)<-[:CREATED_PAGE]-(this1:User) - WHERE ($jwt.sub IS NOT NULL AND this1.authId = $jwt.sub) + MATCH (this)<-[:CREATED_PAGE]-(this3:User) + WHERE ($jwt.sub IS NOT NULL AND this3.authId = $jwt.sub) } OR (($param3 IS NOT NULL AND this.shared = $param3) AND (EXISTS { - MATCH (this)<-[:HAS_PAGE]-(this2:Workspace) + MATCH (this)<-[:HAS_PAGE]-(this4:Workspace) WHERE (EXISTS { - MATCH (this2)<-[:MEMBER_OF]-(this3:User) - WHERE ($jwt.sub IS NOT NULL AND this3.authId = $jwt.sub) + MATCH (this4)<-[:MEMBER_OF]-(this5:User) + WHERE ($jwt.sub IS NOT NULL AND this5.authId = $jwt.sub) } OR EXISTS { - MATCH (this2)-[:HAS_ADMIN]->(this4:User) - WHERE ($jwt.sub IS NOT NULL AND this4.authId = $jwt.sub) + MATCH (this4)-[:HAS_ADMIN]->(this6:User) + WHERE ($jwt.sub IS NOT NULL AND this6.authId = $jwt.sub) }) } AND NOT (EXISTS { - MATCH (this)<-[:HAS_PAGE]-(this2:Workspace) + MATCH (this)<-[:HAS_PAGE]-(this4:Workspace) WHERE NOT (EXISTS { - MATCH (this2)<-[:MEMBER_OF]-(this3:User) - WHERE ($jwt.sub IS NOT NULL AND this3.authId = $jwt.sub) + MATCH (this4)<-[:MEMBER_OF]-(this5:User) + WHERE ($jwt.sub IS NOT NULL AND this5.authId = $jwt.sub) } OR EXISTS { - MATCH (this2)-[:HAS_ADMIN]->(this4:User) - WHERE ($jwt.sub IS NOT NULL AND this4.authId = $jwt.sub) + MATCH (this4)-[:HAS_ADMIN]->(this6:User) + WHERE ($jwt.sub IS NOT NULL AND this6.authId = $jwt.sub) }) })))))) RETURN this { .id } AS this" @@ -279,9 +291,9 @@ describe("https://github.com/neo4j/graphql/issues/505", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"my-workspace-id\\", \\"isAuthenticated\\": false, \\"jwt\\": {}, + \\"param2\\": \\"my-workspace-id\\", \\"param3\\": true }" `); diff --git a/packages/graphql/tests/tck/issues/5534.test.ts b/packages/graphql/tests/tck/issues/5534.test.ts index a8d81d8228..6ce28b7f95 100644 --- a/packages/graphql/tests/tck/issues/5534.test.ts +++ b/packages/graphql/tests/tck/issues/5534.test.ts @@ -77,17 +77,18 @@ describe("https://github.com/neo4j/graphql/issues/5534", () => { "MATCH (this:Product) CALL { WITH this - MATCH (this)<-[this0:PRODUCT_HAS_FAMILY_PRODUCT]-(this1:Product|Banana) - RETURN count(this1) = $param0 AS var2 + MATCH (this)<-[this0:PRODUCT_HAS_FAMILY_PRODUCT]-(this1:Product) + WHERE (($param0 IS NOT NULL AND this.isPublic = $param0) AND ($param1 IS NOT NULL AND this.isEmpty = $param1)) + RETURN count(this1) = $param2 AS var2 } WITH * - WHERE (var2 = true AND (($param1 IS NOT NULL AND this.isPublic = $param1) AND ($param2 IS NOT NULL AND this.isEmpty = $param2))) + WHERE (var2 = true AND (($param3 IS NOT NULL AND this.isPublic = $param3) AND ($param4 IS NOT NULL AND this.isEmpty = $param4))) WITH * - LIMIT $param3 + LIMIT $param5 CALL { WITH this MATCH (this)<-[this3:PRODUCT_HAS_FAMILY_PRODUCT]-(this4:Product) - WHERE (($param4 IS NOT NULL AND this4.isPublic = $param4) AND ($param5 IS NOT NULL AND this4.isEmpty = $param5)) + WHERE (($param6 IS NOT NULL AND this4.isPublic = $param6) AND ($param7 IS NOT NULL AND this4.isEmpty = $param7)) RETURN count(this4) AS var5 } RETURN this { .productId, variantsAggregate: { count: var5 } } AS this" @@ -95,18 +96,20 @@ describe("https://github.com/neo4j/graphql/issues/5534", () => { expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": { + \\"param0\\": true, + \\"param1\\": true, + \\"param2\\": { \\"low\\": 1, \\"high\\": 0 }, - \\"param1\\": true, - \\"param2\\": true, - \\"param3\\": { + \\"param3\\": true, + \\"param4\\": true, + \\"param5\\": { \\"low\\": 1, \\"high\\": 0 }, - \\"param4\\": true, - \\"param5\\": true + \\"param6\\": true, + \\"param7\\": true }" `); }); From 18baef57c0daae3417dc10f776766742f6af1f40 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Mon, 27 Jan 2025 16:53:14 +0000 Subject: [PATCH 04/13] Add authorization to connection filters --- .../queryAST/ast/filters/ConnectionFilter.ts | 26 +++++++++++-- .../AuthConnectionFilter.ts | 2 +- .../queryAST/factory/FilterFactory.ts | 15 +++++++- .../tests/tck/advanced-filtering.test.ts | 38 ++++++++++++------- 4 files changed, 63 insertions(+), 18 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts index 135336c333..dfd7cf950b 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts @@ -21,6 +21,7 @@ import Cypher from "@neo4j/cypher-builder"; import type { ConcreteEntityAdapter } from "../../../../schema-model/entity/model-adapters/ConcreteEntityAdapter"; import type { InterfaceEntityAdapter } from "../../../../schema-model/entity/model-adapters/InterfaceEntityAdapter"; import type { RelationshipAdapter } from "../../../../schema-model/relationship/model-adapters/RelationshipAdapter"; +import { filterTruthy } from "../../../../utils/utils"; import { hasTarget } from "../../utils/context-has-target"; import { getEntityLabels } from "../../utils/create-node-from-entity"; import { isConcreteEntity } from "../../utils/is-concrete-entity"; @@ -29,6 +30,7 @@ import type { QueryASTContext } from "../QueryASTContext"; import type { QueryASTNode } from "../QueryASTNode"; import type { RelationshipWhereOperator } from "./Filter"; import { Filter } from "./Filter"; +import type { AuthorizationFilters } from "./authorization-filters/AuthorizationFilters"; export class ConnectionFilter extends Filter { protected innerFilters: Filter[] = []; @@ -39,6 +41,8 @@ export class ConnectionFilter extends Filter { // as subqueries and store them protected subqueryPredicate: Cypher.Predicate | undefined; + private authFilters: Record = {}; + constructor({ relationship, target, @@ -58,6 +62,10 @@ export class ConnectionFilter extends Filter { this.innerFilters.push(...filters); } + public addAuthFilters(name: string, ...filter: AuthorizationFilters[]) { + this.authFilters[name] = filter; + } + public getChildren(): QueryASTNode[] { return [...this.innerFilters]; } @@ -132,15 +140,20 @@ export class ConnectionFilter extends Filter { * } * RETURN this { .name } AS this **/ - protected getLabelPredicate(context: QueryASTContext): Cypher.Predicate | undefined { + protected getLabelAndAuthorizationPredicate(context: QueryASTContext): Cypher.Predicate | undefined { if (!hasTarget(context)) { throw new Error("No parent node found!"); } if (isConcreteEntity(this.target)) { + const authFilterPredicate = this.getAuthFilterPredicate(this.target.name, context); + if (authFilterPredicate.length) { + return Cypher.and(...authFilterPredicate); + } return; } const labelPredicate = this.target.concreteEntities.map((e) => { - return context.target.hasLabels(...e.labels); + const authFilterPredicate = this.getAuthFilterPredicate(e.name, context); + return Cypher.and(context.target.hasLabels(...e.getLabels()), ...authFilterPredicate); }); return Cypher.or(...labelPredicate); } @@ -150,7 +163,7 @@ export class ConnectionFilter extends Filter { queryASTContext: QueryASTContext ): Cypher.Predicate | undefined { const connectionFilter = this.innerFilters.map((c) => c.getPredicate(queryASTContext)); - const labelPredicate = this.getLabelPredicate(queryASTContext); + const labelPredicate = this.getLabelAndAuthorizationPredicate(queryASTContext); const innerPredicate = Cypher.and(...connectionFilter, labelPredicate); if (!innerPredicate) { @@ -291,4 +304,11 @@ export class ConnectionFilter extends Filter { return [Cypher.utils.concat(match, ...subqueries), Cypher.utils.concat(match2, ...subqueries2)]; } + + private getAuthFilterPredicate(name: string, context: QueryASTContext): Cypher.Predicate[] { + const authFilters = this.authFilters[name]; + if (!authFilters) return []; + + return filterTruthy(authFilters.map((f) => f.getPredicate(context))); + } } diff --git a/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthConnectionFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthConnectionFilter.ts index a40e967ed7..8cd67d4145 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthConnectionFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/authorization-filters/AuthConnectionFilter.ts @@ -27,7 +27,7 @@ export class AuthConnectionFilter extends ConnectionFilter { queryASTContext: QueryASTContext ): Cypher.Predicate | undefined { const connectionFilter = this.innerFilters.map((c) => c.getPredicate(queryASTContext)); - const labelPredicate = this.getLabelPredicate(queryASTContext); + const labelPredicate = this.getLabelAndAuthorizationPredicate(queryASTContext); const innerPredicate = Cypher.and(...connectionFilter, labelPredicate); const useExist = queryASTContext.neo4jGraphQLContext.neo4jDatabaseInfo?.gte("5.0"); diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index 67466bc9a6..a48e82d445 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -93,6 +93,7 @@ export class FilterFactory { relationship: relationship, target: relationship.target, operator, + context, }); const filters = this.createConnectionPredicates({ rel: relationship, @@ -116,6 +117,7 @@ export class FilterFactory { relationship: relationship, target: concreteEntity, operator, + context, }); const filters = this.createConnectionPredicates({ @@ -458,8 +460,19 @@ export class FilterFactory { relationship: RelationshipAdapter; target: ConcreteEntityAdapter | InterfaceEntityAdapter; operator: RelationshipWhereOperator; + context: Neo4jGraphQLTranslationContext; }): ConnectionFilter { - return new ConnectionFilter(options); + const connectionFilter = new ConnectionFilter(options); + const filteredEntities = getConcreteEntities(options.relationship.target); + for (const concreteEntity of filteredEntities) { + const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ + entity: concreteEntity, + operations: ["READ"], + context: options.context, + }); + connectionFilter.addAuthFilters(concreteEntity.name, ...authFilters); + } + return connectionFilter; } public createInterfaceNodeFilters({ diff --git a/packages/graphql/tests/tck/advanced-filtering.test.ts b/packages/graphql/tests/tck/advanced-filtering.test.ts index 8a896aa5cf..acba11b84b 100644 --- a/packages/graphql/tests/tck/advanced-filtering.test.ts +++ b/packages/graphql/tests/tck/advanced-filtering.test.ts @@ -684,14 +684,16 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE this1.name = $param0 + WHERE (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) } RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); @@ -711,14 +713,16 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE NOT (EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE this1.name = $param0 + WHERE (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) }) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); @@ -743,16 +747,18 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE (EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE this1.name = $param0 + WHERE (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) } AND NOT (EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE NOT (this1.name = $param0) + WHERE NOT (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) })) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); @@ -765,13 +771,15 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE NOT (EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE this1.name = $param0 + WHERE (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) }) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); @@ -782,12 +790,14 @@ describe("Cypher Advanced Filtering", () => { expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` "MATCH (this:Movie) - WHERE single(this0 IN [(this)-[this1:IN_GENRE]->(this0:Genre) WHERE this0.name = $param0 | 1] WHERE true) + WHERE single(this0 IN [(this)-[this1:IN_GENRE]->(this0:Genre) WHERE (this0.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this0.name = $param2))) | 1] WHERE true) RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); @@ -800,13 +810,15 @@ describe("Cypher Advanced Filtering", () => { "MATCH (this:Movie) WHERE EXISTS { MATCH (this)-[this0:IN_GENRE]->(this1:Genre) - WHERE this1.name = $param0 + WHERE (this1.name = $param0 AND ($isAuthenticated = true AND ($param2 IS NOT NULL AND this1.name = $param2))) } RETURN this { .actorCount } AS this" `); expect(formatParams(result.params)).toMatchInlineSnapshot(` "{ - \\"param0\\": \\"some genre\\" + \\"param0\\": \\"some genre\\", + \\"isAuthenticated\\": false, + \\"param2\\": \\"some genre\\" }" `); }); From abed4b9e5aabc605889e757c8d1b5c98fbb6545a Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 6 Feb 2025 17:22:56 +0000 Subject: [PATCH 05/13] Don't generate authorization from within authorization --- .../queryAST/factory/AuthFilterFactory.ts | 8 +++++ .../queryAST/factory/FilterFactory.ts | 34 +++++++++---------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts index 7be3725408..10a95d2382 100644 --- a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts @@ -36,6 +36,7 @@ import { LogicalFilter } from "../ast/filters/LogicalFilter"; import type { RelationshipFilter } from "../ast/filters/RelationshipFilter"; import { AuthConnectionFilter } from "../ast/filters/authorization-filters/AuthConnectionFilter"; import { AuthRelationshipFilter } from "../ast/filters/authorization-filters/AuthRelationshipFilter"; +import type { AuthorizationFilters } from "../ast/filters/authorization-filters/AuthorizationFilters"; import { JWTFilter } from "../ast/filters/authorization-filters/JWTFilter"; import { CypherFilter } from "../ast/filters/property-filters/CypherFilter"; import { ParamPropertyFilter } from "../ast/filters/property-filters/ParamPropertyFilter"; @@ -282,4 +283,11 @@ export class AuthFilterFactory extends FilterFactory { }): ConnectionFilter { return new AuthConnectionFilter(options); } + + protected getAuthFilters( + _entity: ConcreteEntityAdapter, + _context: Neo4jGraphQLTranslationContext + ): AuthorizationFilters[] { + return []; + } } diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index a48e82d445..9fb99d11f7 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -42,6 +42,7 @@ import { AggregationFilter } from "../ast/filters/aggregation/AggregationFilter" import { AggregationPropertyFilter } from "../ast/filters/aggregation/AggregationPropertyFilter"; import { AggregationTimeFilter } from "../ast/filters/aggregation/AggregationTimePropertyFilter"; import { CountFilter } from "../ast/filters/aggregation/CountFilter"; +import type { AuthorizationFilters } from "../ast/filters/authorization-filters/AuthorizationFilters"; import { CypherFilter } from "../ast/filters/property-filters/CypherFilter"; import { DateTimeFilter } from "../ast/filters/property-filters/DateTimeFilter"; import { DurationFilter } from "../ast/filters/property-filters/DurationFilter"; @@ -74,7 +75,8 @@ type AggregateWhereInput = { }; export class FilterFactory { - private queryASTFactory: QueryASTFactory; + protected queryASTFactory: QueryASTFactory; + constructor(queryASTFactory: QueryASTFactory) { this.queryASTFactory = queryASTFactory; } @@ -349,12 +351,7 @@ export class FilterFactory { operator: operator ?? "SOME", }); - const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ - entity: concreteEntity, - operations: ["READ"], - // attributes: this.getSelectedAttributes(entity, projectionFields), - context, - }); + const authFilters = this.getAuthFilters(concreteEntity, context); relationshipFilter.addAuthFilters(...authFilters); @@ -465,11 +462,7 @@ export class FilterFactory { const connectionFilter = new ConnectionFilter(options); const filteredEntities = getConcreteEntities(options.relationship.target); for (const concreteEntity of filteredEntities) { - const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ - entity: concreteEntity, - operations: ["READ"], - context: options.context, - }); + const authFilters = this.getAuthFilters(concreteEntity, options.context); connectionFilter.addAuthFilters(concreteEntity.name, ...authFilters); } return connectionFilter; @@ -754,6 +747,17 @@ export class FilterFactory { throw new Error(`Invalid operator ${operator}`); } + protected getAuthFilters( + entity: ConcreteEntityAdapter, + context: Neo4jGraphQLTranslationContext + ): AuthorizationFilters[] { + return this.queryASTFactory.authorizationFactory.getAuthFilters({ + entity, + operations: ["READ"], + context, + }); + } + private createRelatedNodeFilters({ relationship, value, @@ -962,11 +966,7 @@ export class FilterFactory { const aggregationFilter = new AggregationFilter(relationship); const filteredEntities = getConcreteEntities(relationship.target, where); for (const concreteEntity of filteredEntities) { - const authFilters = this.queryASTFactory.authorizationFactory.getAuthFilters({ - entity: concreteEntity, - operations: ["READ"], - context, - }); + const authFilters = this.getAuthFilters(concreteEntity, context); aggregationFilter.addAuthFilters(concreteEntity.name, ...authFilters); } From 03570530a3150bf004cde4c85e01674cc9d7b435 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 6 Feb 2025 17:23:03 +0000 Subject: [PATCH 06/13] Add template changeset --- .changeset/strong-cameras-heal.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/strong-cameras-heal.md diff --git a/.changeset/strong-cameras-heal.md b/.changeset/strong-cameras-heal.md new file mode 100644 index 0000000000..46a9b6b7e3 --- /dev/null +++ b/.changeset/strong-cameras-heal.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graphql": major +--- + +Authorization for filters From 3de579da227a3459ac02be7e566144ab5e49cd0f Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 6 Feb 2025 18:17:03 +0000 Subject: [PATCH 07/13] Add authorization for filtered fields --- .../queryAST/factory/AuthFilterFactory.ts | 1 + .../queryAST/factory/FilterFactory.ts | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts index 10a95d2382..2dbfc1ef80 100644 --- a/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/AuthFilterFactory.ts @@ -286,6 +286,7 @@ export class AuthFilterFactory extends FilterFactory { protected getAuthFilters( _entity: ConcreteEntityAdapter, + _attributes: AttributeAdapter[], _context: Neo4jGraphQLTranslationContext ): AuthorizationFilters[] { return []; diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index 9fb99d11f7..e6bd706f5a 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -96,6 +96,7 @@ export class FilterFactory { target: relationship.target, operator, context, + where, }); const filters = this.createConnectionPredicates({ rel: relationship, @@ -120,6 +121,7 @@ export class FilterFactory { target: concreteEntity, operator, context, + where, }); const filters = this.createConnectionPredicates({ @@ -351,7 +353,11 @@ export class FilterFactory { operator: operator ?? "SOME", }); - const authFilters = this.getAuthFilters(concreteEntity, context); + const attributes = filterTruthy( + Object.keys(where[concreteEntity.name] ?? where).map((key) => concreteEntity.findAttribute(key)) + ); + + const authFilters = this.getAuthFilters(concreteEntity, attributes, context); relationshipFilter.addAuthFilters(...authFilters); @@ -458,11 +464,16 @@ export class FilterFactory { target: ConcreteEntityAdapter | InterfaceEntityAdapter; operator: RelationshipWhereOperator; context: Neo4jGraphQLTranslationContext; + where: ConnectionWhereArg; }): ConnectionFilter { const connectionFilter = new ConnectionFilter(options); const filteredEntities = getConcreteEntities(options.relationship.target); for (const concreteEntity of filteredEntities) { - const authFilters = this.getAuthFilters(concreteEntity, options.context); + const attributes = filterTruthy( + Object.keys(options.where.node || {}).map((key) => concreteEntity.findAttribute(key)) + ); + + const authFilters = this.getAuthFilters(concreteEntity, attributes, options.context); connectionFilter.addAuthFilters(concreteEntity.name, ...authFilters); } return connectionFilter; @@ -749,12 +760,14 @@ export class FilterFactory { protected getAuthFilters( entity: ConcreteEntityAdapter, + attributes: AttributeAdapter[] | undefined, context: Neo4jGraphQLTranslationContext ): AuthorizationFilters[] { return this.queryASTFactory.authorizationFactory.getAuthFilters({ entity, operations: ["READ"], context, + attributes, }); } @@ -966,7 +979,11 @@ export class FilterFactory { const aggregationFilter = new AggregationFilter(relationship); const filteredEntities = getConcreteEntities(relationship.target, where); for (const concreteEntity of filteredEntities) { - const authFilters = this.getAuthFilters(concreteEntity, context); + const attributes = filterTruthy( + Object.keys(where.node || {}).map((key) => concreteEntity.findAttribute(key)) + ); + + const authFilters = this.getAuthFilters(concreteEntity, attributes, context); aggregationFilter.addAuthFilters(concreteEntity.name, ...authFilters); } From 3872b2ffac460666a2b216d40508fd2795769cf9 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 6 Feb 2025 18:33:35 +0000 Subject: [PATCH 08/13] Remove unused code --- .../queryAST/ast/filters/aggregation/AggregationFilter.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts index 485e49540e..483c5161ae 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationFilter.ts @@ -118,10 +118,6 @@ export class AggregationFilter extends Filter { return Cypher.eq(this.subqueryReturnVariable, Cypher.true); } - // private getAuthFilterSubqueries(context: QueryASTContext): Cypher.Clause[] { - // return this.authFilters.flatMap((f) => f.getSubqueries(context)); - // } - private getAuthFilterPredicate(name: string, context: QueryASTContext): Cypher.Predicate[] { const authFilters = this.authFilters[name]; if (!authFilters) return []; From 3ae8d55e10bf31d119177b3e698b053a8299a211 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Mon, 10 Feb 2025 17:59:14 +0000 Subject: [PATCH 09/13] Add integration test for issue --- .../tests/integration/issues/5534.int.test.ts | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 packages/graphql/tests/integration/issues/5534.int.test.ts diff --git a/packages/graphql/tests/integration/issues/5534.int.test.ts b/packages/graphql/tests/integration/issues/5534.int.test.ts new file mode 100644 index 0000000000..e697df178c --- /dev/null +++ b/packages/graphql/tests/integration/issues/5534.int.test.ts @@ -0,0 +1,167 @@ +/* + * 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/5534", () => { + const testHelper = new TestHelper(); + + const secret = "secret"; + + let Product: UniqueType; + + beforeAll(async () => { + Product = testHelper.createUniqueType("Product"); + + const typeDefs = /* GraphQL */ ` + type ${Product} + @node + @mutation(operations: []) + @authorization( + filter: [ + { + requireAuthentication: false + operations: [READ, AGGREGATE] + where: { AND: [{ node: { isPublic: { eq: true } } }, { node: { isEmpty: { eq: false } } }] } + } + ] + ) + @subscription(events: []) { + """ + Unique Identifier of this product + """ + productId: String! + isEmpty: Boolean! @default(value: false) + isPublic: Boolean! @default(value: false) + """ + The product variants belonging to this product + """ + variants: [${Product}!]! + @relationship( + type: "PRODUCT_HAS_FAMILY_PRODUCT" + direction: IN + nestedOperations: [] + ) + @settable(onCreate: false, onUpdate: false) + } + `; + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + }); + + afterAll(async () => { + await testHelper.close(); + }); + + test("result with aggregate filter should match results from selection set", async () => { + await testHelper.executeCypher(` + CREATE (a:${Product} { productId: "A", isEmpty: false, isPublic: true }) + CREATE (b:${Product} { productId: "B", isEmpty: false, isPublic: true }) + CREATE (c:${Product} { productId: "C", isEmpty: true, isPublic: true }) + CREATE (a)-[:PRODUCT_HAS_FAMILY_PRODUCT]->(b) + CREATE (a)-[:PRODUCT_HAS_FAMILY_PRODUCT]->(c) + `); + + const token = createBearerToken(secret, { sub: "sub" }); + + const productsQuery = /* GraphQL */ ` + query { + ${Product.plural} { + productId + isEmpty + isPublic + } + } + `; + + const productsResponse = await testHelper.executeGraphQLWithToken(productsQuery, token); + + expect(productsResponse.errors).toBeFalsy(); + expect(productsResponse.data).toEqual({ + [Product.plural]: expect.toIncludeSameMembers([ + { productId: "A", isEmpty: false, isPublic: true }, + { productId: "B", isEmpty: false, isPublic: true }, + ]), + }); + + const productsAndVariantsQuery = /* GraphQL */ ` + query { + ${Product.plural} { + productId + isEmpty + isPublic + variants { + productId + isEmpty + isPublic + } + } + } + `; + + const productsAndVariantsResponse = await testHelper.executeGraphQLWithToken(productsAndVariantsQuery, token); + + expect(productsAndVariantsResponse.errors).toBeFalsy(); + expect(productsAndVariantsResponse.data).toEqual({ + [Product.plural]: expect.toIncludeSameMembers([ + { + productId: "A", + isEmpty: false, + isPublic: true, + variants: [], + }, + { + productId: "B", + isEmpty: false, + isPublic: true, + variants: [{ productId: "A", isEmpty: false, isPublic: true }], + }, + ]), + }); + + const filteredProductsAndVariantsQuery = /* GraphQL */ ` + query { + ${Product.plural}(where: { variantsAggregate: { count: { eq: 1 } } }) { + productId + variantsAggregate { + count + } + } + } + `; + + const filteredProductsAndVariantsResponse = await testHelper.executeGraphQLWithToken( + filteredProductsAndVariantsQuery, + token + ); + + expect(filteredProductsAndVariantsResponse.errors).toBeFalsy(); + expect(filteredProductsAndVariantsResponse.data).toEqual({ + [Product.plural]: expect.toIncludeSameMembers([{ productId: "B", variantsAggregate: { count: 1 } }]), + }); + }); +}); From 2df03649374cd0972d96ed118139d01021b72d95 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Mon, 17 Feb 2025 16:56:12 +0000 Subject: [PATCH 10/13] Add `FILTER` operation, and also add for authentication --- .../type-dependant-directives/authentication.ts | 3 ++- .../type-dependant-directives/authorization.ts | 11 ++++++----- .../type-dependant-directives/static-definitions.ts | 2 ++ .../annotation/AuthenticationAnnotation.ts | 1 + .../annotation/AuthorizationAnnotation.ts | 3 ++- .../annotations-parser/authentication-annotation.ts | 1 + .../src/translate/queryAST/factory/FilterFactory.ts | 9 ++++++++- .../graphql/tests/integration/issues/5534.int.test.ts | 2 +- packages/graphql/tests/tck/issues/5534.test.ts | 2 +- 9 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/graphql/src/graphql/directives/type-dependant-directives/authentication.ts b/packages/graphql/src/graphql/directives/type-dependant-directives/authentication.ts index 65c946eccd..25eb43fe92 100644 --- a/packages/graphql/src/graphql/directives/type-dependant-directives/authentication.ts +++ b/packages/graphql/src/graphql/directives/type-dependant-directives/authentication.ts @@ -19,11 +19,12 @@ import { astFromDirective } from "@graphql-tools/utils"; import type { DirectiveDefinitionNode } from "graphql"; -import { GraphQLString, GraphQLDirective, GraphQLInputObjectType, GraphQLList, DirectiveLocation } from "graphql"; +import { DirectiveLocation, GraphQLDirective, GraphQLInputObjectType, GraphQLList, GraphQLString } from "graphql"; import { AUTHENTICATION_OPERATION } from "./static-definitions"; const authenticationDefaultOperations = [ "READ", + "FILTER", "AGGREGATE", "CREATE", "UPDATE", diff --git a/packages/graphql/src/graphql/directives/type-dependant-directives/authorization.ts b/packages/graphql/src/graphql/directives/type-dependant-directives/authorization.ts index dd267a07c8..6d431fba5d 100644 --- a/packages/graphql/src/graphql/directives/type-dependant-directives/authorization.ts +++ b/packages/graphql/src/graphql/directives/type-dependant-directives/authorization.ts @@ -18,15 +18,15 @@ */ import { astFromDirective, astFromInputObjectType } from "@graphql-tools/utils"; -import type { TypeDefinitionNode, DirectiveDefinitionNode } from "graphql"; +import type { DirectiveDefinitionNode, TypeDefinitionNode } from "graphql"; import { - GraphQLString, - GraphQLSchema, + DirectiveLocation, + GraphQLBoolean, GraphQLDirective, GraphQLInputObjectType, GraphQLList, - GraphQLBoolean, - DirectiveLocation, + GraphQLSchema, + GraphQLString, } from "graphql"; import { AUTHORIZATION_FILTER_OPERATION, @@ -86,6 +86,7 @@ function createAuthorizationFilterRule( type: new GraphQLList(AUTHORIZATION_FILTER_OPERATION), defaultValue: [ "READ", + "FILTER", "AGGREGATE", "UPDATE", "DELETE", diff --git a/packages/graphql/src/graphql/directives/type-dependant-directives/static-definitions.ts b/packages/graphql/src/graphql/directives/type-dependant-directives/static-definitions.ts index 1127c126f9..3e2837a847 100644 --- a/packages/graphql/src/graphql/directives/type-dependant-directives/static-definitions.ts +++ b/packages/graphql/src/graphql/directives/type-dependant-directives/static-definitions.ts @@ -41,6 +41,7 @@ export const AUTHORIZATION_FILTER_OPERATION = new GraphQLEnumType({ name: "AuthorizationFilterOperation", values: { READ: { value: "READ" }, + FILTER: { value: "FILTER" }, AGGREGATE: { value: "AGGREGATE" }, UPDATE: { value: "UPDATE" }, DELETE: { value: "DELETE" }, @@ -54,6 +55,7 @@ export const AUTHENTICATION_OPERATION = new GraphQLEnumType({ values: { CREATE: { value: "CREATE" }, READ: { value: "READ" }, + FILTER: { value: "FILTER" }, AGGREGATE: { value: "AGGREGATE" }, UPDATE: { value: "UPDATE" }, DELETE: { value: "DELETE" }, diff --git a/packages/graphql/src/schema-model/annotation/AuthenticationAnnotation.ts b/packages/graphql/src/schema-model/annotation/AuthenticationAnnotation.ts index 756d047698..3147c9a92a 100644 --- a/packages/graphql/src/schema-model/annotation/AuthenticationAnnotation.ts +++ b/packages/graphql/src/schema-model/annotation/AuthenticationAnnotation.ts @@ -22,6 +22,7 @@ import type { Annotation } from "./Annotation"; export type AuthenticationOperation = | "READ" + | "FILTER" | "AGGREGATE" | "CREATE" | "UPDATE" diff --git a/packages/graphql/src/schema-model/annotation/AuthorizationAnnotation.ts b/packages/graphql/src/schema-model/annotation/AuthorizationAnnotation.ts index be768017ea..e03c1d3247 100644 --- a/packages/graphql/src/schema-model/annotation/AuthorizationAnnotation.ts +++ b/packages/graphql/src/schema-model/annotation/AuthorizationAnnotation.ts @@ -18,13 +18,14 @@ */ import type { GraphQLWhereArg } from "../../types"; -import type { Annotation } from "./Annotation"; import type { ValueOf } from "../../utils/value-of"; +import type { Annotation } from "./Annotation"; export const AuthorizationAnnotationArguments = ["filter", "validate"] as const; export const AuthorizationFilterOperationRule = [ "READ", + "FILTER", "AGGREGATE", "UPDATE", "DELETE", diff --git a/packages/graphql/src/schema-model/parser/annotations-parser/authentication-annotation.ts b/packages/graphql/src/schema-model/parser/annotations-parser/authentication-annotation.ts index cc2b739248..ba950b9324 100644 --- a/packages/graphql/src/schema-model/parser/annotations-parser/authentication-annotation.ts +++ b/packages/graphql/src/schema-model/parser/annotations-parser/authentication-annotation.ts @@ -24,6 +24,7 @@ import { parseArgumentsFromUnknownDirective } from "../parse-arguments"; const authenticationDefaultOperations: AuthenticationOperation[] = [ "READ", + "FILTER", "AGGREGATE", "CREATE", "UPDATE", diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index e6bd706f5a..a4fba209fb 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -28,6 +28,7 @@ import type { ConnectionWhereArg, GraphQLWhereArg } from "../../../types"; import type { Neo4jGraphQLTranslationContext } from "../../../types/neo4j-graphql-translation-context"; import { fromGlobalId } from "../../../utils/global-ids"; import { asArray, filterTruthy } from "../../../utils/utils"; +import { checkEntityAuthentication } from "../../authorization/check-authentication"; import { isLogicalOperator } from "../../utils/logical-operators"; import { ConnectionFilter } from "../ast/filters/ConnectionFilter"; import { CypherOneToOneRelationshipFilter } from "../ast/filters/CypherOneToOneRelationshipFilter"; @@ -763,9 +764,15 @@ export class FilterFactory { attributes: AttributeAdapter[] | undefined, context: Neo4jGraphQLTranslationContext ): AuthorizationFilters[] { + checkEntityAuthentication({ + entity: entity.entity, + targetOperations: ["FILTER"], + context, + }); + return this.queryASTFactory.authorizationFactory.getAuthFilters({ entity, - operations: ["READ"], + operations: ["FILTER"], context, attributes, }); diff --git a/packages/graphql/tests/integration/issues/5534.int.test.ts b/packages/graphql/tests/integration/issues/5534.int.test.ts index e697df178c..d63a21f03f 100644 --- a/packages/graphql/tests/integration/issues/5534.int.test.ts +++ b/packages/graphql/tests/integration/issues/5534.int.test.ts @@ -39,7 +39,7 @@ describe("https://github.com/neo4j/graphql/issues/5534", () => { filter: [ { requireAuthentication: false - operations: [READ, AGGREGATE] + operations: [READ, AGGREGATE, FILTER] where: { AND: [{ node: { isPublic: { eq: true } } }, { node: { isEmpty: { eq: false } } }] } } ] diff --git a/packages/graphql/tests/tck/issues/5534.test.ts b/packages/graphql/tests/tck/issues/5534.test.ts index 6ce28b7f95..3e7f8c3acf 100644 --- a/packages/graphql/tests/tck/issues/5534.test.ts +++ b/packages/graphql/tests/tck/issues/5534.test.ts @@ -33,7 +33,7 @@ describe("https://github.com/neo4j/graphql/issues/5534", () => { filter: [ { requireAuthentication: false - operations: [READ, AGGREGATE] + operations: [READ, AGGREGATE, FILTER] where: { AND: [{ node: { isPublic: { eq: true } } }, { node: { isEmpty: { eq: true } } }] } } ] From 3c1542534dd6f0187d95e83123b2e438e0287c30 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Mon, 17 Feb 2025 17:57:37 +0000 Subject: [PATCH 11/13] Add integration tests for authentication --- .../queryAST/factory/FilterFactory.ts | 9 + .../is-authenticated.int.test.ts | 196 ++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index a4fba209fb..8d4ee9466d 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -770,6 +770,15 @@ export class FilterFactory { context, }); + attributes?.forEach((attribute) => { + checkEntityAuthentication({ + entity: entity.entity, + targetOperations: ["FILTER"], + context, + field: attribute.name, + }); + }); + return this.queryASTFactory.authorizationFactory.getAuthFilters({ entity, operations: ["FILTER"], diff --git a/packages/graphql/tests/integration/directives/authorization/is-authenticated.int.test.ts b/packages/graphql/tests/integration/directives/authorization/is-authenticated.int.test.ts index 84a125cdea..815f5b3752 100644 --- a/packages/graphql/tests/integration/directives/authorization/is-authenticated.int.test.ts +++ b/packages/graphql/tests/integration/directives/authorization/is-authenticated.int.test.ts @@ -3321,4 +3321,200 @@ describe("auth/is-authenticated", () => { }); }); }); + + describe("filter", () => { + test("should throw if not authenticated type definition", async () => { + const typeDefs = /* GraphQL */ ` + type ${Product} @node { + id: ID + name: String + purchasedBy: [${User}!]! @relationship(type: "PURCHASED", direction: IN) + } + + type ${User} @authentication(operations: [FILTER]) @node { + id: ID + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + const query = /* GraphQL */ ` + { + ${Product.plural}(where: { purchasedBy: { some: { id: { eq: "1" } } } }) { + id + } + } + `; + + const token = "not valid token"; + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect((gqlResult.errors as any[])[0].message).toBe("Unauthenticated"); + }); + + test("should not throw if authenticated type definition", async () => { + const typeDefs = /* GraphQL */ ` + type ${Product} @node { + id: ID + name: String + purchasedBy: [${User}!]! @relationship(type: "PURCHASED", direction: IN) + } + + type ${User} @authentication(operations: [FILTER]) @node { + id: ID + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + const query = /* GraphQL */ ` + { + ${Product.plural}(where: { purchasedBy: { some: { id: { eq: "1" } } } }) { + id + } + } + `; + + const token = createBearerToken(secret, { roles: ["super-admin", "admin"] }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + }); + + test("should not throw if authenticated with correct role type definition", async () => { + const typeDefs = /* GraphQL */ ` + type JWTPayload @jwt { + roles: [String!]! + } + + type ${Product} @node { + id: ID + name: String + purchasedBy: [${User}!]! @relationship(type: "PURCHASED", direction: IN) + } + + type ${User} @authentication(operations: [FILTER], jwt: { roles: { includes: "admin" } }) @node { + id: ID + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + const query = /* GraphQL */ ` + { + ${Product.plural}(where: { purchasedBy: { some: { id: { eq: "1" } } } }) { + id + } + } + `; + + const token = createBearerToken(secret, { roles: ["super-admin", "admin"] }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + }); + + test("should throw if authenticated with incorrect role type definition", async () => { + const typeDefs = /* GraphQL */ ` + type JWTPayload @jwt { + roles: [String!]! + } + + type ${Product} @node { + id: ID + name: String + purchasedBy: [${User}!]! @relationship(type: "PURCHASED", direction: IN) + } + + type ${User} @authentication(operations: [FILTER], jwt: { roles: { includes: "admin" } }) @node { + id: ID + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + const query = /* GraphQL */ ` + { + ${Product.plural}(where: { purchasedBy: { some: { id: { eq: "1" } } } }) { + id + } + } + `; + + const token = createBearerToken(secret, { roles: ["super-admin"] }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect((gqlResult.errors as any[])[0].message).toBe("Unauthenticated"); + }); + + test("should throw if not authenticated on field definition", async () => { + const typeDefs = /* GraphQL */ ` + type ${Product} @node { + id: ID + name: String + purchasedBy: [${User}!]! @relationship(type: "PURCHASED", direction: IN) + } + + type ${User} @node { + id: ID + password: String @authentication(operations: [FILTER]) + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + const query = /* GraphQL */ ` + { + ${Product.plural}(where: { purchasedBy: { some: { password: { eq: "password" } } } }) { + id + } + } + `; + + const token = "not valid token"; + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect((gqlResult.errors as any[])[0].message).toBe("Unauthenticated"); + }); + }); }); From ba5e65d5adcc8242fbdd2a0ab96db7dc766f6e98 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 20 Feb 2025 15:01:28 +0000 Subject: [PATCH 12/13] Add tests over relationship fields --- .../authorization/where.int.test.ts | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/packages/graphql/tests/integration/directives/authorization/where.int.test.ts b/packages/graphql/tests/integration/directives/authorization/where.int.test.ts index bfce10d846..0558519cc8 100644 --- a/packages/graphql/tests/integration/directives/authorization/where.int.test.ts +++ b/packages/graphql/tests/integration/directives/authorization/where.int.test.ts @@ -813,4 +813,132 @@ describe("auth/where", () => { expect(users).toEqual([{ id: userId, posts: [{ id: postId2 }] }]); }); }); + + describe("filter", () => { + test("should add $jwt.id filter over relationship", async () => { + const typeDefs = /* GraphQL */ ` + type ${User} @node { + id: ID + name: String + posts: [${Post}!]! @relationship(type: "HAS_POST", direction: OUT) + } + + type ${Post} @node { + id: ID + creator: [${User}!]! @relationship(type: "HAS_POST", direction: IN) + } + + extend type ${User} @authorization(filter: [{ operations: [FILTER], where: { node: { id: { eq: "$jwt.sub" } } } }]) + `; + + const userId1 = generate({ + charset: "alphabetic", + }); + + const userId2 = generate({ + charset: "alphabetic", + }); + + const postId1 = generate({ + charset: "alphabetic", + }); + + const postId2 = generate({ + charset: "alphabetic", + }); + + const query = /* GraphQL */ ` + { + ${Post.plural}(where: { creator: { some: { name: { eq: "darrell" } } } }) { + id + } + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (:${User} {id: "${userId1}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId1}"}) + CREATE (:${User} {id: "${userId2}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId2}"}) + `); + + const token = createBearerToken(secret, { sub: userId1 }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + + expect(gqlResult.data).toEqual({ [Post.plural]: [{ id: postId1 }] }); + }); + + test("should add $jwt.id to filter over connection", async () => { + const typeDefs = /* GraphQL */ ` + type ${User} @node { + id: ID + name: String + posts: [${Post}!]! @relationship(type: "HAS_POST", direction: OUT) + } + + type ${Post} @node { + id: ID + creator: [${User}!]! @relationship(type: "HAS_POST", direction: IN) + } + + extend type ${User} @authorization(filter: [{ operations: [FILTER], where: { node: { id: { eq: "$jwt.sub" } } } }]) + `; + + const userId1 = generate({ + charset: "alphabetic", + }); + + const userId2 = generate({ + charset: "alphabetic", + }); + + const postId1 = generate({ + charset: "alphabetic", + }); + + const postId2 = generate({ + charset: "alphabetic", + }); + + const query = /* GraphQL */ ` + { + ${Post.plural}(where: { creatorConnection: { some: { node: { name: { eq: "darrell" } } } } }) { + id + } + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (:${User} {id: "${userId1}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId1}"}) + CREATE (:${User} {id: "${userId2}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId2}"}) + `); + + const token = createBearerToken(secret, { sub: userId1 }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + + expect(gqlResult.data).toEqual({ [Post.plural]: [{ id: postId1 }] }); + }); + }); }); From 837275e8ddb81a2bb5840b8ae6ac9b802e57fcd3 Mon Sep 17 00:00:00 2001 From: Darrell Warde Date: Thu, 27 Feb 2025 16:24:49 +0000 Subject: [PATCH 13/13] Commit to get branch to the point to which filtering will work. However, the code here will not work for the `single` case, as it will try to add subqueries inside a list comprehension. --- .../queryAST/ast/filters/ConnectionFilter.ts | 15 +- .../ast/filters/RelationshipFilter.ts | 25 ++- .../authorization/where.int.test.ts | 150 ++++++++++++++++++ 3 files changed, 181 insertions(+), 9 deletions(-) diff --git a/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts index dfd7cf950b..ede5b0693d 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/ConnectionFilter.ts @@ -216,6 +216,10 @@ export class ConnectionFilter extends Filter { const returnVar = new Cypher.Variable(); const innerFiltersPredicates: Cypher.Predicate[] = []; + const authFilterSubqueries = this.getAuthFilterSubqueries(this.target.name, queryASTContext).map((sq) => + new Cypher.Call(sq).importWith(queryASTContext.target) + ); + const subqueries = this.innerFilters.flatMap((f) => { const nestedSubqueries = f .getSubqueries(queryASTContext) @@ -231,7 +235,7 @@ export class ConnectionFilter extends Filter { return clauses; }); - if (subqueries.length === 0) return []; // Hack logic to change predicates logic + // if (subqueries.length === 0) return []; // Hack logic to change predicates logic const comparisonValue = this.operator === "NONE" ? Cypher.false : Cypher.true; this.subqueryPredicate = Cypher.eq(returnVar, comparisonValue); @@ -244,7 +248,7 @@ export class ConnectionFilter extends Filter { const withPredicateReturn = new Cypher.With("*") .where(Cypher.and(...innerFiltersPredicates)) .return([countComparisonPredicate, returnVar]); - return [Cypher.utils.concat(match, ...subqueries, withPredicateReturn)]; + return [Cypher.utils.concat(match, ...authFilterSubqueries, ...subqueries, withPredicateReturn)]; } // This method has a big deal of complexity due to a couple of factors: @@ -311,4 +315,11 @@ export class ConnectionFilter extends Filter { return filterTruthy(authFilters.map((f) => f.getPredicate(context))); } + + protected getAuthFilterSubqueries(name: string, context: QueryASTContext): Cypher.Clause[] { + const authFilters = this.authFilters[name]; + if (!authFilters) return []; + + return filterTruthy(authFilters.flatMap((f) => f.getSubqueries(context))); + } } diff --git a/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts b/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts index 93898ac773..17f2c835bc 100644 --- a/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts +++ b/packages/graphql/src/translate/queryAST/ast/filters/RelationshipFilter.ts @@ -92,7 +92,6 @@ export class RelationshipFilter extends Filter { const nestedSubqueries = this.targetNodeFilters.flatMap((f) => f.getSubqueries(nestedContext)); const nestedSelection = this.getNestedSelectionSubqueries(nestedContext); - const authFilterSubqueries = this.getAuthFilterSubqueries(nestedContext); if (nestedSubqueries.length > 0) { subqueries.push(...this.getNestedSubqueries(nestedContext)); @@ -102,10 +101,6 @@ export class RelationshipFilter extends Filter { subqueries.push(...nestedSelection); } - if (authFilterSubqueries.length > 0) { - subqueries.push(...authFilterSubqueries); - } - return subqueries; } @@ -160,6 +155,11 @@ export class RelationshipFilter extends Filter { context: QueryASTContext ): Cypher.Predicate | undefined { const predicates = this.targetNodeFilters.map((c) => c.getPredicate(context)); + + const authFilterSubqueries = this.getAuthFilterSubqueries(context).map((sq) => + new Cypher.Call(sq).importWith("*") + ); + const authPredicates = this.getAuthFilterPredicate(context); const innerPredicate = Cypher.and(...authPredicates, ...predicates); @@ -186,12 +186,23 @@ export class RelationshipFilter extends Filter { } case "NONE": case "SOME": { + let exists: Cypher.Exists; + const match = new Cypher.Match(pattern); + if (innerPredicate) { - match.where(innerPredicate); + if (authFilterSubqueries.length > 0) { + const withPredicateReturn = new Cypher.With("*").where(Cypher.and(innerPredicate)); + const clause = Cypher.utils.concat(match, ...authFilterSubqueries, withPredicateReturn); + exists = new Cypher.Exists(clause); + } else { + match.where(innerPredicate); + exists = new Cypher.Exists(match); + } + } else { + exists = new Cypher.Exists(match); } - const exists = new Cypher.Exists(match); if (this.operator === "NONE") { return Cypher.not(exists); } diff --git a/packages/graphql/tests/integration/directives/authorization/where.int.test.ts b/packages/graphql/tests/integration/directives/authorization/where.int.test.ts index 0558519cc8..aa4fa096d1 100644 --- a/packages/graphql/tests/integration/directives/authorization/where.int.test.ts +++ b/packages/graphql/tests/integration/directives/authorization/where.int.test.ts @@ -940,5 +940,155 @@ describe("auth/where", () => { expect(gqlResult.data).toEqual({ [Post.plural]: [{ id: postId1 }] }); }); + + test("should add filter to relationship filter for users with over 2 posts", async () => { + const typeDefs = /* GraphQL */ ` + type ${User} @node { + id: ID + name: String + posts: [${Post}!]! @relationship(type: "HAS_POST", direction: OUT) + } + + type ${Post} @node { + id: ID + creator: [${User}!]! @relationship(type: "HAS_POST", direction: IN) + } + + extend type ${User} @authorization(filter: [{ operations: [FILTER], where: { node: { postsAggregate: { count: { gt: 2 } } } } }]) + `; + + const userId1 = generate({ + charset: "alphabetic", + }); + + const userId2 = generate({ + charset: "alphabetic", + }); + + const postId1 = generate({ + charset: "alphabetic", + }); + + const postId2 = generate({ + charset: "alphabetic", + }); + + const postId3 = generate({ + charset: "alphabetic", + }); + + const postId4 = generate({ + charset: "alphabetic", + }); + + const query = /* GraphQL */ ` + { + ${Post.plural}(where: { creator: { some: { name: { eq: "darrell" } } } }) { + id + } + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (:${User} {id: "${userId1}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId1}"}) + CREATE (u:${User} {id: "${userId2}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId2}"}) + CREATE (u)-[:HAS_POST]->(:${Post} {id: "${postId3}"}) + CREATE (u)-[:HAS_POST]->(:${Post} {id: "${postId4}"}) + `); + + const token = createBearerToken(secret, { sub: userId1 }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + + expect(gqlResult.data).toEqual({ + [Post.plural]: expect.toIncludeSameMembers([{ id: postId2 }, { id: postId3 }, { id: postId4 }]), + }); + }); + + test("should add filter to connection filter for users with over 2 posts", async () => { + const typeDefs = /* GraphQL */ ` + type ${User} @node { + id: ID + name: String + posts: [${Post}!]! @relationship(type: "HAS_POST", direction: OUT) + } + + type ${Post} @node { + id: ID + creator: [${User}!]! @relationship(type: "HAS_POST", direction: IN) + } + + extend type ${User} @authorization(filter: [{ operations: [FILTER], where: { node: { postsAggregate: { count: { gt: 2 } } } } }]) + `; + + const userId1 = generate({ + charset: "alphabetic", + }); + + const userId2 = generate({ + charset: "alphabetic", + }); + + const postId1 = generate({ + charset: "alphabetic", + }); + + const postId2 = generate({ + charset: "alphabetic", + }); + + const postId3 = generate({ + charset: "alphabetic", + }); + + const postId4 = generate({ + charset: "alphabetic", + }); + + const query = /* GraphQL */ ` + { + ${Post.plural}(where: { creatorConnection: { some: { node: { name: { eq: "darrell" } } } } }) { + id + } + } + `; + + await testHelper.initNeo4jGraphQL({ + typeDefs, + features: { + authorization: { + key: secret, + }, + }, + }); + + await testHelper.executeCypher(` + CREATE (:${User} {id: "${userId1}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId1}"}) + CREATE (u:${User} {id: "${userId2}", name: "darrell"})-[:HAS_POST]->(:${Post} {id: "${postId2}"}) + CREATE (u)-[:HAS_POST]->(:${Post} {id: "${postId3}"}) + CREATE (u)-[:HAS_POST]->(:${Post} {id: "${postId4}"}) + `); + + const token = createBearerToken(secret, { sub: userId1 }); + + const gqlResult = await testHelper.executeGraphQLWithToken(query, token); + + expect(gqlResult.errors).toBeUndefined(); + + expect(gqlResult.data).toEqual({ + [Post.plural]: expect.toIncludeSameMembers([{ id: postId2 }, { id: postId3 }, { id: postId4 }]), + }); + }); }); });