server/bugfix: Include permission filter in the exists clause. Fixes #6931

GitOrigin-RevId: d3080dfa00c96afcf1254d83757a5e50a0726381
2024-09-20 06:58:39 +03:00 · 2021-05-24 10:08:52 +01:00 · 2021-05-24 10:08:52 +01:00 · c9e7e10eaa
commit c9e7e10eaa
parent 63594e1828
7 changed files with 107 additions and 16 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -15,6 +15,7 @@
 - server: MSSQL: Support _lt, _eq, etc. for text/ntext types.
 - server: MSSQL: Fix offset when there's no order by.
 - server: MSSQL: Support booleans better.
+- server: Include permission filter in the exists clause (fix #6931)
 - server: add support for adding multi-column foreign key relationships
 - server: fix a bug where `@skip` and `@include` were not allowed on the same field
 - server: properly reject queries containing unknown or misplaced directives
--- a/server/src-lib/Hasura/GraphQL/Schema/BoolExp.hs
+++ b/server/src-lib/Hasura/GraphQL/Schema/BoolExp.hs
@ -17,6 +17,7 @@ import           Hasura.GraphQL.Parser         (InputFieldsParser, Kind (..), Pa
                                                UnpreparedValue)
 import           Hasura.GraphQL.Parser.Class
 import           Hasura.GraphQL.Schema.Backend
+import           Hasura.GraphQL.Schema.Common  (partialSQLExpToUnpreparedValue)
 import           Hasura.GraphQL.Schema.Table
 import           Hasura.RQL.Types

@ -76,7 +77,10 @@ boolExp sourceName tableInfo selectPermissions = memoizeOn 'boolExp (sourceName,
        FIRelationship relationshipInfo -> do
          remoteTableInfo <- askTableInfo sourceName $ riRTable relationshipInfo
          remotePermissions <- lift $ tableSelectPermissions remoteTableInfo
-          lift $ fmap (AVRel relationshipInfo) <$> boolExp sourceName remoteTableInfo remotePermissions
+          let remoteTableFilter = fmapAnnBoolExp partialSQLExpToUnpreparedValue $
+                                  maybe annBoolExpTrue spiFilter remotePermissions
+          remoteBoolExp <- lift $ boolExp sourceName remoteTableInfo remotePermissions
+          pure $ fmap (AVRel relationshipInfo . andAnnBoolExps remoteTableFilter) remoteBoolExp

        -- Using computed fields in boolean expressions is not currently supported.
        FIComputedField _ -> empty
--- a/server/tests-py/queries/explain/permissions_query_mssql.yaml
+++ b/server/tests-py/queries/explain/permissions_query_mssql.yaml
@ -3,21 +3,17 @@ url: /v1/graphql/explain
 status: 200
 response:
  - field: user
-    sql: |-
-      SELECT ISNULL((SELECT [t_user1].[id] AS [id],
-             [t_user1].[name] AS [name],
-             [t_user1].[age] AS [age]
-      FROM [dbo].[user] AS [t_user1]
-      OUTER APPLY (SELECT *
-                   FROM OPENJSON((N''+NCHAR(123)+''+NCHAR(34)+'positionalArguments'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(91)+''+NCHAR(93)+','+NCHAR(34)+'namedArguments'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(123)+''+NCHAR(125)+','+NCHAR(34)+'session'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(123)+''+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(34)+'1'+NCHAR(34)+''+NCHAR(125)+''+NCHAR(125)+''))
-                        WITH ([session] NVARCHAR(MAX) AS JSON,
-                             [namedArguments] NVARCHAR(MAX) AS JSON,
-                             [positionalArguments] NVARCHAR(MAX) AS JSON) AS [row]) 
-      AS [row]
-      WHERE ((((([t_user1].[id]) = (JSON_VALUE([row].[session], (N''+NCHAR(36)+'.'+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''))))
-             OR ((([t_user1].[id]) IS NULL)
-            AND ((JSON_VALUE([row].[session], (N''+NCHAR(36)+'.'+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''))) IS NULL)))))
-      FOR JSON PATH), '[]')
+    sql:
+      "SELECT ISNULL((SELECT [t_user1].[id] AS [id],\n       [t_user1].[name] AS\
+      \ [name],\n       [t_user1].[age] AS [age]\nFROM [dbo].[user] AS [t_user1]\nOUTER\
+      \ APPLY (SELECT *\n             FROM OPENJSON((N''+NCHAR(123)+''+NCHAR(34)+'positionalArguments'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(91)+''+NCHAR(93)+','+NCHAR(34)+'namedArguments'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(123)+''+NCHAR(125)+','+NCHAR(34)+'session'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(123)+''+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''+NCHAR(58)+''+NCHAR(34)+'1'+NCHAR(34)+''+NCHAR(125)+''+NCHAR(125)+''))\n\
+      \                  WITH ([session] NVARCHAR(MAX) AS JSON,\n                  \
+      \     [namedArguments] NVARCHAR(MAX) AS JSON,\n                       [positionalArguments]\
+      \ NVARCHAR(MAX) AS JSON) AS [row]\n             WHERE (1=1)) \nAS [row]\nWHERE\
+      \ ((((([t_user1].[id]) = (JSON_VALUE([row].[session], (N''+NCHAR(36)+'.'+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''))))\n\
+      \      OR ((([t_user1].[id]) IS NULL)\n      AND ((JSON_VALUE([row].[session],\
+      \ (N''+NCHAR(36)+'.'+NCHAR(34)+'x-hasura-user-id'+NCHAR(34)+''))) IS NULL)))))\n\
+      FOR JSON PATH), '[]')"
 query:
  user:
    X-Hasura-Role: user
--- a/server/tests-py/queries/graphql_query/boolexp/basic/select_author_article_where_permissions.yaml
+++ b/server/tests-py/queries/graphql_query/boolexp/basic/select_author_article_where_permissions.yaml
@ -0,0 +1,37 @@
+# Test case for bug reported at https://github.com/hasura/graphql-engine/issues/6931
+- description: Select author and their articles as an admin
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      author:
+        - name: Author 1
+        - name: Author 2
+        - name: Author 3
+  headers:
+    X-Hasura-Role: admin
+  query:
+    query: |
+      query {
+        author(where: {articles: {}}) {
+          name
+        }
+      }
+
+- description: Select author and their articles as a user
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      author:
+        - name: Author 1
+        - name: Author 3
+  headers:
+    X-Hasura-Role: user
+  query:
+    query: |
+      query {
+        author(where: {articles: {}}) {
+          name
+        }
+      }
--- a/server/tests-py/queries/graphql_query/boolexp/basic/setup.yaml
+++ b/server/tests-py/queries/graphql_query/boolexp/basic/setup.yaml
@ -258,5 +258,29 @@ args:
            id: X-Hasura-User-Id
            is_admin: true

+- type: create_select_permission
+  args:
+    table: article
+    role: user
+    permission:
+      columns:
+      - author_id
+      - content
+      - id
+      - title
+      filter:
+        is_published:
+          _eq: true
+
+- type: create_select_permission
+  args:
+    table: author
+    role: user
+    permission:
+      columns:
+      - id
+      - name
+      filter: {}
+
 - type: track_table
  args: table_with_sql_identifier
--- a/server/tests-py/queries/graphql_query/boolexp/basic/setup_mssql.yaml
+++ b/server/tests-py/queries/graphql_query/boolexp/basic/setup_mssql.yaml
@ -98,3 +98,29 @@ args:
      foreign_key_constraint_on:
        table: message
        column: parent_id
+
+- type: mssql_create_select_permission
+  args:
+    source: mssql
+    table: article
+    role: user
+    permission:
+      columns:
+      - author_id
+      - content
+      - id
+      - title
+      filter:
+        is_published:
+          _eq: 1
+
+- type: mssql_create_select_permission
+  args:
+    source: mssql
+    table: author
+    role: user
+    permission:
+      columns:
+      - id
+      - name
+      filter: {}
--- a/server/tests-py/test_graphql_queries.py
+++ b/server/tests-py/test_graphql_queries.py
@ -423,6 +423,9 @@ class TestGraphQLQueryBoolExpBasicCommon:
    def test_author_article_where_nin(self, hge_ctx, transport):
        check_query_f(hge_ctx, self.dir() + '/select_author_article_where_nin.yaml', transport)

+    def test_author_article_where_permissions(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/select_author_article_where_permissions.yaml', transport)
+
    @classmethod
    def dir(cls):
        return 'queries/graphql_query/boolexp/basic'