server: limit of an inherited role will be the max of the limit of th…

…e limits of the roles GitOrigin-RevId: 1373f80
etrepum · Apr 21, 2021 · 7be8003 · 7be8003
1 parent 054a41d
commit 7be8003
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@ only when there are enough present in the items inventory.
 - server: fix bug with catalog upgrade from alpha.7 (fix #6802)
 - server: fix a bug in remote schema permissions that could result in an invalid GraphQL schema (fix #6029, #6703)
 - server: support query multiplexing in MSSQL subscriptions
+- server: an inherited role's limit will be the max limit of all the roles (#6671)
 - console: add bigquery support (#1000)
 - cli: add support for bigquery in metadata operations
 - cli: fix regression - `metadata apply —dry-run` was overwriting local metadata files with metadata on server when it should just display the differences.

diff --git a/docs/graphql/core/auth/authorization/inherited-roles.rst b/docs/graphql/core/auth/authorization/inherited-roles.rst
@@ -124,7 +124,7 @@ The above GraphQL query will be translated to the following SQL query.
 
 The other parameters of the select permission will be combined in the following manner:
 
-1. Limit - Minimum of the limits will be the limit of the inherited role
+1. Limit - Maximum of the limits will be the limit of the inherited role
 2. Allow aggregations - If any of the role allows aggregation, then the inherited role will allow aggregation
 3. Scalar computed fields - same as table column fields, as in the above example
 

diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache/Permission.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache/Permission.hs
@@ -84,7 +84,7 @@ data CombinedSelPermInfo (b :: BackendType)
 --      nullable to accomodate cell-value nullification.
 --   2. Scalar computed fields - Scalar computed fields work the same as Columns (#1)
 --   3. Filter / Boolean expression - The filters are combined using a `BoolOr`
---   4. Limit - Limits are combined by taking the minimum of the two limits
+--   4. Limit - Limits are combined by taking the maximum of the two limits
 --   5. Allow Aggregation - Aggregation is allowed, if any of the permissions allow it.
 --   6. Request Headers - Request headers are concatenated
 --
@@ -136,7 +136,7 @@ combineSelectPermInfos selPermInfos@(headSelPermInfo NE.:| restSelPermInfos) =
             (Nothing, Nothing) -> Nothing
             (Just l, Nothing)  -> Just l
             (Nothing, Just r)  -> Just r
-            (Just l , Just r)  -> Just $ min l r
+            (Just l , Just r)  -> Just $ max l r
       , cspiAllowAgg = cspiAllowAgg lSelPermInfo || cspiAllowAgg accSelPermInfo
       , cspiRequiredHeaders = (cspiRequiredHeaders lSelPermInfo) <> (cspiRequiredHeaders accSelPermInfo)
       }

diff --git a/server/tests-py/queries/graphql_query/permissions/inherited_roles/basic_inherited_roles.yaml b/server/tests-py/queries/graphql_query/permissions/inherited_roles/basic_inherited_roles.yaml
@@ -187,3 +187,26 @@
           name
         }
       }
+
+- description: "The limits of the role should be combined to be the max of the limits of the rows. The
+  'non_paying_user' inherited role is created out of the 'free_user' role which has limit set to 2
+  and the 'premium_user_free_trial' role which has limit set to 4. So 4 rows are expected to be returned
+  for the inherited role."
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: non_paying_user
+  response:
+    data:
+      articles:
+        - title: title 2
+        - title: title 3
+        - title: title 4
+        - title: title 5
+  query:
+    query: |
+      query {
+        articles {
+          title
+        }
+      }
diff --git a/server/tests-py/queries/graphql_query/permissions/inherited_roles/setup.yaml b/server/tests-py/queries/graphql_query/permissions/inherited_roles/setup.yaml
@@ -164,10 +164,43 @@ args:
       columns: "*"
       filter: {}
 
+- type: pg_create_select_permission
+  args:
+    table: articles
+    role: free_user
+    permission:
+      columns:
+        - title
+        - content
+      filter:
+        is_published:
+          _eq: true
+      limit: 2
+
+- type: pg_create_select_permission
+  args:
+    table: articles
+    role: premium_user_free_trial
+    permission:
+      columns:
+        - title
+        - content
+      filter:
+        is_published:
+          _eq: true
+      limit: 4
+
 - type: add_inherited_role
   args:
     role_name: author_editor_guest_inherited_role
     role_set:
       - author
       - editor
       - guest
+
+- type: add_inherited_role
+  args:
+    role_name: non_paying_user
+    role_set:
+      - free_user
+      - premium_user_free_trial
diff --git a/server/tests-py/queries/graphql_query/permissions/inherited_roles/teardown.yaml b/server/tests-py/queries/graphql_query/permissions/inherited_roles/teardown.yaml
@@ -7,3 +7,7 @@ args:
   - type: drop_inherited_role
     args:
       role_name: author_editor_guest_inherited_role
+
+  - type: drop_inherited_role
+    args:
+      role_name: non_paying_user