@paf31 The check condition in our insert permissions (and in Postgres's RLS) is not an invariant like Postgres's table level constraints, it will only be checked during the operation.

and be the same across updates and inserts

You can have different conditions to check on insert and update (a use case is in #3536). This is also allowed in Postgres's RLS and as long as the implementation is straight forward, we should try to remain as close to RLS as possible.

The idea of check constraints as invariants doesn't quite work when we allow exists in conditions, because we can break the invariant by updating the joined table. Not sure if we want to consider that here

The check constraint will only be checked during the operation.

It's also possible for one user to make it impossible for another user in a different role to update a row, in a new way which was not possible before

I'm sorry I don't quite follow this. The admin role could also modify some column involved in the filter condition because of which filter condition would never satisfy.

@lexi-lambda

It’s possible that sometimes people want these things to be different, but it seems likely that, in the vast majority of cases, they want them all to be the same

The console already helps with this situation, it lets you copy a condition from other permissions. So you would only write these rules once if they are all the same (but yes you would still have to say 'same as ..' multiple times which I don't think can be avoided).

Do you think we can come up with a design that matches that logical model a little bit better? I like Phil’s idea about making these a property of the table rather than something that belongs to a particular operation.

I agree, our permissions API on the server is a bit low level (the console improves the DX by providing options like copying conditions etc), we can definitely add higher level abstractions at the server itself, maybe like Phil suggested, we can allow users to define conditions on a table and allow referring to these conditions by names when specifying permissions. However I feel that we can open a new RFC for this as

1. The scope of these improvements would affect all permissions, not just insert and update.
2. Console can help our users in cases where they have to specify the same constraint across insert and update permissions.

I just wanted to add my 2 cents around how I think about check contraints and permissions. This might not be completely related but seems relevant.

• Any property of an entity which should always hold true should be part of check constraints / put behind biz logic during mutations
• Any property of an entity that needs properties of the user making the request to be decided, go into permission rules.

We often think of using permissions as a way to enforce general data consistency but that should be an anti-pattern as I can always then cause data inconsistencies by making a request as an admin.
Permission rules should always be seen from a lens of user level protection and not a way to define properties of your entities. The same follows for role-level columns presets vs PG defaults as well.

Sorry if this is a dumb question, but is there a reason why we're not reusing Postgres' row level security directly then? I know this would require mapping our roles to Postgres roles, or something like that, but it seems simpler than implementing a mirror of its permission system. Is it missing something we want?

is there a reason why we're not reusing Postgres' row level security

Couple of reasons:

1. We want to make as few changes as possible to the underlying Postgres database - creating new roles, defining permissions and enabling RLS on the tables are fairly intrusive changes and requires privileges that most of users may not be comfortable giving them to graphql-engine. In fact, some of the managed Postgres vendors (Heroku) do not allow creating new roles.
2. We cannot do the kind of optimizations that we do with our subscriptions implementation if we use RLS.

Any thoughts on what the UI should look like for permissions? Right now it refers to "checks" even though those are currently really filters, but now we will have checks too :)

If an insert becomes an update via ON CONFLICT, do you think we should use the insert check or the update check?

If an insert becomes an update via ON CONFLICT, do you think we should use the insert check or the update check?

I believe the update check should kick in as technically the insert is now reduced to an update

Any thoughts on what the UI should look like for permissions? Right now it refers to "checks" even though those are currently really filters, but now we will have checks too :)

Seems like the row permissions section will need to be further split into 2 sections now, a "before check" and an "after check". I currently can't seem figure out what the messaging should be for the 2 sections to get away with minimal changes. Will give it some more thought and get back. Ideas are welcome of course :)

If an insert becomes an update via ON CONFLICT, do you think we should use the insert check or the update check?

@paf31 This is discussed in the 'Implementation' section of the RFC.

...The tricky part would be the behaviour when on_conflict is used:

1. When there is no conflict, the insert permission's check condition has to hold true on the inserted row.
2. When there is a conflict, the update should only happen if the row can be updated, i.e, the update permission's filter condition holds true and after the row is updated, the update permission's check condition has to hold true.

Seems like the row permissions section will need to be further split into 2 sections now, a "before check" and an "after check". I currently can't seem figure out what the messaging should be for the 2 sections to get away with minimal changes. Will give it some more thought and get back. Ideas are welcome of course :)

In that case, I will just carry on without implementing the console changes for now. It should be fine to merge this without UI support, since there are no breaking metadata changes. Perhaps if anyone needs this feature immediately, we can guide them on using the console to set these permissions? @0x777 @rikinsk

In the meantime, I will work on updates to the ON CONFLICT logic and the documentation.

Perhaps if anyone needs this feature immediately, we can guide them on using the console to set these permissions? @0x777 @rikinsk

@paf31 I really want to test it with your help. I can provide extensive feedback about the feature. As you may already know, I have suggested and gave a real-world example for it on #3536 also in this comment

The lack of this feature is a blocker at the moment, unfortunately. I really appreciate all of your kind efforts.

@yasinarik If you're able to build my branch then you should be able to use the API or the console to set update permissions on your table. There is a small example in the test suite in my branch:

If you're able to build my branch then you should be able to use the API or the console to set update permissions on your table. There is a small example in the test suite in my branch:

@paf31 Really sorry for bothering you too much Dear Freeman; however, I think I need a guide on this. I have never used Hasura without the web console (always used it with the UI). May I request a bit more help?

I've checked the example btw. How long will it take do you guess the update check feature to be released officially under the master branch?

@yasinarik One option is to export metadata from the settings page in the console, edit the JSON to add the check condition in the update permission object, and reimport the result.

The simpler way might be to use the create_update_permission API, which I've updated the documentation for in my PR (docs here). You can now specify a check value in the permissions object, which has the same JSON format as the filter property, namely either a boolean expression or the name of a session variable.

Please let me know if you need more help.

@paf31 Dear Phil, I couldn't manage to build your branch that includes the required changes. Actually I couldn't find the right resources to do that. Also, I can say that I don't know how to build your branch and deploy that version on, let's say, Heroku.

I understood how to export metadata and make changes then import it. It's OK.

Is This Feasible?

Apart from that, per my understanding, there seems to be no way to combine filter - check pairs in single update permission. With the new branch, there is only one possibility at the moment. All the before conditions are collected inside the filter property and all the after conditions are collected inside the check property.

I don't know if it makes sense to you guys; however, my idea is to combine the separately written before and after conditions. The current improvement is great, I have to say that I can't express how I am glad about it. ☺️

I just want to tell you that there are a lot of real-world scenarios that it is not appropriate to define all the before conditions in a single filter property and to define all the after conditions in a single check property. It can cause exceptions to occur.

I imagine defining the filter&check pairs separately so it will be truly possible to exactly point a logically complex situation out without any exceptions.

Let me illustrate:

"_or": [
  {
    "filter": "some condition",
    "check": "some condition",
  },
  {
    "filter": "some condition",
    "check": "some condition",
  },
  {
    "filter": "some condition",
    "check": "some condition",
  },
]

Extended Example:

{
  "_or": {
    [
      /// CASE 1:
      {
        "filter": {
          "_and": {
            [
              {
                "id_Doer": "x-hasura-id",
              },
              {
                "followState": "false",
              }
            ],
          }
        },
        "check": {
          "followState": "requested",
        },
      },

      /// END OF CASE 1 ----
      /// -----------------------

      /// CASE 2:
      {
        "filter": {
          "_and": {
            [
              {
                "id_Doer": "x-hasura-id",
              },
              {
                "followState": "requested",
              }
            ],
          }
        },
        "check": {
          "followState": "false",
        },
      },

      /// END OF CASE 2
      /// -----------------------

      /// CASE 3:
      {
        "filter": {
          "_and": {
            [
              {
                "id_Doer": "x-hasura-id",
              },
              {
                "followState": "true",
              }
            ],
          }
        },
        "check": {
          "followState": "false",
        },
      },

      /// END OF CASE 3 ----
      /// -----------------------

      /// CASE 4:
      {
        "filter": {
          "_and": {
            [
              {
                "id_Target": "x-hasura-id",
              },
              {
                "followState": "true",
              }
            ],
          }
        },
        "check": {
          "followState": "false",
        },
      },

      /// END OF CASE 4 ----
      /// -----------------------

      /// CASE 5:
      {
        "filter": {
          "_and": {
            [
              {
                "id_Target": "x-hasura-id",
              },
              {
                "followState": "req",
              }
            ],
          }
        },
        "check": {
          "_or": {
            [
              {
                "followState": "true",
              },
              {
                "followState": "false",
              }
            ],
          },
        },
      },
      /// END OF CASE 5 ----
      /// -----------------------
    ],
  },
};

/// 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.
///
/// 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.
///
/// CASE 3:
/// id_Doer is the logged in user.
/// If followState is "true",
/// it can only become "false".
/// MEANING: userDoer unfollows userTarget.
///
/// CASE 4:
/// id_Target is the logged in user.
/// If followState is "true",
/// it can only become "false".
/// MEANING: userTarget can remove userDoer from its followers.
///
/// CASE 5:
/// id_Target is the logged in user.
/// If followState is "req",
/// it can become "true" or "false".
/// MEANING: userTarget can accept or decline the follow request of userDoer.

@0x777 @rikinsk @lexi-lambda

Review app https://hge-ci-pull-3750.herokuapp.com is deleted

Deploy preview for hasura-docs ready!

Built with commit 52e2ea8

https://deploy-preview-3750--hasura-docs.netlify.com

@yasinarik

I just want to tell you that there are a lot of real-world scenarios that it is not appropriate to define all the before conditions in a single filter property and to define all the after conditions in a single check property. It can cause exceptions to occur.

I definitely agree with this.

I imagine defining the filter&check pairs separately so it will be truly possible to exactly point a logically complex situation out without any exceptions.

I'm not sure what the exact interface should be, but I think the biggest limitation currently in the check expression is that you cannot use the values of columns from before the update (old value of a column).

Adding the ability to use 'old' values of a column in check expression is not easy to implement, we'll need to 1. come up with a meaningful syntax and 2. map to appropriate postgres sql statements (this is particularly hard because of upserts)

About your use case here, wouldn't the recommendation in #3536 (comment) work?

One idea could be to use a list of (filter, check) expressions per role (or use multiple roles with single (filter,check)). This would be related to the write version of this RFC.

But, that would be outside the scope of this discussion.