Auth (and other) directives are applied to OGM calls when custom resolver context is passed

[happenslol]

Describe the bug I'm honestly not sure if this is a bug or just undocumented behavior introduced in 4.0.0, but it fills a niche my team was previously covering with custom code in a fork of @neo4j/graphql, so I wanted to get some clarity and find out whether we can rely on this as a feature.

Basically, when a custom resolver is implemented, and the context that resolver receives is passed to the OGM, the schemaModel of the resolver is used instead of the schema the OGM was constructed with. This results in @authorization, @authentication, @private and so being respected, even though they have been filtered out in the OGM using the filterDocument function in the OGM constructor.

Since we previously had the requirement to apply auth to OGM calls (in the situation where a custom resolver would return sub-entities managed by the OGM and not by us, i.e. "re-entering" into the graph after a custom resolver), we had implemented a flag that would prevent filterDocument from filtering out auth directives. This allowed us to construct 2 OGMs where one would respect auth, and the other could be used for "internal" queries.
The current behavior in 4.0.0 essentially allows the following:

// This call respects @authorization
function customResolver(source, args, context, info) {
  return ogm.model("Post").find({ selectionSet, context })
}

// This will return anything
function customResolver(source, args, context, info) {
  return ogm.model("Post").find({ selectionSet })
}

In practice, we would strip only the schemaModel from the context to keep the transaction and executionContext in general.

Type definitions Any custom resolver accessing another authenticated or private field through the OGM should do the trick here, although I haven't tested the private case yet. Let's take the examples from the documentation:

type User {
  id: ID!
}

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

# Add a custom resolver:
type Query {
  postById(id: ID!): Post
}

Now, the code above can be used in the custom resolver to reproduce the behavior.

Additional context From what I can tell, the schemaModel is first added to the context of every resolver in wrapQueryAndMutation (packages/graphql/src/classes/Neo4jGraphQL.ts):

const wrapResolverArgs: WrapResolverArguments = {
    driver: this.driver,
    nodes: this.nodes,
    relationships: this.relationships,
    schemaModel: this.schemaModel,
    // ...
};

When the OGM calls any endpoint, it will pass the context along (packages/ogm/src/classes/Model.ts):

const result = await graphql({
    schema: this.schema,
    source: query,
    rootValue,
    // Context is passed here, which will contain schemaModel with authorization etc. when
    // coming from a custom resolver
    contextValue: { sessionConfig: { database: this.database }, ...context }, // <--
    variableValues,
});

Now, the difference introduced in 4.0.0 is that the schemaModel introduced in wrapQueryAndMutation is overwritten by the incoming context (packages/graphql/src/schema/resolvers/composition/wrap-query-and-mutation.ts):

const internalContext = {
    nodes,
    relationships,
    schemaModel,
    features,
    subscriptionsEnabled,
    executor,
    neo4jDatabaseInfo,
    authorization: authorizationContext,
    // Consider anything in here overrides
    // If this contains a schemaModel, it will override the one above
    ...context, // <--
};

// Also, the additional spread here seems unnecessary? any value in context
// is already in `internalContext`, and nothing in `internalContext` can override
// fields in `context`
return next(root, args, { ...context, ...internalContext }, info);

And this overwritten value is then used when constructing auth filters (packages/graphql/src/translate/authorization/create-authorization-after-predicate.ts):

// This then creates an AuthFilterFactory internally, using the same schema
const factory = new QueryASTFactory(context.schemaModel);

Previously, the context value was always taken from the wrapResolver arguments, so it could never be changed after the OGM was constructed with the filtered document.

Another implication of this behavior (which might prevent us from using this) is that OGM methods will only be available for fields that don't have an @exclude/@query(read: false)/... annotation, since the model will not contain the correct endpoints otherwise.

[neo4j-team-graphql]

Many thanks for raising this bug report @happenslol. :bug: We will now attempt to reproduce the bug based on the steps you have provided.

Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:

• Type definitions
• Resolvers
• Query and/or Mutation (or multiple) needed to reproduce

If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket.

Thanks again! :pray:

[mjfwebb]

Hey @happenslol, can you possibly provide a single code example that I can use to confirm this?

Including OGM instantiation, library instantiation, type definitions, resolvers, and the values in the context you are passing in.

Thank you!

[happenslol]

Hey, thanks for your response. I will get a minimal reproduction in the form of a complete repository to you as soon as possible, but it could take a few days since I'm pretty busy at the moment.

So far, I was able to figure out that the problem only occurs with top level custom resolvers - not field level resolvers annotated with @customResolver (I've wrongly added a @customResolver annotation to a top level resolver in my code example - sorry about that, it's corrected now). I'll report back as soon as I get my minimal example ready.

[vpctorr]

Another implication of this behavior (which might prevent us from using this) is that OGM methods will only be available for fields that don't have an @exclude/@query(read: false)/... annotation, since the model will not contain the correct endpoints otherwise.

So this is what I've been struggling with for ages !

I couldn't understand why passing context to the OGM would sometimes cause errors. It's something that I actually need to do, since it contains executionContext and cypherParams.

There's actually so much stuff that's added into the context…

…including the Neo4JGraphQL schema, that ends up being used by the OGM, and causes errors when accessing @private fields or running queries with @query(read: false), etc. which should not happen at all !

[angrykoala]

Hi @happenslol I've given it a go to try and reproduce this behaviour and hasn't been able to

I'm trying using the following typedefs:

type Movie @authorization(filter: [{ where: { node: { public: "true" } } }]) {
    title: String @private
    public: String
}

type Query {
    getMovies: [Movie!]!
}

And the following customResolvers:

const Movie = ogm.model("Movie");

const resolvers: Neo4jGraphQLConstructor["resolvers"] = {
    Query: {
        getMovies: async (parent, args, context, resolveInfo) => {
            const movies = await Movie.find({ selectionSet: `{ title }`, context });
            console.log(movies);
            return movies;
        },
    },
};

If I understood correctly, both the @authorization and @private directives are being applied to the OGM in getMovies for you. However, when running the following query:

query Movies {
  movies {
    public
  }
  getMovies {
    public
  }
}

I receive an empty array for movies (as none of them fulfils the requirement of public: "true"), but I receive an array of all the movies in getMovies (with null values, as none of them has the public value set). Also, the private field gets accessed in the resolver (all the movie titles are logged out in the console.log(movies)

So, I'm not sure if I'm missing something, but this seems to be the expected behaviour and not what you are experiencing right? Is there anything else that may help reproduce this issue?

I'm using the following versions of the library and ogm:

"@neo4j/graphql": "^5.3.1",
"@neo4j/graphql-ogm": "^5.3.1",

[happenslol]

Hey, thanks for having a look at this.

I've had other priorities so I haven't been able to create a complete reproduction - I currently have an example working that partially shows the issue though. I can demonstrate the context being polluted with lots of values (and overwriting existing values) when the OGM is used inside of a top-level resolver. However, in my current reproduction this doesn't lead to the directive being applied (even though they are present in context.schemaModel), and I can't quite figure out why.

The issue I described in the first post is something that occurred in a large project with lots of moving parts, so there might have been something else at play here. I still think the context pollution warrants a fix, since it results in the context containing lots of untyped extra fields.

I'll upload my partial reproduction tomorrow, so you can at least check out that part.

[angrykoala]

Ok, so the problem is that the context is polluted when passing the context from the top level resolver, not that it affects the directives being applied right?

[happenslol]

Here is my partial reproduction.

You can see what I mean by placing logs in the @neo4j/graphql package (either by linking it locally or editing files in node_modules directly), specifically in schema/resolvers/composition/wrap-query-and-mutation, in the wrapQueryAndMutation function. In the first call on line 53 or my reproduction, context.schemaModel will be present (and will overwrite the schema model you are explicitly setting later in the function), while in the second call on line 54, it will be undefined.

In the issue I had in our project, the schema was being polluted, partly overwritten, and from what I could tell, this led to directives being applied in OGM calls (since many of the translators/factories work by using the schemaModel from the context). I was definitely seeing auth being applied to OGM queries, in the actual database queries output using the debug flag.
While it is possible that something else was causing it, and it has proven difficult to reproduce that effect, the issue went away as soon as I removed the context spread.

[angrykoala]

Thanks @happenslol :+1: