Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filters of @authorization directive are not generated as WHERE clauses for subqueries which match on the node for which these filters are defined. #5534

Open
andreloeffelmann opened this issue Sep 5, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@andreloeffelmann
Copy link

Describe the bug
When defining filters via the @authorization directive, these filters are not rendered as WHERE clauses inside subqueries which match/reference on the (root)-node for which these filters are defined.

Type definitions

type Product @mutation(operations: []) @authorization(filter: [{requireAuthentication: false, operations: [READ,AGGREGATE], where: {AND: [{node:{isPublic:true}}, {node:{isEmpty:false}}] }}]) @subscription(events: []) {
	"""Unique Identifier of this product"""
	productId: Int! @unique
	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: [], queryDirection: DEFAULT_DIRECTED) @settable(onCreate: false, onUpdate: false)
}

To Reproduce
Steps to reproduce the behavior:

  1. Run a server with the type definitions and DEBUG enabled
  2. Execute the following Query:
query{
 products(options:{limit:1} where:{variantsAggregate:{count:1}}){
    productId  
    variantsAggregate{
      count
    }
  }
}
  1. See the generated cypher-query:
MATCH (this:Product)
CALL {
    WITH this
    MATCH (this)<-[this0:PRODUCT_HAS_FAMILY_PRODUCT]-(this1:Product)
    RETURN count(this1) = $param0 AS var2
}
WITH *
WHERE (var2 = true AND (($param1 IS NOT NULL AND this.isEmpty = $param1) AND ($param2 IS NOT NULL AND this.isPublic = $param2)))
WITH *

LIMIT $param3
CALL {
    WITH this
    MATCH (this)<-[this3:PRODUCT_HAS_FAMILY_PRODUCT]-(this4:Product)
    WHERE (($param4 IS NOT NULL AND this4.isEmpty = $param4) AND ($param5 IS NOT NULL AND this4.isPublic = $param5))
    RETURN count(this4) AS var5
}
RETURN this { .productId, variantsAggregate: { count: var5 } } AS this
cypher params: {
  param0: Integer { low: 1, high: 0 },
  param1: false,
  param2: true,
  param3: Integer { low: 1, high: 0 },
  param4: false,
  param5: true
}

The generated query misses the authorization filters inside the subquery on the referenced Product node. The query should look like this (see 4th line):

MATCH (this:Product)
CALL {
    WITH this
    MATCH (this)<-[this0:PRODUCT_HAS_FAMILY_PRODUCT]-(this1:Product) WHERE (($param1 IS NOT NULL AND this.isEmpty = $param1) AND ($param2 IS NOT NULL AND this.isPublic = $param2))
    RETURN count(this1) = $param0 AS var2
}
WITH *
WHERE (var2 = true AND (($param1 IS NOT NULL AND this.isEmpty = $param1) AND ($param2 IS NOT NULL AND this.isPublic = $param2)))
WITH *

LIMIT $param3
CALL {
    WITH this
    MATCH (this)<-[this3:PRODUCT_HAS_FAMILY_PRODUCT]-(this4:Product)
    WHERE (($param4 IS NOT NULL AND this4.isEmpty = $param4) AND ($param5 IS NOT NULL AND this4.isPublic = $param5))
    RETURN count(this4) AS var5
}
RETURN this { .productId, variantsAggregate: { count: var5 } } AS this
cypher params: {
  param0: Integer { low: 1, high: 0 },
  param1: false,
  param2: true,
  param3: Integer { low: 1, high: 0 },
  param4: false,
  param5: true
}

Expected behavior
Authorization filters from the @authorization directive should be generated as WHERE clauses on all subqueries/queries which match on a node for which these filters are defined.

System

@andreloeffelmann andreloeffelmann added the bug Something isn't working label Sep 5, 2024
@andreloeffelmann
Copy link
Author

Corresponding ZenDesk ticket here

@darrellwarde
Copy link
Contributor

Hey @andreloeffelmann, I'm going to relabel this as a feature request, because this isn't intended to work this way - this would essentially be classed as something like a new operation FILTER inside the @authorization directive which isn't currently available. The READ and AGGREGATE operations apply to the fields inside the selection set, and fields within the input objects.

@darrellwarde darrellwarde added enhancement New feature or request and removed bug Something isn't working labels Sep 5, 2024
@andreloeffelmann
Copy link
Author

Hey @darrellwarde, I see your point from a developer's perspective.
However, the current state of neo4j-graphql is just not consistent. When executing the query of the example above I receive products which have exactly 1 variants but also those which have 0 or more than 1. The filter of the query just does not work.
And this is something which is not transparent/logically conclusive for the API consumer.
Even worse, it was pure coincidence that we figured this out! The documentation does not mention that @authorization will not work inside filters of a simple READ query.

If you see this as a feature request - OK. But if so, neo4j-graphql should NOT render relationship fields into the filter *Where type which connect to a type for which an @authorization directive is declared. Better to not offer a functionality than offering it in a buggy way!

So, from my point of view, I see the following solutions:

  • either fix the "bug" without adding a new operation FILTER inside the @authorization directive
  • or introduce a new operation FILTER inside the @authorization directive AND remove the relationship fields from the generated filter *Where type which connect to a type for which an @authorization directive is declared if the FILTER operation is not given

Either way, when do you think can we expect a solution on this? This is an internal high-prio case which I already reported via our enterprise support.

@andreloeffelmann
Copy link
Author

Hey @darrellwarde , any news on this? Was hoping that this will be addressed in v6, but according to the docs it was not?

@darrellwarde
Copy link
Contributor

Hey @andreloeffelmann, I totally missed your last message in September, my apologies!

When executing the query of the example above I receive products which have exactly 1 variants but also those which have 0 or more than 1. The filter of the query just does not work.

I'm reading this issue over and over and I can't see how this would be related to this authorization feature request - the variantsAggregate filter that you're applying to your query is being translated into Cypher so this shouldn't be the case even without this authorization filters. Or am I missing something here?

Either way, when do you think can we expect a solution on this? This is an internal high-prio case which I already reported via our enterprise support.

We will have to talk about if and how we address this for Version 7 if it could be a breaking change. However, my view still stands that the authorization features of the library exist to control the reading (i.e. the return of data to consumers) and writing (i.e. database writes) through the API. The usage of the authorization features in this manner, to achieve some kind of "default filter", was never really the intended purpose of the feature. Our primary focus for this feature is that data is secure, which I still believe is the case with the state of things in this issue.

This is not to say that we will not look at this, but just to offer some explanation as why this has not been looked at yet in this time where we have a bunch of high priority architectural changes to work on.

@andreloeffelmann
Copy link
Author

Hey @darrellwarde,

it is not about some kind of "default filter", it is about data being secure.
Let me try to explain that in more detail:

given the type definitions above we insert 3 Product nodes into the database having these properties:

  • Product A: isEmpty: false, isPublic: true
  • Product B: isEmpty: false, isPublic: true
  • Product C: isEmpty: true, isPublic: true

and we create the relation PRODUCT_HAS_FAMILY_PRODUCT from A to B and from A to C. So A has now two variants products.

if we query for products like this:

query{
 products{
    isEmpty
    isPublic
  }
}

we get 2 Product nodes back, A and B, and that's correct, since the authorization filter kicks in and filters C out because it does not match the filter.

Now we want to get the variants of each Product, so we run the following query:

query{
 products{
    isEmpty
    isPublic
    variants{
      isEmpty
      isPublic
    }
  }
}

and we get Product A with one variant (B) and Product B with zero variants. Works as expected!

But now we want to get Products which only have one variant connected. So we run the following query:

query{
 products(where:{variantsAggregate:{count:1}}){
    productId  
    variantsAggregate{
      count
    }
  }
}

We expect one Product node (A) to be returned since A has only one variant connected (from the API's point of view) - the former query correctly returned one variant for A.
But: zero Product nodes are returned. That's because the authorization filter is not considered for the connected Product and thus the generated query misses the authorization filters inside the subquery on the referenced Product node (see my initial post). And this causes the query to return zero nodes - it returns the "truth" of the database, because there in fact are NO Product nodes which have exactly one variant connected.

I do not see why this is some kind of "default" filtering. The API does not produce consistent results.
It is not about that the variantsAggregate filter that I am applying to my query is not being translated into Cypher - it is! But it misses the authorization filter to filter out nodes which are not allowed to be seen for the API.

Hope I could explain the case better now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants