hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.18k stars 2.77k forks source link

Add Permission Rules For Checking the New (or Incoming, or Next) Value of a Field #3536

Closed yasinarik closed 3 years ago

yasinarik commented 4 years ago

Problem:

Currently, it is possible to add row permission rules for checking the existing value of a column field.

However, per my research, it's not possible to add control logics into row permission rules for checking the incoming (or new, or next) value of a field. In other words, this will ultimately result in a custom check constraint that is applied BEFORE a mutation.

This could be related to "actions" feature that is still under progress. If so, when will the actions become ready to use?

To illustrate:

This is a one way relationship table. I took my time and tried to represent the exact same situation that we are going to use in production.

Let the column fields be: (Simplified)

id_Doer   |   id_Target   |   followState

followState is a String field and may have 3 values => "false", "requested", and "true".

Let UserTarget has a private profile. So any other user can only request following UserTarget or if they already requested, they can cancel request.

So here is an example situation: (Simplified)

Code Block 1: UserA is the logged in user who is trying to perform mutations on the same row.

_and: [
 id_Doer = X-Hasura-Id, (UserDoer is the logged in user. I can do this, no problem.),
 UserTarget = private,  (I can also check this since there are array & object relations. No problem)
    _or: [
      _and: [
       existing_value_of_followState = "false",  (UserDoer not yet requested)
       incoming_value_of_followState = "requested",    (UserDoer can request following)
     ],

      _and: [
       existing_value_of_followState = "requested",  (If UserDoer requested following)
       incoming_value_of_followState = "false",    (UserTarget can cancel the request)
     ], 

      _and: [
       existing_value_of_followState = "true",  (If UserDoer is following UserTarget)
       incoming_value_of_followState = "false",    (UserDoer can unfollow UserTarget)
     ],  
 ],

Code Block 2: UserB is the logged in user who is trying to perform mutations on the same row.

_and: [
 id_Target = X-Hasura-Id, (UserTarget is the logged in user. I can do this, no problem.),
 UserTarget = private,  (I can also check this since there are array & object relations. No problem)

    _or: [ 

      _and: [
      existing_value_of_followState = "requested",  (Request already made by UserDoer before)
      incoming_value_of_followState = "false",    (UserTarget can decline the request)
     ],

      _and: [
      existing_value_of_followState = "requested",  (Request already made by UserDoer before)
      incoming_value_of_followState = "true",    (UserTarget can accept the request)
     ],

      _and: [
      existing_value_of_followState = "true",  (UserDoer already following UserTarget)
      incoming_value_of_followState = "false", (UserTarget can remove UserDoer from its followers)
     ],
 ],

What does it mean :

But... I couldn't find a way to apply rules on the incoming (or new, or next) value of a field.

Is it a breaking change or very hard to implement? This can save a lot of complex stuff, really.

I'm in love with Hasura and I really think this feature should be added.

PS: I my not know if there is already a solution **exactly

** for what I'm trying to do. If so, sorry for the hustle 😄

User Relationship Diagram
tirumaraiselvan commented 4 years ago

Related to #384 , dupe?

yasinarik commented 4 years ago

Related to #384 , dupe?

Hi @tirumaraiselvan , how are you? I hope you are having a great day.

It seems related with #384 , but I have wrote a more definitive and clear problem-suggestion pair, to be honest. Also, the above example is intended for production use. Herein, the example clearly shows that these kind of rule can be applied to any field possible.

So, I think the above example will help people to think that this isn't solely related to input (incoming) values from the headers such as X-Hasura-Id.

Indeed, the suggested permission rule of being able to check the existing values and input values at the same time, covers & includes the values coming from both the headers and the body.

With suggestion: You add a check so that the user_id field after modification must still equal X-HASURA-USER-ID, and everything works fine.

Differently from the above quote, I've suggested applying these rules 'before' any modification. The permission rules are already being applied 'before' in the current version of Hasura.

Moreover, the rules I suggested and the problems I'm facing are not related with exposing or not exposing certain column fields of a table.

Lastly, per available in the documentations, having checked the new 'actions' feature out, I've understood that the actions feature will be a very extended logic controller and will let us control the behavior extensively. I can't wait to test it out.

However, anyways I think that accessing the input values directly from permission rules tab is relatively easier for us to use, and easier for Hasura team to implement.

yasinarik commented 4 years ago

Similar to #1700 .

Will you implement my suggestion soon? @tirumaraiselvan @marionschleifer

tirumaraiselvan commented 4 years ago

@yasinarik Yes, this is something that we will be keen to do.

yasinarik commented 4 years ago

Asking this is sounds weird to me too but I have an urge to ask. :)

Are there any estimations on when this feature will be added? What should I understand from the labels?

This feature is super important for us to be able to use Hasura in production for a global scaled app. For the last 4 months, we haven't faced any other blocker.

We really admire and appreciate the effort of the Hasura Team. So we can't wait to make the returning of the value you add our business.

@0x777 @tirumaraiselvan

tirumaraiselvan commented 4 years ago

@paf31 @0x777 Could you pls post your thoughts here on the technical feasibility of this feature?

Also, instead of this exact interface, is there something else we can build so that the use-case of checking "before" and "after" values can be modelled by the permission system?

0x777 commented 4 years ago

Hi @yasinarik, please take a look at https://github.com/hasura/graphql-engine/pull/3750, we are planning to allow adding a check condition on the updated row (the update will fail if this condition does not meet this condition). Once we have that, your requirement can be represented as follows:

  1. The filter condition (the rows that can be updated) will be as follows:

    {
     "_or": [
       { "id_Doer": "x-hasura-id"},
       { "id_Target": "x-hasura-id"}
     ]
    }

    i.e, you can update only those rows where you are the 'doer' or the 'target'.

  2. The allowed columns to be updated are only followState.

  3. The row which has been updated has to hold the following condition (check):

    {
     "_or": [
       {
         "id_Doer" : "x-hasura-id",
         "followState": {"_in": ["false", "requested"]}
       },
       {
         "id_Target" : "x-hasura-id",
         "followState": {"_in": ["false", "true"]}
       }
     ]
    }

    The above condition states that, after the update,

    1. If the user is a 'doer', the followState can only be set to false or requested, i.e, the 'doer' can only request to follow or stop following the 'target'.
    2. If the user is a 'target', the followState can only be set to false or true, i.e, the 'target' can only approve or reject a request.

Note that once a 'doer' sets the followState to false, the target shouldn't be able to set it to true, so the filter has to be tightened as follows:

{
  "_or": [
    { "id_Doer": "x-hasura-id"},
    { 
      "id_Target": "x-hasura-id", 
      "followState": { "_in": ["true", "requested"] }
    }
  ]
}

This still allows one transition which you may not want to allow: a 'doer' can change the followState from true to requested, but that is a transition that can anyways be achieved as follows: true to false to requested.

yasinarik commented 4 years ago

Hi @0x777 ! How are you? Thank you for letting me know, I've just seen your comment. I will write back to you soon once I read, understand, and test those.

yasinarik commented 4 years ago

@0x777 OK! I have carefully followed your comments here and there. Yes! The behavior should be like that.

This still allows one transition which you may not want to allow: a 'doer' can change the followState from true to requested, but that is a transition that can anyways be achieved as follows: true to false to requested.

I believe even this part will be satisfied. Per your example, here is what I've always thought:

Example Situation: userTarget has a private profile. Only followState and timeStamp etc. columns can be updated.

{
  /// Must satisfy -> CASE 1 or CASE 2 or CASE 3
  "_or": {
    [
      /// CASE 1:
      /// id_Doer is the logged in user.
      /// If followState is "false",
      /// it can only become "requested".
      /// MEANING: userDoer sends a follow request to userTarget.
      {
        "_and": {
          [
            {
              "filter": {
                "_and": {
                  [
                    {
                      "id_Doer": "x-hasura-id",
                    },
                    {
                      "followState": "false",
                    }
                  ],
                }
              },
            },
            {
              "check": {
                "followState": "requested",
              },
            },
          ],
        },
      },

      /// END OF CASE 1 ----

      /// CASE 2:
      /// id_Doer is the logged in user.
      /// If followState is "requested",
      /// it can only become "false".
      /// MEANING: userDoer cancels the awaiting follow request sent to userTarget.
      {
        "_and": {
          [
            {
              "filter": {
                "_and": {
                  [
                    {
                      "id_Doer": "x-hasura-id",
                    },
                    {
                      "followState": "requested",
                    }
                  ],
                }
              },
            },
            {
              "check": {
                "followState": "false",
              },
            },
          ],
        },
      },

      /// END OF CASE 2 ----

      /// CASE 3:
      /// id_Doer is the logged in user.
      /// If followState is "true",
      /// it can only become "false".
      /// MEANING: userDoer unfollows userTarget.
      {
        "_and": {
          [
            {
              "filter": {
                "_and": {
                  [
                    {
                      "id_Doer": "x-hasura-id",
                    },
                    {
                      "followState": "true",
                    }
                  ],
                }
              },
            },
            {
              "check": {
                "followState": "false",
              },
            },
          ],
        },
      },
      /// END OF CASE 3 ----
    ],
  },

}

How will the actual implementation of these look like on the Hasura panel? I mean, on the user side. As far as I understand, filter will be used for the current values of the rows and check will be used for updated values of the rows. However, the update is pseudo until it satisfies the check conditions. I just don't understand how we can apply the filter and check. Via the dropdown selector?

Btw, it would be even greater if we can write the rules as JSON format instead of the dropdown selector. Because when I miss something or make a little mistake, I have to start over.

EDIT: According to your new response, I will write all the rules including every situation down in JSON format.

0x777 commented 4 years ago

The check feature in update permissions is not yet implemented. But it'll be added very soon.

Btw, it would be even greater if we can write the rules as JSON format instead of the dropdown selector. Because when I miss something or make a little mistake, I have to start over.

You can always export the metadata, edit it as a json file and import it.

yasinarik commented 4 years ago

The check feature in update permissions is not yet implemented. But it'll be added very soon.

Btw, it would be even greater if we can write the rules as JSON format instead of the dropdown selector. Because when I miss something or make a little mistake, I have to start over.

You can always export the metadata, edit it as a json file and import it.

Oh well great to know that. I can't wait to use the check feature and am really thankful for your effort. @0x777

tirumaraiselvan commented 4 years ago

check has been implemented in v1.2.0.beta.1 onwards and it will be available in the console in v1.2 stable as well via https://github.com/hasura/graphql-engine/pull/4313

0x777 commented 3 years ago

Closing this as the necessary check feature in update permissions that is required to model these conditions is implemented for update mutations from v1.2.