Add check expresion to update permissions (close #384) (#3804)

* Add check expresion to update permissions (close #384) * wip on conflict behavior * Handle upserts for views properly * Use insert check if there is no update check * Fix the test * Improve error message slightly Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com>
2024-12-15 17:31:56 +03:00 · 2020-02-12 23:38:49 -08:00 · 2020-02-12 23:38:49 -08:00 · f615abd2f2
commit f615abd2f2
parent 441c0a2452
17 changed files with 198 additions and 73 deletions
--- a/console/src/components/Services/Data/TableBrowseRows/FilterQuery.js
+++ b/console/src/components/Services/Data/TableBrowseRows/FilterQuery.js
@ -269,13 +269,17 @@ class FilterQuery extends Component {
        >
          <div>
            <div
-              className={`${styles.queryBox} col-xs-6 ${styles.padd_left_remove}`}
+              className={`${styles.queryBox} col-xs-6 ${
+                styles.padd_left_remove
+              }`}
            >
              <span className={styles.subheading_text}>Filter</span>
              {renderWheres(whereAnd, tableSchema, dispatch)}
            </div>
            <div
-              className={`${styles.queryBox} col-xs-6 ${styles.padd_left_remove}`}
+              className={`${styles.queryBox} col-xs-6 ${
+                styles.padd_left_remove
+              }`}
            >
              <b className={styles.subheading_text}>Sort</b>
              {renderSorts(orderBy, tableSchema, dispatch)}
--- a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs
+++ b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs
@ -29,9 +29,11 @@ import           Hasura.GraphQL.Resolve.Mutation
 import           Hasura.GraphQL.Resolve.Select
 import           Hasura.GraphQL.Validate.Field
 import           Hasura.GraphQL.Validate.Types
-import           Hasura.RQL.DML.Insert             (insertCheckExpr)
-import           Hasura.RQL.DML.Internal           (convAnnBoolExpPartialSQL, convPartialSQLExp,
-                                                    dmlTxErrorHandler, sessVarFromCurrentSetting)
+import           Hasura.RQL.DML.Insert             (insertOrUpdateCheckExpr)
+import           Hasura.RQL.DML.Internal           (convPartialSQLExp,
+                                                    convAnnBoolExpPartialSQL, 
+                                                    dmlTxErrorHandler,
+                                                    sessVarFromCurrentSetting)
 import           Hasura.RQL.DML.Mutation
 import           Hasura.RQL.GBoolExp               (toSQLBoolExp)
 import           Hasura.RQL.Types
@ -50,7 +52,7 @@ data AnnIns a
  = AnnIns
  { _aiInsObj         :: !a
  , _aiConflictClause :: !(Maybe RI.ConflictClauseP1)
-  , _aiCheckCond      :: AnnBoolExpPartialSQL
+  , _aiCheckCond      :: !(AnnBoolExpPartialSQL, Maybe AnnBoolExpPartialSQL)
  , _aiTableCols      :: ![PGColumnInfo]
  , _aiDefVals        :: !(Map.HashMap PGCol S.SQLExp)
  } deriving (Show, Eq, Functor, Foldable, Traversable)
@ -140,7 +142,7 @@ traverseInsObj rim allColMap (gName, annVal) defVal@(AnnInsObj cols objRels arrR
            dataObj <- asObject dataVal
            annDataObj <- mkAnnInsObj rtRelInfoMap rtColMap dataObj
            ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm rtColMap
-            let singleObjIns = AnnIns annDataObj ccM checkCond rtCols rtDefValsRes
+            let singleObjIns = AnnIns annDataObj ccM (checkCond, rtUpdPerm >>= upfiCheck) rtCols rtDefValsRes
                objRelIns = RelIns singleObjIns relInfo
            return (AnnInsObj cols (objRelIns:objRels) arrRels)

@ -151,7 +153,7 @@ traverseInsObj rim allColMap (gName, annVal) defVal@(AnnInsObj cols objRels arrR
                    dataObj <- asObject arrDataVal
                    mkAnnInsObj rtRelInfoMap rtColMap dataObj
                  ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm rtColMap
-                  let multiObjIns = AnnIns annDataObjs ccM checkCond rtCols rtDefValsRes
+                  let multiObjIns = AnnIns annDataObjs ccM (checkCond, rtUpdPerm >>= upfiCheck) rtCols rtDefValsRes
                      arrRelIns = RelIns multiObjIns relInfo
                  return (AnnInsObj cols objRels (arrRelIns:arrRels))
            -- if array relation insert input data has empty objects
@ -172,7 +174,7 @@ parseOnConflict tn updFiltrM allColMap val = withPathK "on_conflict" $
    case updCols of
      [] -> return $ RI.CP1DoNothing $ Just constraint
      _  -> do
-          UpdPermForIns _ updFiltr preSet <- onNothing updFiltrM $ throw500
+          UpdPermForIns _ _ updFiltr preSet <- onNothing updFiltrM $ throw500
            "cannot update columns since update permission is not defined"
          preSetRes <- mapM (convPartialSQLExp sessVarFromCurrentSetting) preSet
          updFltrRes <- traverseAnnBoolExp
@ -223,9 +225,9 @@ mkInsertQ
  -> [PGColWithValue]
  -> Map.HashMap PGCol S.SQLExp
  -> RoleName
-  -> AnnBoolExpSQL
+  -> (AnnBoolExpSQL, Maybe AnnBoolExpSQL)
  -> m CTEExp
-mkInsertQ tn onConflictM insCols defVals role checkCond = do
+mkInsertQ tn onConflictM insCols defVals role (insCheck, updCheck) = do
  (givenCols, args) <- flip runStateT Seq.Empty $ toSQLExps insCols
  let sqlConflict = RI.toSQLConflict <$> onConflictM
      sqlExps = mkSQLRow defVals givenCols
@ -236,7 +238,11 @@ mkInsertQ tn onConflictM insCols defVals role checkCond = do
          . Just
          $ S.RetExp
            [ S.selectStar
-            , insertCheckExpr (toSQLBoolExp (S.QualTable tn) checkCond)
+            , S.Extractor
+                (insertOrUpdateCheckExpr tn onConflictM
+                  (toSQLBoolExp (S.QualTable tn) insCheck)
+                  (fmap (toSQLBoolExp (S.QualTable tn)) updCheck))
+                Nothing
            ]

      adminIns = return (CTEExp (S.CTEInsert sqlInsert) args)
@ -365,9 +371,11 @@ insertObj strfyNum role tn singleObjIns addCols = do
      finalInsCols = cols <> objRelDeterminedCols <> addCols

  -- prepare insert query as with expression
-  checkExpr <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting checkCond
+  insCheck <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting insCond
+  updCheck <- traverse (convAnnBoolExpPartialSQL sessVarFromCurrentSetting) updCond
      
-  CTEExp cte insPArgs <- mkInsertQ tn onConflictM finalInsCols defVals role checkExpr
+  CTEExp cte insPArgs <-
+    mkInsertQ tn onConflictM finalInsCols defVals role (insCheck, updCheck)

  MutateResp affRows colVals <- mutateAndFetchCols tn allCols (cte, insPArgs) strfyNum
  colValM <- asSingleObject colVals
@ -377,7 +385,7 @@ insertObj strfyNum role tn singleObjIns addCols = do

  return (totAffRows, colValM)
  where
-    AnnIns annObj onConflictM checkCond allCols defVals = singleObjIns
+    AnnIns annObj onConflictM (insCond, updCond) allCols defVals = singleObjIns
    AnnInsObj cols objRels arrRels = annObj

    arrRelDepCols = flip getColInfos allCols $
@ -412,7 +420,7 @@ insertMultipleObjects
 insertMultipleObjects strfyNum role tn multiObjIns addCols mutFlds errP =
  bool withoutRelsInsert withRelsInsert anyRelsToInsert
  where
-    AnnIns insObjs onConflictM checkCond tableColInfos defVals = multiObjIns
+    AnnIns insObjs onConflictM (insCond, updCond) tableColInfos defVals = multiObjIns
    singleObjInserts = multiToSingles multiObjIns
    insCols = map _aioColumns insObjs
    allInsObjRels = concatMap _aioObjRels insObjs
@ -433,10 +441,11 @@ insertMultipleObjects strfyNum role tn multiObjIns addCols mutFlds errP =
        rowsWithCol <- mapM toSQLExps withAddCols
        return $ map (mkSQLRow defVals) rowsWithCol

-      checkExpr <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting checkCond
+      insCheck <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting insCond
+      updCheck <- traverse (convAnnBoolExpPartialSQL sessVarFromCurrentSetting) updCond
      
      let insQP1 = RI.InsertQueryP1 tn tableCols sqlRows onConflictM 
-                     (Just checkExpr) mutFlds tableColInfos
+                     (insCheck, updCheck) mutFlds tableColInfos
          p1 = (insQP1, prepArgs)
      RI.insertP2 strfyNum p1

@ -480,7 +489,7 @@ convertInsert role tn fld = prefixErrPath fld $ do
      conflictClauseM <- forM onConflictM $ parseOnConflict tn updPerm tableColMap
      defValMapRes <- mapM (convPartialSQLExp sessVarFromCurrentSetting)
                      defValMap
-      let multiObjIns = AnnIns annInsObjs conflictClauseM checkCond
+      let multiObjIns = AnnIns annInsObjs conflictClauseM (checkCond, updPerm >>= upfiCheck)
                          tableCols defValMapRes
          tableCols = Map.elems tableColMap
      strfyNum <- stringifyNum <$> asks getter
--- a/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs
+++ b/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs
@ -148,14 +148,15 @@ convertUpdateP1 opCtx fld = do

  mutFlds <- convertMutResp (_fType fld) $ _fSelSet fld

-  pure $ RU.AnnUpd tn updateItems (unresolvedPermFilter, whereExp) mutFlds allCols
+  pure $ RU.AnnUpd tn updateItems (unresolvedPermFilter, whereExp) unresolvedPermCheck mutFlds allCols
  where
    convObjWithOp' = convObjWithOp colGNameMap
    allCols = Map.elems colGNameMap
-    UpdOpCtx tn _ colGNameMap filterExp preSetCols = opCtx
+    UpdOpCtx tn _ colGNameMap filterExp checkExpr preSetCols = opCtx
    args = _fArguments fld
    resolvedPreSetItems = Map.toList $ fmap partialSQLExpToUnresolvedVal preSetCols
    unresolvedPermFilter = fmapAnnBoolExp partialSQLExpToUnresolvedVal filterExp
+    unresolvedPermCheck = maybe annBoolExpTrue (fmapAnnBoolExp partialSQLExpToUnresolvedVal) checkExpr

    resolveUpdateOperator operator resolveAction =
      (operator,) <$> withArgM args operator resolveAction
--- a/server/src-lib/Hasura/GraphQL/Resolve/Types.hs
+++ b/server/src-lib/Hasura/GraphQL/Resolve/Types.hs
@ -83,6 +83,7 @@ data UpdOpCtx
  , _uocHeaders    :: ![T.Text]
  , _uocAllCols    :: !PGColGNameMap
  , _uocFilter     :: !AnnBoolExpPartialSQL
+  , _uocCheck      :: !(Maybe AnnBoolExpPartialSQL)
  , _uocPresetCols :: !PreSetColsPartial
  } deriving (Show, Eq)

@ -173,6 +174,7 @@ type RelationInfoMap = Map.HashMap RelName RelInfo
 data UpdPermForIns
  = UpdPermForIns
  { upfiCols   :: ![PGCol]
+  , upfiCheck  :: !(Maybe AnnBoolExpPartialSQL)
  , upfiFilter :: !AnnBoolExpPartialSQL
  , upfiSet    :: !PreSetColsPartial
  } deriving (Show, Eq)
--- a/server/src-lib/Hasura/GraphQL/Schema.hs
+++ b/server/src-lib/Hasura/GraphQL/Schema.hs
@ -323,7 +323,7 @@ getRootFldsRole'
  -> [FunctionInfo]
  -> Maybe ([T.Text], Bool) -- insert perm
  -> Maybe (AnnBoolExpPartialSQL, Maybe Int, [T.Text], Bool) -- select filter
-  -> Maybe ([PGColumnInfo], PreSetColsPartial, AnnBoolExpPartialSQL, [T.Text]) -- update filter
+  -> Maybe ([PGColumnInfo], PreSetColsPartial, AnnBoolExpPartialSQL, Maybe AnnBoolExpPartialSQL, [T.Text]) -- update filter
  -> Maybe (AnnBoolExpPartialSQL, [T.Text]) -- delete filter
  -> Maybe ViewInfo
  -> TableConfig -- custom config
@ -363,13 +363,13 @@ getRootFldsRole' tn primaryKey constraints fields funcs insM
    insCustName = getCustomNameWith _tcrfInsert
    getInsDet (hdrs, upsertPerm) =
      let isUpsertable = upsertable constraints upsertPerm $ isJust viM
-      in ( MCInsert $ InsOpCtx tn $ hdrs `union` maybe [] (\(_, _, _, x) -> x) updM
+      in ( MCInsert $ InsOpCtx tn $ hdrs `union` maybe [] (\(_, _, _, _, x) -> x) updM
         , mkInsMutFld insCustName tn isUpsertable
         )

    updCustName = getCustomNameWith _tcrfUpdate
-    getUpdDet (updCols, preSetCols, updFltr, hdrs) =
-      ( MCUpdate $ UpdOpCtx tn hdrs colGNameMap updFltr preSetCols
+    getUpdDet (updCols, preSetCols, updFltr, updCheck, hdrs) =
+      ( MCUpdate $ UpdOpCtx tn hdrs colGNameMap updFltr updCheck preSetCols
      , mkUpdMutFld updCustName tn updCols
      )

@ -517,7 +517,7 @@ mkInsCtx role tableCache fields insPermInfo updPermM = do
    setCols = ipiSet insPermInfo
    checkCond = ipiCheck insPermInfo
    updPermForIns = mkUpdPermForIns <$> updPermM
-    mkUpdPermForIns upi = UpdPermForIns (toList $ upiCols upi)
+    mkUpdPermForIns upi = UpdPermForIns (toList $ upiCols upi) (upiCheck upi)
                          (upiFilter upi) (upiSet upi)

    isInsertable Nothing _          = False
@ -538,7 +538,7 @@ mkAdminInsCtx tc fields = do
      isMutable viIsInsertable viewInfoM && isValidRel relName remoteTable

  let relInfoMap = Map.fromList $ catMaybes relTupsM
-      updPerm = UpdPermForIns updCols noFilter Map.empty
+      updPerm = UpdPermForIns updCols Nothing noFilter Map.empty

  return $ InsCtx colGNameMap noFilter Map.empty relInfoMap (Just updPerm)
  where
@ -650,6 +650,7 @@ getRootFldsRole tn pCols constraints fields funcs viM (RolePermInfo insM selM up
    mkUpd u = ( flip getColInfos allCols $ Set.toList $ upiCols u
              , upiSet u
              , upiFilter u
+              , upiCheck u
              , upiRequiredHeaders u
              )
    mkDel d = (dpiFilter d, dpiRequiredHeaders d)
@ -683,7 +684,7 @@ mkGCtxMapTable tableCache funcCache tabInfo = do
    adminRootFlds =
      getRootFldsRole' tn primaryKey validConstraints fields tabFuncs
      (Just ([], True)) (Just (noFilter, Nothing, [], True))
-      (Just (cols, mempty, noFilter, [])) (Just (noFilter, []))
+      (Just (cols, mempty, noFilter, Nothing, [])) (Just (noFilter, []))
      viewInfo customConfig

 noFilter :: AnnBoolExpPartialSQL
--- a/server/src-lib/Hasura/RQL/DDL/Metadata/Types.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Metadata/Types.hs
@ -345,9 +345,10 @@ replaceMetadataToOrdJSON ( ReplaceMetadata
        updPermDefToOrdJSON :: Permission.UpdPermDef -> AO.Value
        updPermDefToOrdJSON = permDefToOrdJSON updPermToOrdJSON
          where
-            updPermToOrdJSON (Permission.UpdPerm columns set fltr) =
+            updPermToOrdJSON (Permission.UpdPerm columns set fltr check) =
              AO.object $ [ ("columns", AO.toOrdered columns)
                          , ("filter", AO.toOrdered fltr)
+                          , ("check", AO.toOrdered check)
                          ] <> catMaybes [maybeSetToMaybeOrdPair set]

        delPermDefToOrdJSON :: Permission.DelPermDef -> AO.Value
--- a/server/src-lib/Hasura/RQL/DDL/Permission.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Permission.hs
@ -96,13 +96,13 @@ buildInsPermInfo
  -> FieldInfoMap FieldInfo
  -> PermDef InsPerm
  -> m (WithDeps InsPermInfo)
-buildInsPermInfo tn fieldInfoMap (PermDef _rn (InsPerm chk set mCols) _) =
+buildInsPermInfo tn fieldInfoMap (PermDef _rn (InsPerm checkCond set mCols) _) =
  withPathK "permission" $ do
-    (be, beDeps) <- withPathK "check" $ procBoolExp tn fieldInfoMap chk
+    (be, beDeps) <- withPathK "check" $ procBoolExp tn fieldInfoMap checkCond
    (setColsSQL, setHdrs, setColDeps) <- procSetObj tn fieldInfoMap set
    void $ withPathK "columns" $ indexedForM insCols $ \col ->
           askPGType fieldInfoMap col ""
-    let fltrHeaders = getDependentHeaders chk
+    let fltrHeaders = getDependentHeaders checkCond
        reqHdrs = fltrHeaders `union` setHdrs
        insColDeps = map (mkColDep DRUntyped tn) insCols
        deps = mkParentDep tn : beDeps ++ setColDeps ++ insColDeps
@ -201,7 +201,12 @@ data UpdPerm
  = UpdPerm
  { ucColumns :: !PermColSpec -- Allowed columns
  , ucSet     :: !(Maybe (ColumnValues Value)) -- Preset columns
-  , ucFilter  :: !BoolExp     -- Filter expression
+  , ucFilter  :: !BoolExp     -- Filter expression (applied before update)
+  , ucCheck   :: !(Maybe BoolExp)     
+  -- ^ Check expression, which must be true after update.
+  -- This is optional because we don't want to break the v1 API
+  -- but Nothing should be equivalent to the expression which always
+  -- returns true.
  } deriving (Show, Eq, Lift, Generic)
 instance Cacheable UpdPerm
 $(deriveJSON (aesonDrop 2 snakeCase){omitNothingFields=True} ''UpdPerm)
@ -216,10 +221,12 @@ buildUpdPermInfo
  -> FieldInfoMap FieldInfo
  -> UpdPerm
  -> m (WithDeps UpdPermInfo)
-buildUpdPermInfo tn fieldInfoMap (UpdPerm colSpec set fltr) = do
+buildUpdPermInfo tn fieldInfoMap (UpdPerm colSpec set fltr check) = do
  (be, beDeps) <- withPathK "filter" $
    procBoolExp tn fieldInfoMap fltr
    
+  checkExpr <- traverse (withPathK "check" . procBoolExp tn fieldInfoMap) check
+
  (setColsSQL, setHeaders, setColDeps) <- procSetObj tn fieldInfoMap set

  -- check if the columns exist
@ -227,12 +234,12 @@ buildUpdPermInfo tn fieldInfoMap (UpdPerm colSpec set fltr) = do
       askPGType fieldInfoMap updCol relInUpdErr

  let updColDeps = map (mkColDep DRUntyped tn) updCols
-      deps = mkParentDep tn : beDeps ++ updColDeps ++ setColDeps
+      deps = mkParentDep tn : beDeps ++ maybe [] snd checkExpr ++ updColDeps ++ setColDeps
      depHeaders = getDependentHeaders fltr
      reqHeaders = depHeaders `union` setHeaders
      updColsWithoutPreSets = updCols \\ HM.keys setColsSQL

-  return (UpdPermInfo (HS.fromList updColsWithoutPreSets) tn be setColsSQL reqHeaders, deps)
+  return (UpdPermInfo (HS.fromList updColsWithoutPreSets) tn be (fst <$> checkExpr) setColsSQL reqHeaders, deps)

  where
    updCols     = convColSpec fieldInfoMap colSpec
--- a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs
@ -228,16 +228,18 @@ updateSelPermFlds refQT rename rn (SelPerm cols fltr limit aggAllwd computedFiel
 updateUpdPermFlds
  :: (MonadTx m, CacheRM m)
  => QualifiedTable -> Rename -> RoleName -> UpdPerm -> m ()
-updateUpdPermFlds refQT rename rn (UpdPerm cols preset fltr) = do
+updateUpdPermFlds refQT rename rn (UpdPerm cols preset fltr check) = do
  updatedPerm <- case rename of
    RTable rt -> do
      let updFltr = updateTableInBoolExp rt fltr
-      return $ UpdPerm cols preset updFltr
+          updCheck = fmap (updateTableInBoolExp rt) check
+      return $ UpdPerm cols preset updFltr updCheck
    RField rf -> do
      updFltr <- updateFieldInBoolExp refQT rf fltr
+      updCheck <- traverse (updateFieldInBoolExp refQT rf) check
      let updCols = updateCols refQT rf cols
          updPresetM = updatePreset refQT rf <$> preset
-      return $ UpdPerm updCols updPresetM updFltr
+      return $ UpdPerm updCols updPresetM updFltr updCheck
  liftTx $ updatePermDefInCatalog PTUpdate refQT rn updatedPerm

 updateDelPermFlds
--- a/server/src-lib/Hasura/RQL/DML/Insert.hs
+++ b/server/src-lib/Hasura/RQL/DML/Insert.hs
@ -36,13 +36,13 @@ data InsertQueryP1
  , iqp1Cols      :: ![PGCol]
  , iqp1Tuples    :: ![[S.SQLExp]]
  , iqp1Conflict  :: !(Maybe ConflictClauseP1)
-  , iqp1CheckCond :: !(Maybe AnnBoolExpSQL)
+  , iqp1CheckCond :: !(AnnBoolExpSQL, Maybe AnnBoolExpSQL)
  , iqp1MutFlds   :: !MutFlds
  , iqp1AllCols   :: ![PGColumnInfo]
  } deriving (Show, Eq)

 mkInsertCTE :: InsertQueryP1 -> S.CTE
-mkInsertCTE (InsertQueryP1 tn cols vals c checkCond _ _) =
+mkInsertCTE (InsertQueryP1 tn cols vals c (insCheck, updCheck) _ _) =
    S.CTEInsert insert
  where
    tupVals = S.ValuesExp $ map S.TupleExp vals
@ -50,13 +50,13 @@ mkInsertCTE (InsertQueryP1 tn cols vals c checkCond _ _) =
      S.SQLInsert tn cols tupVals (toSQLConflict <$> c) 
        . Just 
        . S.RetExp 
-        $ maybe 
-            [S.selectStar] 
-            (\e -> 
-              [ S.selectStar
-              , insertCheckExpr (toSQLBoolExp (S.QualTable tn) e)
-              ]) 
-            checkCond
+        $ [ S.selectStar
+          , S.Extractor 
+              (insertOrUpdateCheckExpr tn c
+                (toSQLBoolExp (S.QualTable tn) insCheck) 
+                (fmap (toSQLBoolExp (S.QualTable tn)) updCheck))
+              Nothing
+          ]

 toSQLConflict :: ConflictClauseP1 -> S.SQLConflict
 toSQLConflict conflict = case conflict of
@ -186,6 +186,7 @@ convInsertQuery objsParser sessVarBldr prepFn (InsertQuery tableName val oC mRet

  -- Check if the role has insert permissions
  insPerm   <- askInsPermInfo tableInfo
+  updPerm   <- askPermInfo' PAUpdate tableInfo

  -- Check if all dependent headers are present
  validateHeaders $ ipiRequiredHeaders insPerm
@ -215,7 +216,8 @@ convInsertQuery objsParser sessVarBldr prepFn (InsertQuery tableName val oC mRet
  let sqlExps = map snd insTuples
      inpCols = HS.toList $ HS.fromList $ concatMap fst insTuples

-  checkExpr <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting (ipiCheck insPerm)
+  insCheck <- convAnnBoolExpPartialSQL sessVarFromCurrentSetting (ipiCheck insPerm)
+  updCheck <- traverse (convAnnBoolExpPartialSQL sessVarFromCurrentSetting) (upiCheck =<< updPerm)
  
  conflictClause <- withPathK "on_conflict" $ forM oC $ \c -> do
      roleName <- askCurRole
@ -225,7 +227,7 @@ convInsertQuery objsParser sessVarBldr prepFn (InsertQuery tableName val oC mRet
      buildConflictClause sessVarBldr tableInfo inpCols c
  
  return $ InsertQueryP1 tableName insCols sqlExps
-           conflictClause (Just checkExpr) mutFlds allCols
+           conflictClause (insCheck, updCheck) mutFlds allCols
  where
    selNecessaryMsg =
      "; \"returning\" can only be used if the role has "
@ -268,19 +270,65 @@ insertP2 strfyNum (u, p) =
 -- >     THEN NULL 
 -- >     ELSE hdb_catalog.check_violation('insert check constraint failed') 
 -- >   END
-insertCheckExpr
-  :: S.BoolExp
-  -> S.Extractor
-insertCheckExpr condExpr = 
-  S.Extractor
-    (S.SECond condExpr S.SENull
+insertCheckExpr :: Text -> S.BoolExp -> S.SQLExp
+insertCheckExpr errorMessage condExpr = 
+  S.SECond condExpr S.SENull
    (S.SEFunction 
      (S.FunctionExp 
        (QualifiedObject (SchemaName "hdb_catalog") (FunctionName "check_violation")) 
-          (S.FunctionArgs [S.SELit "insert check constraint failed"] mempty)
+        (S.FunctionArgs [S.SELit errorMessage] mempty)
        Nothing)
-      ))
-    Nothing
+    )
+    
+-- | When inserting data, we might need to also enforce the update
+-- check condition, because we might fall back to an update via an
+-- @ON CONFLICT@ clause.
+-- 
+-- We generate something which looks like
+--
+-- > INSERT INTO 
+-- >   ...
+-- > ON CONFLICT DO UPDATE SET 
+-- >   ...
+-- > RETURNING 
+-- >   *, 
+-- >   CASE WHEN xmax = 0
+-- >     THEN CASE WHEN {insert_cond} 
+-- >            THEN NULL 
+-- >            ELSE hdb_catalog.check_violation('insert check constraint failed') 
+-- >          END
+-- >     ELSE CASE WHEN {update_cond} 
+-- >            THEN NULL 
+-- >            ELSE hdb_catalog.check_violation('update check constraint failed') 
+-- >          END
+-- >   END
+-- 
+-- See @https://stackoverflow.com/q/34762732@ for more information on the use of
+-- the @xmax@ system column.
+insertOrUpdateCheckExpr 
+  :: QualifiedTable 
+  -> Maybe ConflictClauseP1
+  -> S.BoolExp 
+  -> Maybe S.BoolExp 
+  -> S.SQLExp
+insertOrUpdateCheckExpr qt (Just _conflict) insCheck (Just updCheck) = 
+  S.SECond 
+    (S.BECompare 
+      S.SEQ 
+      (S.SEQIden (S.QIden (S.mkQual qt) (Iden "xmax")))
+      (S.SEUnsafe "0"))
+    (insertCheckExpr "insert check constraint failed" insCheck)
+    (insertCheckExpr "update check constraint failed" updCheck)
+insertOrUpdateCheckExpr _ _ insCheck _ =
+  -- If we won't generate an ON CONFLICT clause, there is no point
+  -- in testing xmax. In particular, views don't provide the xmax
+  -- system column, but we don't provide ON CONFLICT for views,
+  -- even if they are auto-updatable, so we can fortunately avoid
+  -- having to test the non-existent xmax value.
+  --
+  -- Alternatively, if there is no update check constraint, we should
+  -- use the insert check constraint, for backwards compatibility.
+  insertCheckExpr "insert check constraint failed" insCheck

 runInsert
  :: (QErrM m, UserInfoM m, CacheRM m, MonadTx m, HasSQLGenCtx m)
--- a/server/src-lib/Hasura/RQL/DML/Internal.hs
+++ b/server/src-lib/Hasura/RQL/DML/Internal.hs
@ -41,7 +41,7 @@ mkAdminRolePermInfo ti =
    i = InsPermInfo (HS.fromList pgCols) annBoolExpTrue M.empty []
    s = SelPermInfo (HS.fromList pgCols) (HS.fromList scalarComputedFields) tn annBoolExpTrue
        Nothing True []
-    u = UpdPermInfo (HS.fromList pgCols) tn annBoolExpTrue M.empty []
+    u = UpdPermInfo (HS.fromList pgCols) tn annBoolExpTrue Nothing M.empty []
    d = DelPermInfo tn annBoolExpTrue []

 askPermInfo'
--- a/server/src-lib/Hasura/RQL/DML/Update.hs
+++ b/server/src-lib/Hasura/RQL/DML/Update.hs
@ -16,6 +16,7 @@ import qualified Data.Sequence            as DS

 import           Hasura.EncJSON
 import           Hasura.Prelude
+import           Hasura.RQL.DML.Insert    (insertCheckExpr)
 import           Hasura.RQL.DML.Internal
 import           Hasura.RQL.DML.Mutation
 import           Hasura.RQL.DML.Returning
@ -35,6 +36,7 @@ data AnnUpdG v
  -- we don't prepare the arguments for returning
  -- however the session variable can still be
  -- converted as desired
+  , upq1Check   :: !(AnnBoolExp v)
  , uqp1MutFlds :: !(MutFldsG v)
  , uqp1AllCols :: ![PGColumnInfo]
  } deriving (Show, Eq)
@ -48,22 +50,30 @@ traverseAnnUpd f annUpd =
  AnnUpd tn
  <$> traverse (traverse f) setExps
  <*> ((,) <$> traverseAnnBoolExp f whr <*> traverseAnnBoolExp f fltr)
+  <*> traverseAnnBoolExp f chk
  <*> traverseMutFlds f mutFlds
  <*> pure allCols
  where
-    AnnUpd tn setExps (whr, fltr) mutFlds allCols = annUpd
+    AnnUpd tn setExps (whr, fltr) chk mutFlds allCols = annUpd

 type AnnUpd = AnnUpdG S.SQLExp

 mkUpdateCTE
  :: AnnUpd -> S.CTE
-mkUpdateCTE (AnnUpd tn setExps (permFltr, wc) _ _) =
+mkUpdateCTE (AnnUpd tn setExps (permFltr, wc) chk _ _) =
  S.CTEUpdate update
  where
-    update = S.SQLUpdate tn setExp Nothing tableFltr $ Just S.returningStar
+    update = 
+      S.SQLUpdate tn setExp Nothing tableFltr 
+        . Just 
+        . S.RetExp 
+        $ [ S.selectStar 
+          , S.Extractor (insertCheckExpr "update check constraint failed" checkExpr) Nothing
+          ]
    setExp    = S.SetExp $ map S.SetExpItem setExps
-    tableFltr = Just $ S.WhereFrag $
-                toSQLBoolExp (S.QualTable tn) $ andAnnBoolExps permFltr wc
+    tableFltr = Just $ S.WhereFrag tableFltrExpr
+    tableFltrExpr = toSQLBoolExp (S.QualTable tn) $ andAnnBoolExps permFltr wc
+    checkExpr = toSQLBoolExp (S.QualTable tn) chk

 convInc
  :: (QErrM m)
@ -188,11 +198,15 @@ validateUpdateQueryWith sessVarBldr prepValBldr uq = do

  resolvedUpdFltr <- convAnnBoolExpPartialSQL sessVarBldr $
                     upiFilter updPerm
+  resolvedUpdCheck <- fromMaybe gBoolExpTrue <$>
+                        traverse (convAnnBoolExpPartialSQL sessVarBldr)
+                          (upiCheck updPerm)

  return $ AnnUpd
    tableName
    setExpItems
    (resolvedUpdFltr, annSQLBoolExp)
+    resolvedUpdCheck
    (mkDefaultMutFlds mAnnRetCols)
    allCols
  where
--- a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs
+++ b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs
@ -235,6 +235,7 @@ data UpdPermInfo
  { upiCols            :: !(HS.HashSet PGCol)
  , upiTable           :: !QualifiedTable
  , upiFilter          :: !AnnBoolExpPartialSQL
+  , upiCheck           :: !(Maybe AnnBoolExpPartialSQL)
  , upiSet             :: !PreSetColsPartial
  , upiRequiredHeaders :: ![T.Text]
  } deriving (Show, Eq, Generic)
--- a/server/tests-py/queries/graphql_mutation/update/permissions/schema_setup.yaml
+++ b/server/tests-py/queries/graphql_mutation/update/permissions/schema_setup.yaml
@ -89,7 +89,6 @@ args:
          - author_id: X-HASURA-USER-ID
          - is_published: true

-
 #Article update permission for user
 #Allow modifications only on unpublished articles
 - type: create_update_permission
@ -107,6 +106,8 @@ args:
        $and:
          - author_id: X-HASURA-USER-ID
          - is_published: false
+      check:
+        is_published: false

 #Resident table
 - type: run_sql
--- a/server/tests-py/queries/graphql_mutation/update/permissions/user_cannot_publish.yaml
+++ b/server/tests-py/queries/graphql_mutation/update/permissions/user_cannot_publish.yaml
@ -0,0 +1,27 @@
+description: Try to change the published flag on an unpublished article
+url: /v1/graphql
+response:
+  errors:
+  - extensions:
+      path: $
+      code: permission-error
+    message: Check constraint violation. update check constraint failed
+headers:
+  X-Hasura-Role: user
+  X-Hasura-User-Id: '1'
+status: 200
+query:
+  query: |
+    mutation update_article {
+      update_article(
+        where: {id: {_eq: 2}},
+        _set: {
+           is_published: true
+        }
+      ) {
+        affected_rows
+        returning{
+          id
+        }
+      }
+    }
--- a/server/tests-py/queries/v1/metadata_order/clear_export_metadata.yaml
+++ b/server/tests-py/queries/v1/metadata_order/clear_export_metadata.yaml
@ -244,10 +244,12 @@
        permission:
          columns: "*"
          filter: {}
+          check:
      - role: role2
        permission:
          columns: "*"
          filter: {}
+          check:
      delete_permissions:
      - role: role1
        permission:
--- a/server/tests-py/queries/v1/metadata_order/export_metadata.yaml
+++ b/server/tests-py/queries/v1/metadata_order/export_metadata.yaml
@ -48,10 +48,12 @@ response:
      permission:
        columns: "*"
        filter: {}
+        check:
    - role: role2
      permission:
        columns: "*"
        filter: {}
+        check:
    delete_permissions:
    - role: role1
      permission:
--- a/server/tests-py/test_graphql_mutations.py
+++ b/server/tests-py/test_graphql_mutations.py
@ -338,6 +338,9 @@ class TestGraphqlUpdatePermissions(DefaultTestMutations):
    def test_user_cannot_update_another_users_article(self, hge_ctx, transport):
        check_query_f(hge_ctx, self.dir() + "/user_cannot_update_another_users_article.yaml")
    
+    def test_user_cannot_publish(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + "/user_cannot_publish.yaml")
+
    def test_user_cannot_update_id_col(self, hge_ctx, transport):
        check_query_f(hge_ctx, self.dir() + "/user_cannot_update_id_col_article.yaml")