server: fix on_conflict missing when no column update permissions

GitOrigin-RevId: 34dd9f648ca1e268274b6244c48c9e9710c4477d

CHANGELOG.md

@@ -1,8 +1,13 @@
 # Hasura GraphQL Engine Changelog
 
 ## Next release
+
+
+### Bug fixes and improvements
+
 (Add entries below in the order of: server, console, cli, docs, others)
 
+- server: fix regression: `on_conflict` was missing in the schema for inserts in tables where the current user has no columns listed in their update permissions (fix #6804)
 - cli: fix regression - `metadata apply —dry-run` was overwriting local metadata files with metadata on server when it should just display the differences.
 - cli: add support for `api_limits` metadata object
 

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

@@ -34,6 +34,7 @@ import           Hasura.GraphQL.Schema.Select
 import           Hasura.GraphQL.Schema.Table
 import           Hasura.RQL.Types
 
+
 -- insert
 
 -- | Construct a root field, normally called insert_tablename, that can be used to add several rows to a DB table
@@ -217,7 +218,11 @@ mkInsertObject objects table columns conflictClause insertPerms updatePerms =
         defaultValues = Map.union (partialSQLExpToUnpreparedValue <$> ipiSet insertPerms)
           $ Map.fromList [(column, UVLiteral $ columnDefaultValue @b column) | column <- pgiColumn <$> columns]
 
--- | Specifies the "ON CONFLICT" SQL clause
+-- | Specifies the "ON CONFLICT" SQL clause.
+-- This object cannot exist if there aren't any unique or primary keys constraints. However,
+-- if there are no columns for which the current role has update permissions, we must still
+-- accept an empty list for `update_columns`; we do this by adding a placeholder value to
+-- the enum (see 'tableUpdateColumnsEnum').
 conflictObject
   :: forall b r m n
    . MonadBuildSchema b r m n
@@ -226,8 +231,8 @@ conflictObject
   -> UpdPermInfo b
   -> m (Maybe (Parser 'Input n (IR.ConflictClauseP1 b (UnpreparedValue b))))
 conflictObject table selectPerms updatePerms = runMaybeT $ do
-  tableGQLName      <- getTableGQLName @b table
-  columnsEnum      <- MaybeT $ tableUpdateColumnsEnum table updatePerms
+  tableGQLName     <- getTableGQLName @b table
+  columnsEnum      <- lift $ tableUpdateColumnsEnum table updatePerms
   constraints      <- MaybeT $ tciUniqueOrPrimaryKeyConstraints . _tiCoreInfo <$> askTableInfo @b table
   constraintParser <- lift $ conflictConstraint constraints table
   whereExpParser   <- lift $ boolExp table selectPerms
@@ -238,8 +243,10 @@ conflictObject table selectPerms updatePerms = runMaybeT $ do
       whereExpName   = $$(G.litName "where")
       fieldsParser = do
         constraint <- IR.CTConstraint <$> P.field constraintName Nothing constraintParser
-        columns    <- P.field columnsName Nothing $ P.list columnsEnum
         whereExp   <- P.fieldOptional whereExpName Nothing whereExpParser
+        columns    <- P.fieldWithDefault columnsName Nothing (G.VList []) (P.list columnsEnum) `P.bindFields` \cs ->
+          -- this can only happen if the placeholder was used
+          sequenceA cs `onNothing` parseError "erroneous column name"
         pure $ case columns of
           [] -> IR.CP1DoNothing $ Just constraint
           _  -> IR.CP1Update constraint columns preSetColumns $
@@ -343,7 +350,6 @@ mkUpdateObject table columns updatePerms ((opExps, whereExp), mutationOutput) =
     checkExp = maybe annBoolExpTrue (fmapAnnBoolExp partialSQLExpToUnpreparedValue) $ upiCheck updatePerms
 
 
-
 -- delete
 
 -- | Construct a root field, normally called delete_tablename, that can be used
@@ -402,7 +408,6 @@ mkDeleteObject table columns deletePerms (whereExp, mutationOutput) =
     permissionFilter = fmapAnnBoolExp partialSQLExpToUnpreparedValue $ dpiFilter deletePerms
 
 
-
 -- common
 
 -- | All mutations allow returning results, such as what the updated database

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

@@ -83,29 +83,30 @@ tableSelectColumnsEnum table selectPermissions = do
 -- table. Used for conflict resolution in "insert" mutations, among
 -- others. Maps to the table_update_column object.
 --
--- Return Nothing if there's no column the current user has "update"
--- permissions for.
+-- 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.
 tableUpdateColumnsEnum
   :: forall m n r b
    . (BackendSchema b, MonadSchema n m, MonadRole r m, MonadTableInfo r m)
   => TableName b
   -> UpdPermInfo b
-  -> m (Maybe (Parser 'Both n (Column b)))
+  -> m (Parser 'Both n (Maybe (Column b)))
 tableUpdateColumnsEnum table updatePermissions = do
   tableGQLName <- getTableGQLName @b table
   columns      <- tableUpdateColumns table updatePermissions
-  let enumName    = tableGQLName <> $$(G.litName "_update_column")
-      description = Just $ G.Description $
-        "update columns of table " <>> table
-  pure $ P.enum enumName description <$> nonEmpty
-    [ ( define $ pgiName column
-      , pgiColumn column
-      )
-    | column <- columns
-    ]
+  let enumName   = tableGQLName <> $$(G.litName "_update_column")
+      enumDesc   = Just $ G.Description $ "update columns of table " <>> table
+      altDesc    = Just $ G.Description $ "placeholder for update columns of table " <> table <<> " (current role has no relevant permissions)"
+      enumValues = do
+        column <- columns
+        pure (define $ pgiName column, Just $ pgiColumn column)
+  pure $ case nonEmpty enumValues of
+    Just values -> P.enum enumName enumDesc values
+    Nothing     -> P.enum enumName altDesc $ pure (placeholder, Nothing)
   where
-    define name =
-      P.mkDefinition name (Just $ G.Description "column name") P.EnumValueInfo
+    define name = P.mkDefinition name (Just $ G.Description "column name") P.EnumValueInfo
+    placeholder = P.mkDefinition $$(G.litName "_PLACEHOLDER") (Just $ G.Description "placeholder (do not use)") P.EnumValueInfo
 
 tablePermissions
   :: forall m n r b. (Backend b, MonadSchema n m, MonadTableInfo r m, MonadRole r m)

server/tests-py/queries/graphql_mutation/insert/onconflict/article_on_conflict_empty_update_columns.yaml

@@ -1,32 +1,64 @@
-description: Upserts article data via GraphQL mutation with empty update columns
-url: /v1/graphql
-status: 200
-response:
-  data:
-    insert_article:
-      returning: []
-query:
-  query: |
-    mutation insert_article {
-      insert_article (
-        objects: [
-          {
-            content: "Updated Article 1 content",
-            id: 1
-          },
-          {
-            content: "Updated Article 2 content",
-            id: 2
+- description: Upserts article data via GraphQL mutation with empty update columns
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      insert_article:
+        returning: []
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              content: "Updated Article 1 content",
+              id: 1
+            },
+            {
+              content: "Updated Article 2 content",
+              id: 2
+            }
+            ],
+          on_conflict: {
+            constraint: article_pkey,
+            update_columns: []
           }
-          ],
-        on_conflict: {
-          constraint: article_pkey,
-          update_columns: []
-        }
-      ) {
-        returning {
-          title
-          content
+        ) {
+          returning {
+            title
+            content
+            }
+          }
+        }
+
+- description: "Upserts article data via GraphQL mutation with empty update columns (omitted columns)"
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      insert_article:
+        returning: []
+  query:
+    query: |
+      mutation insert_article {
+        insert_article (
+          objects: [
+            {
+              content: "Updated Article 1 content",
+              id: 1
+            },
+            {
+              content: "Updated Article 2 content",
+              id: 2
+            }
+            ],
+          on_conflict: {
+            constraint: article_pkey,
+          }
+        ) {
+          returning {
+            title
+            content
+            }
           }
         }
-      }

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

@@ -0,0 +1,72 @@
+- 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
+            }
+            ],
+          on_conflict: {
+            constraint: article_pkey
+            update_columns: []
+          }
+        ) {
+          affected_rows
+          returning {
+            title
+            content
+            author_id
+            }
+          }
+        }
+
+- description: Upserts article data via GraphQL mutation as Restricted role (error 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.on_conflict
+        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
+            }
+            ],
+          on_conflict: {
+            constraint: article_pkey
+            update_columns: [_PLACEHOLDER]
+          }
+        ) {
+          affected_rows
+          returning {
+            title
+            content
+            author_id
+            }
+          }
+        }

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

@@ -162,6 +162,18 @@ args:
           - author_id: X-HASURA-USER-ID
           - is_published: true
 
+#Article select permission for restricted
+- type: create_select_permission
+  args:
+    table: article
+    role: restricted
+    permission:
+      columns: '*'
+      filter:
+        $or:
+          - author_id: X-HASURA-USER-ID
+          - is_published: true
+
 #Article select permission for editor
 - type: create_select_permission
   args:
@@ -184,6 +196,15 @@ args:
       check:
         author_id: X-Hasura-User-Id
 
+#Article insert permission for restricted
+- type: create_insert_permission
+  args:
+    table: article
+    role: restricted
+    permission:
+      check:
+        author_id: X-Hasura-User-Id
+
 #Article insert permission for editor
 #Editor can create articles for some of the users
 - type: create_insert_permission
@@ -205,6 +226,16 @@ args:
         author_id: X-Hasura-User-Id
       columns: '*'
 
+#Article udpate permission for restricted
+- type: create_update_permission
+  args:
+    table: article
+    role: restricted
+    permission:
+      filter:
+        author_id: X-Hasura-User-Id
+      columns: []
+
 #Author select permission for user
 - type: create_select_permission
   args:

server/tests-py/test_graphql_mutations.py

@@ -103,6 +103,9 @@ class TestGraphqlInsertPermission:
     def test_user_role_on_conflict_update(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + "/article_on_conflict_user_role.yaml")
 
+    def test_restricted_role_on_conflict_update(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + "/article_on_conflict_restricted_role.yaml")
+
     def test_user_role_on_conflict_constraint_on_error(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + "/article_on_conflict_constraint_on_user_role_error.yaml")