mssql: Test upserts with permissions

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3280
GitOrigin-RevId: 861d4db59f75c83daef975fb3471b3c07889c9bc

server/src-lib/Hasura/Backends/MSSQL/Schema/IfMatched.hs

@@ -72,7 +72,7 @@ ifMatchedObjectParser sourceName tableInfo maybeSelectPerms maybeUpdatePerms = r
   selectPerms <- hoistMaybe maybeSelectPerms
   updatePerms <- hoistMaybe maybeUpdatePerms
   matchColumnsEnum <- MaybeT $ tableInsertMatchColumnsEnum sourceName tableInfo selectPerms
-  updateColumnsEnum <- MaybeT $ tableUpdateColumnsEnum tableInfo updatePerms
+  updateColumnsEnum <- lift $ updateColumnsPlaceholderParser tableInfo updatePerms
 
   -- The style of the above code gives me some cognitive dissonance: We could
   -- push the @hoistMaybe@ checks further away to callers, but not the enum
@@ -98,7 +98,10 @@ ifMatchedObjectParser sourceName tableInfo maybeSelectPerms maybeUpdatePerms = r
       _imMatchColumns <-
         P.fieldWithDefault matchColumnsName Nothing (G.VList []) (P.list matchColumnsEnum)
       _imUpdateColumns <-
-        P.fieldWithDefault updateColumnsName Nothing (G.VList []) (P.list updateColumnsEnum)
+        P.fieldWithDefault updateColumnsName Nothing (G.VList []) (P.list updateColumnsEnum) `P.bindFields` \cs ->
+          -- this can only happen if the placeholder was used
+          sequenceA cs `onNothing` parseError "erroneous column name"
+
       pure $ IfMatched {..}
 
 -- | Table insert_match_columns enum

server/src-lib/Hasura/Backends/Postgres/Schema/OnConflict.hs

@@ -65,7 +65,7 @@ conflictObjectParser ::
   UpdPermInfo ('Postgres pgKind) ->
   m (Parser 'Input n (IR.OnConflictClause ('Postgres pgKind) (UnpreparedValue ('Postgres pgKind))))
 conflictObjectParser sourceName tableInfo constraints selectPerms updatePerms = do
-  updateColumnsEnum <- updateColumnsPlaceholderParser
+  updateColumnsEnum <- updateColumnsPlaceholderParser tableInfo updatePerms
   constraintParser <- conflictConstraint constraints sourceName tableInfo
   whereExpParser <- boolExp sourceName tableInfo selectPerms
   tableGQLName <- getTableGQLName tableInfo
@@ -93,24 +93,6 @@ conflictObjectParser sourceName tableInfo constraints selectPerms updatePerms =
   where
     tableName = tableInfoName tableInfo
 
-    -- If there's no column for which the current user has "update"
-    -- permissions, this functions returns an enum that only contains a
-    -- placeholder, so as to still allow this type to exist in the schema.
-    updateColumnsPlaceholderParser :: m (Parser 'Both n (Maybe (Column ('Postgres pgKind))))
-    updateColumnsPlaceholderParser = do
-      maybeEnum <- tableUpdateColumnsEnum tableInfo updatePerms
-      case maybeEnum of
-        Just e -> pure $ Just <$> e
-        Nothing -> do
-          tableGQLName <- getTableGQLName tableInfo
-          enumName <- P.mkTypename $ tableGQLName <> $$(G.litName "_update_column")
-          pure $
-            P.enum enumName (Just $ G.Description $ "placeholder for update columns of table " <> tableName <<> " (current role has no relevant permissions)") $
-              pure
-                ( P.Definition @P.EnumValueInfo $$(G.litName "_PLACEHOLDER") (Just $ G.Description "placeholder (do not use)") P.EnumValueInfo,
-                  Nothing
-                )
-
 -- | Constructs a Parser for the name of the constraints on a given table.
 --
 -- The TableCoreInfo of a given table contains a list of unique or primary key

server/src-lib/Hasura/GraphQL/Schema/Table.hs

@@ -3,6 +3,7 @@ module Hasura.GraphQL.Schema.Table
   ( getTableGQLName,
     tableSelectColumnsEnum,
     tableUpdateColumnsEnum,
+    updateColumnsPlaceholderParser,
     tablePermissions,
     tableSelectPermissions,
     tableSelectFields,
@@ -106,6 +107,28 @@ tableUpdateColumnsEnum tableInfo updatePermissions = do
   where
     define name = P.Definition name (Just $ G.Description "column name") P.EnumValueInfo
 
+-- If there's no column for which the current user has "update"
+-- permissions, this functions returns an enum that only contains a
+-- placeholder, so as to still allow this type to exist in the schema.
+updateColumnsPlaceholderParser ::
+  MonadBuildSchema backend r m n =>
+  TableInfo backend ->
+  UpdPermInfo backend ->
+  m (Parser 'Both n (Maybe (Column backend)))
+updateColumnsPlaceholderParser tableInfo updatePerms = do
+  maybeEnum <- tableUpdateColumnsEnum tableInfo updatePerms
+  case maybeEnum of
+    Just e -> pure $ Just <$> e
+    Nothing -> do
+      tableGQLName <- getTableGQLName tableInfo
+      enumName <- P.mkTypename $ tableGQLName <> $$(G.litName "_update_column")
+      pure $
+        P.enum enumName (Just $ G.Description $ "placeholder for update columns of table " <> tableInfoName tableInfo <<> " (current role has no relevant permissions)") $
+          pure
+            ( P.Definition @P.EnumValueInfo $$(G.litName "_PLACEHOLDER") (Just $ G.Description "placeholder (do not use)") P.EnumValueInfo,
+              Nothing
+            )
+
 tablePermissions ::
   forall m n r b.
   (Backend b, MonadSchema n m, MonadRole r m) =>

server/src-lib/Hasura/RQL/IR/BoolExp.hs

@@ -195,6 +195,8 @@ data PartialSQLExp (b :: BackendType)
 
 deriving instance (Backend b) => Eq (PartialSQLExp b)
 
+deriving instance (Backend b) => Show (PartialSQLExp b)
+
 instance (Backend b, NFData (BooleanOperators b (PartialSQLExp b))) => NFData (PartialSQLExp b)
 
 instance (Backend b, Hashable (BooleanOperators b (PartialSQLExp b))) => Hashable (PartialSQLExp b)

server/src-lib/Hasura/RQL/Types/Table.hs

@@ -276,6 +276,12 @@ deriving instance
   ) =>
   Eq (InsPermInfo b)
 
+deriving instance
+  ( Backend b,
+    Show (BooleanOperators b (PartialSQLExp b))
+  ) =>
+  Show (InsPermInfo b)
+
 instance
   ( Backend b,
     NFData (BooleanOperators b (PartialSQLExp b))
@@ -383,6 +389,12 @@ deriving instance
   ) =>
   Eq (SelPermInfo b)
 
+deriving instance
+  ( Backend b,
+    Show (BooleanOperators b (PartialSQLExp b))
+  ) =>
+  Show (SelPermInfo b)
+
 instance
   ( Backend b,
     NFData (BooleanOperators b (PartialSQLExp b))
@@ -420,6 +432,12 @@ deriving instance
   ) =>
   Eq (UpdPermInfo b)
 
+deriving instance
+  ( Backend b,
+    Show (BooleanOperators b (PartialSQLExp b))
+  ) =>
+  Show (UpdPermInfo b)
+
 instance
   ( Backend b,
     NFData (BooleanOperators b (PartialSQLExp b))
@@ -454,6 +472,12 @@ deriving instance
   ) =>
   Eq (DelPermInfo b)
 
+deriving instance
+  ( Backend b,
+    Show (BooleanOperators b (PartialSQLExp b))
+  ) =>
+  Show (DelPermInfo b)
+
 instance
   ( Backend b,
     NFData (BooleanOperators b (PartialSQLExp b))

server/tests-py/queries/graphql_mutation/insert/permissions/article_if_matched_restricted_role_mssql.yaml

@@ -0,0 +1,74 @@
+- description: Upserts article data via GraphQL mutation as Restricted role (no column in update permissions)
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: restricted
+    X-Hasura-User-Id: '1'
+  response:
+    data:
+      insert_article:
+        affected_rows: 0
+        returning: []
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              id: 1
+              content: "Updated Article 1 content"
+              author_id: 1
+            }
+            ],
+          if_matched: {
+            match_columns: id
+            update_columns: []
+          }
+        ) {
+          affected_rows
+          returning {
+            id
+            title
+            content
+            author_id
+            }
+          }
+        }
+
+- description: Upserting article data via GraphQL mutation as Restricted role errors when using _PLACEHOLDER
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: restricted
+    X-Hasura-User-Id: '1'
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.insert_article.args.if_matched
+        code: validation-failed
+      message: erroneous column name
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              id: 1
+              content: "Updated Article 1 content"
+              author_id: 1
+            }
+            ],
+          if_matched: {
+            match_columns: id
+            update_columns: [_PLACEHOLDER]
+          }
+        ) {
+          affected_rows
+          returning {
+            id
+            title
+            content
+            author_id
+            }
+          }
+        }

server/tests-py/queries/graphql_mutation/insert/permissions/article_if_matched_user_role_mssql.yaml

@@ -0,0 +1,107 @@
+- description: Upserts article data via GraphQL mutation as user_with_all_perms role
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: user_with_all_perms
+    X-Hasura-User-Id: '1'
+  response:
+    data:
+      insert_article:
+        affected_rows: 1
+        returning:
+        - id: 1
+          title: Article 1
+          content: Updated Article 1 content
+          author_id: 1
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              id: 1
+              title: "unexpected"
+              content: "Updated Article 1 content"
+              author_id: 1
+            }
+            ],
+          if_matched: {
+            match_columns: id
+            update_columns: [content]
+          }
+        ) {
+          affected_rows
+          returning {
+            id
+            title
+            content
+            author_id
+            }
+          }
+        }
+
+- description: Upserting article data via GraphQL mutation as user role fails without update permissions
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: user_with_select_and_insert
+    X-Hasura-User-Id: '1'
+  response:
+      errors:
+      - extensions:
+          path: $.selectionSet.insert_article
+          code: validation-failed
+        message: '"insert_article" has no argument named "if_matched"'
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              id: 1
+              title: "unexpected"
+              content: "Updated Article 1 content"
+              author_id: 1
+            }
+            ],
+          if_matched: {
+            match_columns: id
+            update_columns: [content]
+          }
+        ) {
+          affected_rows
+          }
+        }
+
+- description: Upserting article data via GraphQL mutation as user role fails without select permissions
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: user_with_update_and_insert
+    X-Hasura-User-Id: '1'
+  response:
+      errors:
+      - extensions:
+          path: $.selectionSet.insert_article
+          code: validation-failed
+        message: '"insert_article" has no argument named "if_matched"'
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              id: 1
+              title: "unexpected"
+              content: "Updated Article 1 content"
+              author_id: 1
+            }
+            ],
+          if_matched: {
+            match_columns: id
+            update_columns: [content]
+          }
+        ) {
+          affected_rows
+          }
+        }

server/tests-py/queries/graphql_mutation/insert/permissions/schema_setup_mssql.yaml

@@ -11,12 +11,6 @@ args:
           [is_registered] bit NOT NULL DEFAULT 0
       );
 
-      INSERT INTO
-          author([name], [is_registered])
-      VALUES
-          ('Author 1', 1),
-          ('Author 2', 0);
-
       CREATE TABLE article (
           id int identity NOT NULL PRIMARY KEY,
           title TEXT,

server/tests-py/queries/graphql_mutation/insert/permissions/setup_mssql.yaml

@@ -1,5 +1,6 @@
 type: bulk
 args:
+  # Track tables
   - type: mssql_track_table
     args:
       source: mssql
@@ -10,6 +11,8 @@ args:
       source: mssql
       table:
         name: article
+
+  # Permissions for user on article
   - type: mssql_create_insert_permission
     args:
       source: mssql
@@ -23,3 +26,113 @@ args:
         - title
         - content
         - author_id
+
+  # Permissions for user_with_select_and_insert on article
+  - type: mssql_create_insert_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: user_with_select_and_insert
+      permission:
+        check:
+          author_id: X-Hasura-User-Id
+        columns: '*'
+  - type: mssql_create_select_permission
+    args:
+      source: mssql
+      table: article
+      role: user_with_select_and_inesrt
+      permission:
+        columns: '*'
+        filter:
+          $or:
+            - author_id: X-Hasura-User-Id
+            - is_published: 1
+
+  # Permissions for user_with_update_and_insert on article
+  - type: mssql_create_insert_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: user_with_update_and_insert
+      permission:
+        check:
+          author_id: X-Hasura-User-Id
+        columns: '*'
+  - type: mssql_create_update_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: user_with_update_and_insert
+      permission:
+        filter:
+          author_id: X-Hasura-User-Id
+        columns: '*'
+
+  # Permissions for user_with_all_perms on article
+  - type: mssql_create_select_permission
+    args:
+      source: mssql
+      table: article
+      role: user_with_all_perms
+      permission:
+        columns: '*'
+        filter:
+          $or:
+            - author_id: X-Hasura-User-Id
+            - is_published: 1
+  - type: mssql_create_insert_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: user_with_all_perms
+      permission:
+        check:
+          author_id: X-Hasura-User-Id
+        columns: '*'
+  - type: mssql_create_update_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: user_with_all_perms
+      permission:
+        filter:
+          author_id: X-Hasura-User-Id
+        columns: '*'
+
+  # Permissions for restricted on article
+  - type: mssql_create_select_permission
+    args:
+      source: mssql
+      table: article
+      role: restricted
+      permission:
+        columns: '*'
+        filter:
+          $or:
+            - author_id: X-Hasura-User-Id
+            - is_published: 1
+  - type: mssql_create_insert_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: restricted
+      permission:
+        check:
+          author_id: X-Hasura-User-Id
+  - type: mssql_create_update_permission
+    args:
+      source: mssql
+      table:
+        name: article
+      role: restricted
+      permission:
+        filter:
+          author_id: X-Hasura-User-Id
+        columns: []

server/tests-py/queries/graphql_mutation/insert/permissions/values_setup_mssql.yaml

@@ -0,0 +1,32 @@
+type: bulk
+args:
+
+- type: mssql_run_sql
+  args:
+    source: mssql
+    sql: |
+      INSERT INTO
+          author([name])
+      VALUES
+          ('Author 1'),
+          ('Author 2');
+
+
+      INSERT INTO article (title,content,author_id)
+        VALUES
+        (
+          'Article 1',
+          'Sample article content 1',
+          1
+        ),
+        (
+          'Article 2',
+          'Sample article content 2',
+          1
+        ),
+        (
+          'Article 3',
+          'Sample article content 3',
+          2
+        )
+      ;

server/tests-py/queries/graphql_mutation/insert/permissions/values_teardown_mssql.yaml

@@ -5,3 +5,8 @@ args:
       source: mssql
       sql: |
         DELETE FROM article;
+        DELETE FROM author;
+
+        -- reset identity columns
+        DBCC CHECKIDENT ('article', RESEED, 0);
+        DBCC CHECKIDENT ('author', RESEED, 0);

server/tests-py/test_graphql_mutations.py

@@ -885,6 +885,14 @@ class TestGraphqlInsertPermissionMSSQL:
     def test_insert_permission_columns_fail(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + "/article_user_role_columns_fail_mssql.yaml")
 
+    def test_user_role_if_matched_update(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + "/article_if_matched_user_role_mssql.yaml")
+
+    def test_restricted_role_if_matched_update(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + "/article_if_matched_restricted_role_mssql.yaml")
+
+
+
 @pytest.mark.parametrize("backend", ['mssql'])
 @use_mutation_fixtures
 class TestGraphqlInsertIfMatchedMSSQL: