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

Can't access cypherParams from @authorization rule #4634

Closed
jbhurruth opened this issue Feb 1, 2024 · 8 comments
Closed

Can't access cypherParams from @authorization rule #4634

jbhurruth opened this issue Feb 1, 2024 · 8 comments
Labels
bug Something isn't working confirmed Confirmed bug

Comments

@jbhurruth
Copy link

Describe the bug

I'm trying to do a node property comparison with a DateTime that is passed as a cypherParams field inside an @authorization.filter rule. The cypherParam doesn't seem to be injected into the query correctly, being interpolated as a reference to a string parameter named param1 instead of the DateTime parameter named yesterday. The param1 string is itself the string "$yesterday" so it almost worked but wrapped the reference in an unnecessary layer.

Type definitions

type User @authorization(
  filter: [{
    where: { node: { timestamp_GT: "$yesterday"} }
  }]
) {
  id: ID! @id @unique
  timestamp: DateTime! @timestamp
}
const apolloMiddleware = expressMiddleware(apolloServer, {
  context: async ({ req }) => {
    return {
      cypherParams: {
        yesterday: new Date(Date.now() - 1000 * 60 * 60 * 24),
      },
    };
  },
});

To Reproduce

Run the following mutation and query

mutation {
  createUsers(input: [{}]) {
    users {
      id
      timestamp
    }
    info {
      nodesCreated
    }
  }
}
query {
  users {
    id
    timestamp
  }
}

The result is an empty list.

The generated cypher for the query shows the issue

MATCH (this:User)
WITH *
WHERE ($isAuthenticated = true AND ($param1 IS NOT NULL AND this.timestamp > $param1))
RETURN this { .id, timestamp: apoc.date.convertFormat(toString(this.timestamp), "iso_zoned_date_time", "iso_offset_date_time") } AS this

cypher params: {
  isAuthenticated: false,
  param1: '$yesterday',
  yesterday: 2024-01-31T15:48:37.318Z
}

Expected behavior
The query should return any node that was not created more than 24 hours ago.

System:

@jbhurruth jbhurruth added the bug Something isn't working label Feb 1, 2024
@neo4j-team-graphql
Copy link
Collaborator

Many thanks for raising this bug report @jbhurruth. 🐛 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! 🙏

@darrellwarde darrellwarde added enhancement New feature or request and removed bug Something isn't working labels Feb 1, 2024
@darrellwarde
Copy link
Contributor

I've relabelled this as a feature request - this is not a feature we claim to support so we will have to look into adding support for this.

@darrellwarde
Copy link
Contributor

I think a workaround here is to put yesterday directly into the context and refer to it using $context.yesterday.

@jbhurruth
Copy link
Author

Amazing, thanks! Unfortunately the workaround doesn't work for me. There was a bug in my repro where isAuthenticated: false but even after fixing this the comparison still fails. Everything looks like it should be fine but I guess the param type is ambiguous from the debug logging so I'll dig a bit deeper to confirm that its comparing two DateTimes.

@darrellwarde darrellwarde added bug Something isn't working and removed enhancement New feature or request labels Feb 21, 2024
@neo4j-team-graphql
Copy link
Collaborator

Many thanks for raising this bug report @jbhurruth. 🐛 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! 🙏

@angrykoala angrykoala added the confirmed Confirmed bug label Feb 29, 2024
@neo4j-team-graphql
Copy link
Collaborator

We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @jbhurruth! 🙏 We will now prioritise the bug and address it appropriately.

@darrellwarde
Copy link
Contributor

Amazing, thanks! Unfortunately the workaround doesn't work for me. There was a bug in my repro where isAuthenticated: false but even after fixing this the comparison still fails. Everything looks like it should be fine but I guess the param type is ambiguous from the debug logging so I'll dig a bit deeper to confirm that its comparing two DateTimes.

So doing a final investigation on this, the feature does work as it should. Your issue seems to be with the fact that it is a DateTime type. When passing the value into cypherParams, you should look into parsing it into a Neo4j type using the driver, for example: https://github.com/neo4j/graphql/blob/dev/packages/graphql/src/graphql/scalars/DateTime.ts#L41

I hope that helps!

@darrellwarde darrellwarde closed this as not planned Won't fix, can't repro, duplicate, stale Mar 4, 2024
@jbhurruth
Copy link
Author

@darrellwarde Thanks for that! I forgot to update that I figured it out shortly after my last comment. I didn't expect it to get turned back into a bug ticket based on my overshare of "not your problem" info 😅 It was a bit tricky to debug due to how the debug logging prints JS Dates and neo4j DateTime objects similarly, but all good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed Confirmed bug
Projects
Archived in project
Development

No branches or pull requests

4 participants