Feature Request: Update After @auth Validation

Issues GraphQL
,
1

Experience Report for Feature Request

What you wanted to do

Prevent a user from updating certain fields based on @auth rules for validations, the same way I can prevent a user from adding certain fields based on @auth rules for validations.

What you actually did

I can currently prevent a user from adding a certain value to the database by using many different types of field validation @auth rules:

By using some rules like this:

type Language  @withSubscription @auth(
  add: { rule: "query { queryLanguage(filter: { not: { name: { eq: \"French\" } } }) { name code } }" },
  update: { rule: "query { queryLanguage(filter: { not: { name: { eq: \"French\" } } }) { name code } }" },
){
	code: String! @search(by:[exact]) @id
	name: String! @search(by:[exact])
}

But I cannot prevent the user from changing it to the non-allowable value later (or where it can easily pass any add rule I created).

Why that wasn’t great, with examples

Here it will not allow me to add a new Language.name with the value “French”. This is perfect. However, I can easily add some other value, and change it later to “French,” which negates the add rule.

I feel like this is preventing Dgraph from blossoming with any sort of real backend Validation.

Currently @auth:

  • Add New Value (Access to After Results)
  • Delete Current Value (Access to Before Results)
  • Update Current Value to New Value (Only Access to Before Results)

There needs to be different rules for update, as unlike the other, it contains a before and an after value.

Any external references to support your case

J

5 Likes
2

Why this is VERY important and easy to fix…

Right now, there is no way to add any external application to Dgraph’s GraphQl. Only Dgraph can do that, so we are locked-in without any way to secure mutations unless we want to re-write every single mutation in a custom-mutation. This means for any practical application, regular mutations are worthless.

This is far from trivial and concerns Backend Security.

Some more links I found:

More Problems this solves:

I think most people believe @auth works like this out of the box. There are many more problems this could solve, but very little code for very big reward.

Simple Fix:

Edit auth.go!

  • An add mutation is evaluated by data that WILL be in the database…
  • An update mutation is evaluated by data that is already IN the database

Simple add a new rule handle update-after or afterUpdate or whatever name that…

  • An updateAfter is evaluated by data that WILL be in the database (just using the add validation)

These functions are already written, just add a few new lines. If I were a go programmer, I would do a ping request myself.

This is very important and will solve many problems.

There is potentially another option where any incoming data added by an update mutation is evaluated by BOTH @auth add rules, and @auth update rules, but I feel there could be some conflict here.

Either way, this keeps the database consistent, and is VERY LITTLE WORK with BIG REWARDS for us users!

Thanks,

J

4 Likes
4

Looking at the codebase I think there is something here that is interesting:

This is where the data that is added in the mutation transaction is ran against the rules and if the same number of authorized nodes vs added nodes then it passes auth and commits the transaction otherwose returns an error. This is the case for add mutations but update and delete mutations get processed differently. Basically from my understanding the auth gets added to the mutations upserts and then conditionally mutated.

It appears the rewriteAuthQueries function (defined somewhere?) Is responsible for building queries based upon the type and the mutation, add/update/delete. I believe this will also need updated to build a subset of update pre rules and update post rules.

@Naman @MichelDiz @mrjn Am I even close in my understanding of the codebase on this matter?

I believe this is a VERY IMPORTANT and NEEDED feature to ensure that data is not only secure from being added but also secure from being updated. Right now I can create a task only for me but I can update it and assign it to anyone else when I shouldn’t be able to do so

4 Likes
5

@aman-bansal - Can you look at this problem please sir?

The DGraph Docs acknowledge this problem:

Updates have both a before and after state that can be important for authorization.

For example, consider a rule stating that you can only update your own to-do list items. If evaluated in the database before the mutation (like the delete rules) it would prevent you from updating anyone else’s to-do list items, but does it stop you from updating your own to-do items to have a different owner. If evaluated in the database after the mutation occurs, like for add rules, it would prevent setting the owner to another user, but would not prevent editing other’s posts.

Currently, Dgraph evaluates update rules before the mutation.

And right below that it specifies that new documents from set in an update mutaiton are evaluated:

Because a mutation updates a user’s to-do list by inserting a new to-do list item, it would have to satisfy the rules to update the author and the rules to add a to-do list item. If either fail, the mutation has no effect.

There is a very simple fix here that does not involve creating a new update-after auth rule.

  • Dgraph continues to evaluate update rules before the mutation
  • Dgraph also evaluates all update mutations with add rules after the mutation

This already does this with children. Make it work with parents too!

J

6

@MichelDiz Can we please get someone at Dgraph to acknowledge this problem!

We can’t secure our data and it is a simple fix!

J

7

Your use case fix may not work for all and would be a breaking change in some cases.

Example: a rule is set so a user can reassign their own tasks but they cannot reassign others tasks. This would be an example where a auth pre mutate would pass but auth post mutate would not pass.

So we need a way that with a post mutate could allow a user to unassign a task if assigned to them but not reassign it to somebody else not them. This is where a post mutate auth would be needed.

8

The rules to me are the same, as if I can allow someone to unassign a task, why can’t they create an un-assigned task in the first place. The security should be the same IMHO.

Also, I am not sure a non-administrative user should have the power to unassign a task, but that is neither here nor there…

Either way, you could add an OR ADD rule saying if the user: { eq: $USER } or user: { eq: 'unassigned' }… and eventually when they fix the null problem user: { eq: null }

But more importantly there is a HUGH security loophole here.

Please acknowledge this someone from Dgraph!

You have an unusable product without this fix! You cant use a database in production that is not secure. Please realize this!

J

9

@jdgamble555 We do acknowledge your concern and we are totally committed to fix all security loopholes. To start the discussion around this feature I would like to know more about your use case. As per my understanding, this issue could be easily solved with lambdas functionality. I would like to know more about your schema and what you are trying to achieve to understand better the use case and if there is any other way that can solve this issue. And if it seems necessary to build this feature, we will surely gonna implement this. You can DM me with details if it’s confidential or maybe we can connect over the call to discuss more.

1 Like
10

With respect, this is the most ignorant statement and shows you guys have not read any of my nor @amaster’s posts, nor do you actually understand the use cases of the average developer.

Here are the problems with this statement:

Lambdas

  1. Lambdas require using an external function in javascript / typescript
  2. Executing lambdas are not “easy” and take a lot of time to write code that should be built in to graphql for security.
  3. I can’t even use lambdas in a lot of cases. You guys do not offer a pre-mutation lambda. The post-mutation lambda “web-hook” that you do use is only viable for things like cascade delete and deep mutations (another feature that should be used in graphql, not lambdas).
  4. The post-mutation option that you offer does not have access to the changed data, so I cannot use a post-mutation to correct mutations I do not want to allow.
  5. Your argument for not having a pre-mutation is that it slows down the graphql. However, that is exactly what @custom muations do! They run before, which is what I need and should be included!
  6. For any real security, I have to use a @custom mutation. That means in order to have any real security for EVERY SINGLE mutation (add, delete, update), I have to disable usage on the adds, and create a custom mutation, not for each Type, but for each mutation type for each Type. Insane amounts of code.
  7. If I have to write a custom mutation for every single real security mutation and type, why do we even have graphql for? I am seriously better off using DQL on my own server. Please read that statement again, if you want us to use lambdas for everything, why do we even have graphql?
  8. Arguing with @MichelDiz about the usefulness of backend security is ridiculous, although he is the only person who even responds to my posts. You guys don’t seem to be actively developing graphql, which is a problem and will force your customers to find a better product.
  9. Every single one of your competitors gives you real security options outside of external nodejs functions: Firestore, Supabase, nhost, mongodb atlas, fauna, neo4j aura, neptune, mariadb, hasura cloud, tigergraph cloud, nebula graph, harperdb, couchbase… etc… etc… The list goes on and on. They each have their downfalls as well, but security is not one of them.

Use Cases

  • There are two specific use-cases here regarding the user, and another one I spoke about above in your documentation. You guys admitted this is a known security hole.

The Real Use Case and Why this is really important

Right now we can prevent certain certain types of thing from being added, but not changed later. When you add this feature, as I have spoken about in several other posts (including this one), it opens up a plethora of new features as I said at the beginning of this post (that again I assume no one is reading):

  • required fields
  • not null fields
  • field restrictions
  • value restrictions
  • min, max restraints
  • enum restructions
  • regex restrictions
  • this list goes on and on and on…

This opens the door for security rules. They are needed!

IN SUM

  • We are stuck with your graphql. Get rid of it and just be a cloud hosting platform, or start actively developing it.
  • Lambdas are NOT EASY and you guys expect us to fix every security problem with them while actively not giving us any pre-mutation lambdas. This forces us to have to write 3 @custom mutations for every single type to have any sort of backend security, rendering graphql pointless.
  • Every single one of your competitors has security outside of lambdas functions unless they are just cloud hosting and not a full blown platform.
  • I personally would have several more projects with you guys making you more money etc. as well as at least 5 other people I have seen on these forms saying this is a deal breaker. I don’t want to waste my time with something that is not actively repairing its biggest problems, GraphQL. You will lose several clients if you don’t both:
    • Actively work on fixing the many limitations with GraphQL (starting with security and basic database features like cascade delete and branching out to features like Nested Fields)
    • Let your members know you’re actively working on fixing the many limitations with GraphQL. Are you guys? There is no evidence of this except for old empty promises. I love your product, but this is the truth.

What are you guys? A database as a service, or a full blown platform? The way GraphQL works on your platform with no field security, foreign key constraints, error messages, nested filters, and no sign of improvement, you are just a DBaaS. If that is your goal, then all power to you, get rid of GraphQL.

So yes, please fix this problem. It will give us basic front end security without having to write a custom mutation for every single field for every single type of mutation. Right now, that is ridiculous and the only way to secure your product.

That is my use case.

J

4 Likes
11

I am the customer Dgraph has lost.

I was heavily invested in prototyping a Dgraph based GraphQL stack for my company to evaluate, but all the issues, and need to write untestable lambdas has led me to abandon dgraph and adopt the Neo4j GRAND stack.

It’s very disappointing given the marketing Dgraph has as being the only graph database with native GraphQL support when the Neo stack is far more fully fleshed out, without the security issues.

Perhaps more disappointing is the lack of traction that these user reported issues are getting with the core team, or worse, dismissive responses.

2 Likes
12

@jdgamble555 Looks like something the team said has brushed you the wrong way. I apologize for that.

We internally discussed this change you’re proposing. Your experience report is lacking in examples, so we pieced together roughly you’re asking. @aman-bansal has already offered to jump on a call to better understand this issue. But that hasn’t happened. So, this is my understanding so far.

My understanding is that you want the ACL to run a check after the update as well. This would prohibit say, someone changing the ownership of a task to someone else. While this might solve your problem, this would create a new problem for another user, who does want to transfer the ownership. They would now suddenly not be able to change ownership.

There might be a different solution , which is either achievable via lambdas, as @aman-bansal mentioned. Or, via creating some “immutable” fields – so that the ownership doesn’t change at all. We can help figure those out. We’ve been consistently working on improving how lambdas operate, so users have a lot of flexibility in how they do things.

Essentially you’re proposing a solution – which goes against the ethos of Experience Reports. The proposed solution that you have, it’s unclear to us how it solves this problem generally – or, why say a different solution won’t work. Best is to explain the problem well, and show why the alternative solution is not great.

1 Like
13

Hey @rcbevans ,

I’m curious why you chose GRAND stack over Dgraph Cloud? What are you able to do with GRAND stack, that you can’t do with Dgraph. Happy to jump on a call to understand it better, or of course, you can expand here.

14

One way to solve this would be to do a pre-update and a post-update auth rules. We can re-purpose the current update rule, which would run these two internally. This can then be generically applied to various problems, and provide flexibility.

Does that sound like something that would solve your problem, @jdgamble555 ?

2 Likes
15

@mrjn - Thank you for taking the time to look into this.

Yes, that is the same solution that has been proposed at the beginning of this post… just having a post-update rule separately. I can understand how there could be breaking changes otherwise. Creating this post-update auth rule would definitely fix this problem.

However if you guys want to point to lambdas to solve every single security problem this would not cover, you need to also create pre-hooks as you originally said you were. The only other way to solve problems is to use @custom mutation, which renders the original GraphQL Type pointless. This would pretty much cover all bases, as security is a must.

Again, thank you for looking into this.

Finally, and perhaps most importantly, I think I can speak for all of your forum users by asking that you update your 2021 Product Roadmap so we understand what you guys are actually working on. We understand staff and time are limited, and we understand you can’t read every post. We don’t understand when there is absolutely no communication about the future of Dgraph (at least on the forums). No one wants to invest time in something that may not be a good product in the future. I also suspect it will make @MichelDiz’s job easier. This is by far, the most frustrating problem we have.

Thanks!

J

2 Likes
16

Hey @jdgamble555 ,

Really appreciate the positive tone of your message. Even though we’re stretched thin on multiple fronts, we do really care about our community. And it honestly hurts to see when a user, like yourself, is upset.

We’re bringing in a really solid product person in a month, who can better prioritize these things – so the product roadmap, the decision making around these requests would be better handled than what they’re today.

For this request, I think the team can put this together in a week or so. So, they’ll reach out to have this in your hands soon.

Again, apologize for the initial experience! You’d see something come out soon to solve this problem.

4 Likes
17

Yes! This pre-update and post-update rules are what have been needed for the last year.

Here is this simplified.

Use case: I need to allow a 1) user to update his own profile, but 2) there is a role level field that I do not want to allow a non admin user to set themselves as an admin nor 3) allow a non user to change their own username. But 4) allow an admin to update all users without restroctions

This would need a combination of RBAC and ABAC rules.

type User @auth(
  update: { or: [
    { rule: "query ($USER: String!) { queryUser(filter: { username: { eq: $USER } }) { username } }" }
    { rule: "{$ROLE: { eq: \"ADMIN\" }}" }
  ] }
) {
  username: String! @id
  role: UserRole!
}
enum UserRole {
  USER
  ADMIN
}

This update rule handles #1 and #4 but not #2 or #3

For #2 and #3 we need update auth rules that check the new data from the mutation.

type User @auth(
  update: { or: [
    { rule: "query ($USER: String!) { queryUser(filter: { username: { eq: $USER } }) { username } }" }
    { rule: "{$ROLE: { eq: \"ADMIN\" }}" }
  ] }
  updateAfter: { or: [
    { rule: "query ($USER: String!) { queryUser(filter: { username: { eq: $USER } role: { eq: USER } }) { username } }" }
    { rule: "{$ROLE: { eq: \"ADMIN\" }}" }
  ] }
) {
  username: String! @id
  role: UserRole! @search
}
enum UserRole {
  USER
  ADMIN
}
2 Likes
18 
  updateAfter: { or: [
    { rule: """
      query ($USER: String!) {
        queryUser(filter: {
          username: { eq: $USER }, 
          { not: { has: role } }
        }) {
          username
        }
      }"""
    },
    { rule: "{$ROLE: { eq: \"ADMIN\" }}" }
  ] }

I think you could also just use { not: { has: role } } as a field restriction for field level auth restrictions. I assume this would work like add, which does not query current values (since there are none), even though technically you would probably have a value for role in the database.

That way no one could change values you don’t want them to.

This also wouldn’t interfere with the validation for no null values, since you will never really have a null value in the db.

J

19

Hi @mrjn

Firstly, Dgraph cloud was never an option due to lack of SOC 2 compliance. Neo4j offers AWS Cloudformation templates making it cheaper to stand up an instance in our own cloud.

Secondly, there are a lot of gaps in the Dgraph GraphQL functionality, such as issues with auth rules such as update after auth which make it a no-go for mutations out of the box, and other limitations such as lack of system generated timestamps for creation/update.

For all the issues I ran into the Dgraph team response has been to use lambdas, but there is currently no story for how to effectively develop, or debug lambdas. It’s therefore easier to instead develop a distinct front-end to the database, completely negating the benefit of Dgraph offering GraphQL in the first place.

Neo4j GraphQL library allows me to develop a node based GraphQL front-end which allows me to dip in and out of Cypher where needed, have full control over auth including built in filtering, binding, and ACLs whilst keeping a minimal number of calls to the database to resolve the required fields for the response.

Unfortunately, Cypher is also a defacto standard graph query language meaning there is vastly more content available to learn from. This matter is exacerbated by the thin content available in the DQL documentation.

I do see Dgraph as a very promising product, particularly for webscale multi-tenant scenarios and one day I can see myself revisting Dgraph, but for now the limitations/cost of adoption are just too high for me to be able to justify to my team.

5 Likes
20

@jdgamble555 i have started working on post update auth validation. I think things are going in good direction and I should be able to give you early access in next week or so. Anyways I will keep this thread updated.
If all goes good, it will be part of next release i.e. v21.09.1 :slight_smile: .

4 Likes
21

Thank you, @jdgamble555 for putting so much time into your pleas for better GraphQL support here. You do a great job of articulating not only the many reasons why those features (especially related to auth) are desperately needed, but also the frustration that comes with constantly being told by Dgraph, “just do it with DQL or lambdas”, or “you simple request for a basic feature offered by competitors isn’t reasonable”. Many of us feel it and I’m glad you were able to put it so eloquently.

2 Likes