Fix MySQL PUT/PATCH regarding row-level update database policy

config-generators/mysql-commands.txt

@@ -61,6 +61,9 @@ update Stock --config "dab-config.MySql.json" --permissions "test_role_with_excl
 update Stock --config "dab-config.MySql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "categoryName"
 update Stock --config "dab-config.MySql.json" --permissions "test_role_with_policy_excluded_fields:create,update,delete"
 update Stock --config "dab-config.MySql.json" --permissions "test_role_with_policy_excluded_fields:read" --fields.exclude "categoryName" --policy-database "@item.piecesAvailable ne 0"
+update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:update" --policy-database "@item.pieceid ne 1"
+update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:create"
+update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:read"
 update Book --config "dab-config.MySql.json" --permissions "authenticated:create,read,update,delete"
 update Book --config "dab-config.MySql.json" --relationship publishers --target.entity Publisher --cardinality one
 update Book --config "dab-config.MySql.json" --relationship websiteplacement --target.entity BookWebsitePlacement --cardinality one

src/Core/Resolvers/MySqlQueryBuilder.cs

@@ -18,6 +18,13 @@ public class MySqlQueryBuilder : BaseSqlQueryBuilder, IQueryBuilder
         private static DbCommandBuilder _builder = new MySqlCommandBuilder();
         public const string DATABASE_NAME_PARAM = "databaseName";
 
+        /// <summary>
+        /// Column alias under which the number of records already present for the given primary key
+        /// is returned as the first result set of an upsert query. Used by the query executor to
+        /// distinguish an update from an insert and to detect database policy failures.
+        /// </summary>
+        public const string COUNT_ROWS_WITH_GIVEN_PK = "cnt_rows_to_update";
+
         /// <summary>
         /// Adds database specific quotes to string identifier
         /// </summary>
@@ -113,29 +120,70 @@ public string Build(SqlExecuteStructure structure)
         public string Build(SqlUpsertQueryStructure structure)
         {
             (string sets, string updates, string select) = MakeQuerySegmentsForUpdate(structure, structure.OutputColumns);
+            string tableName = QuoteIdentifier(structure.DatabaseObject.Name);
+
+            // Predicates identifying the record by its primary key.
+            string pkPredicates = Build(structure.Predicates);
+
+            // Predicates for the UPDATE: primary key + database policy configured for the update operation.
+            // Applying the update policy here ensures a PUT/PATCH cannot overwrite a record the caller is
+            // not authorized to modify (e.g. a row owned by a different user).
+            string updatePredicates = JoinPredicateStrings(
+                pkPredicates,
+                structure.GetDbPolicyForOperation(EntityActionOperation.Update));
+
+            string updateOperations = Build(structure.UpdateOperations, ", ");
 
             if (structure.IsFallbackToUpdate)
             {
+                // Update-only path (e.g. autogenerated primary key): no insert is attempted, so there is no
+                // insert/update race to guard against.
+                //  - @cnt: whether a record exists for the given primary key. Surfaced as the first result
+                //    set so the executor can distinguish a policy failure (403) from a missing record (404).
+                //  - @matched: whether a record exists that ALSO satisfies the update policy. The update
+                //    output is emitted whenever @matched > 0 - regardless of whether any physical value
+                //    actually changed - so that authorized idempotent updates still return the row (HTTP 200)
+                //    even when the connection uses UseAffectedRows=true (where ROW_COUNT() would be 0).
                 return sets + ";\n" +
-                    $"UPDATE {QuoteIdentifier(structure.DatabaseObject.Name)} " +
-                    $"SET {Build(structure.UpdateOperations, ", ")} " +
-                        ", " + updates +
-                    $" WHERE {Build(structure.Predicates)}; " +
-                    $" SET @ROWCOUNT=ROW_COUNT(); " +
-                    $"SELECT " + select + $" WHERE @ROWCOUNT > 0;";
+                    $"SET @cnt := (SELECT COUNT(*) FROM {tableName} WHERE {pkPredicates}); " +
+                    $"SET @matched := (SELECT COUNT(*) FROM {tableName} WHERE {updatePredicates}); " +
+                    $"SELECT @cnt AS {QuoteIdentifier(COUNT_ROWS_WITH_GIVEN_PK)}; " +
+                    $"UPDATE {tableName} SET {updateOperations}, {updates} WHERE {updatePredicates}; " +
+                    $"SELECT {select} WHERE @matched > 0;";
             }
             else
             {
-                string insert = $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.Name)} ({Build(structure.InsertColumns)}) " +
-                        $"VALUES ({string.Join(", ", (structure.Values))}) ";
+                string insertColumns = Build(structure.InsertColumns);
+                string insertValues = string.Join(", ", structure.Values);
+
+                // The insert is expressed as INSERT ... ON DUPLICATE KEY UPDATE with a no-op update clause.
+                // This makes the statement atomic and race-safe: concurrent upserts for the same missing
+                // primary key serialize on the unique index here (one inserts, the others observe the
+                // duplicate and no-op), rather than both reading an "absent" state and then deadlocking on
+                // gap locks / failing with a duplicate-key error.
+                //
+                // The ON DUPLICATE branch runs only when the row already exists, so it is used to set
+                // @existed := 1. Whether the row already existed is therefore detected by @existed - NOT by
+                // ROW_COUNT() - because ROW_COUNT() for a no-op ON DUPLICATE KEY UPDATE is 1 (found) rather
+                // than 0 when the connection reports found rows (UseAffectedRows=false, the default), which
+                // would be indistinguishable from an insert. Setting the primary key column to itself keeps
+                // the existing row unchanged, so the insert values never overwrite it and the update database
+                // policy below cannot be bypassed.
+                string firstPrimaryKey = QuoteIdentifier(structure.PrimaryKey().First());
 
                 return sets + ";\n" +
-                        insert + " ON DUPLICATE KEY " +
-                        $"UPDATE {Build(structure.UpdateOperations, ", ")}" +
-                        $", " + updates + ";" +
-                        $" SET @ROWCOUNT=ROW_COUNT(); " +
-                        $"SELECT " + select + $" WHERE @ROWCOUNT != 1;" +
-                        $"SELECT {MakeUpsertSelections(structure)} WHERE @ROWCOUNT = 1;";
+                    $"SET @existed := 0; " +
+                    $"INSERT INTO {tableName} ({insertColumns}) VALUES ({insertValues}) " +
+                        $"ON DUPLICATE KEY UPDATE {firstPrimaryKey} = IF(@existed := 1, {firstPrimaryKey}, {firstPrimaryKey}); " +
+                    // The row now exists (pre-existing or just inserted). @matched reflects whether the
+                    // row satisfies the update policy; it is only consulted on the update path below.
+                    $"SET @matched := (SELECT COUNT(*) FROM {tableName} WHERE {updatePredicates}); " +
+                    // Surface whether the row already existed (1) or was inserted (0) as the first result set.
+                    $"SELECT @existed AS {QuoteIdentifier(COUNT_ROWS_WITH_GIVEN_PK)}; " +
+                    // Apply the policy-aware update only when the row already existed.
+                    $"UPDATE {tableName} SET {updateOperations}, {updates} WHERE @existed = 1 AND {updatePredicates}; " +
+                    $"SELECT {select} WHERE @existed = 1 AND @matched > 0; " +
+                    $"SELECT {MakeUpsertSelections(structure)} WHERE @existed = 0;";
             }
         }
 

src/Core/Resolvers/MySqlQueryExecutor.cs

@@ -2,11 +2,13 @@
 // Licensed under the MIT License.
 
 using System.Data.Common;
+using System.Net;
 using Azure.Core;
 using Azure.DataApiBuilder.Config;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Core.Configurations;
 using Azure.DataApiBuilder.Core.Models;
+using Azure.DataApiBuilder.Service.Exceptions;
 using Azure.Identity;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
@@ -187,5 +189,105 @@ private bool IsDefaultAccessTokenValid()
 
             return _defaultAccessToken?.Token;
         }
+
+        /// <summary>
+        /// Interprets the result sets produced by an upsert (PUT/PATCH) query built by
+        /// <see cref="MySqlQueryBuilder.Build(SqlUpsertQueryStructure)"/> to determine whether the
+        /// operation resulted in an update or an insert, and to surface database policy failures.
+        /// The upsert query returns:
+        ///   result set #1: 1 if the row already existed (update path), 0 if it was inserted or is absent (insert path).
+        ///   result set #2: the output of the UPDATE (non-empty when a row matched the primary key and the update policy).
+        ///   result set #3 (non-fallback only): the output of the INSERT (non-empty only when a record was inserted).
+        /// </summary>
+        /// <param name="dbDataReader">A DbDataReader.</param>
+        /// <param name="args">The arguments to this handler - args[0] = primary key in pretty format, args[1] = entity name.</param>
+        public override async Task<DbResultSet> GetMultipleResultSetsIfAnyAsync(
+            DbDataReader dbDataReader, List<string>? args = null)
+        {
+            // Result set #1: 1 when the row already existed (update path), 0 when it was inserted or is
+            // absent (insert path). Reused below as numOfRecordsWithGivenPK to drive the update/insert branch.
+            DbResultSet countResultSet = await ExtractResultSetFromDbDataReaderAsync(dbDataReader);
+            DbResultSetRow? countResultSetRow = countResultSet.Rows.FirstOrDefault();
+            int numOfRecordsWithGivenPK;
+
+            if (countResultSetRow is not null &&
+                countResultSetRow.Columns.TryGetValue(MySqlQueryBuilder.COUNT_ROWS_WITH_GIVEN_PK, out object? rowsWithGivenPK) &&
+                rowsWithGivenPK is not null)
+            {
+                numOfRecordsWithGivenPK = Convert.ToInt32(rowsWithGivenPK);
+            }
+            else
+            {
+                throw new DataApiBuilderException(
+                    message: "Neither insert nor update could be performed.",
+                    statusCode: HttpStatusCode.InternalServerError,
+                    subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
+            }
+
+            // Result set #2: output of the UPDATE. Non-empty whenever a row matched the primary key and
+            // satisfied the update database policy - independent of whether any value physically changed,
+            // so authorized idempotent updates are still returned.
+            DbResultSet? updateResultSet = await dbDataReader.NextResultAsync()
+                ? await ExtractResultSetFromDbDataReaderAsync(dbDataReader)
+                : null;
+
+            if (numOfRecordsWithGivenPK == 1)
+            {
+                // A record existed for the given primary key, so an update was attempted.
+                if (updateResultSet is null || updateResultSet.Rows.Count == 0)
+                {
+                    // Record exists but no row matched the primary key + update policy - indicates the
+                    // update database policy was not satisfied (e.g. an attempt to modify another user's row).
+                    throw new DataApiBuilderException(
+                        message: DataApiBuilderException.AUTHORIZATION_FAILURE,
+                        statusCode: HttpStatusCode.Forbidden,
+                        subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure);
+                }
+
+                // Identifies this as the result set of an update operation (used to return HTTP 200
+                // instead of 201 and to omit the location header).
+                updateResultSet.ResultProperties.Add(SqlMutationEngine.IS_UPDATE_RESULT_SET, true);
+                return updateResultSet;
+            }
+
+            // No record existed for the given primary key, so an insert was attempted. The insert output
+            // is in result set #3. For the update-only (fallback) path there is no insert result set.
+            DbResultSet? insertResultSet = await dbDataReader.NextResultAsync()
+                ? await ExtractResultSetFromDbDataReaderAsync(dbDataReader)
+                : null;
+
+            if (insertResultSet is null)
+            {
+                // Update-only path (e.g. autogenerated primary key) and no record was found to update.
+                if (args is not null && args.Count > 1)
+                {
+                    string prettyPrintPk = args[0];
+                    string entityName = args[1];
+
+                    throw new DataApiBuilderException(
+                        message: $"Cannot perform INSERT and could not find {entityName} " +
+                            $"with primary key {prettyPrintPk} to perform UPDATE on.",
+                        statusCode: HttpStatusCode.NotFound,
+                        subStatusCode: DataApiBuilderException.SubStatusCodes.ItemNotFound);
+                }
+
+                throw new DataApiBuilderException(
+                    message: "Neither insert nor update could be performed.",
+                    statusCode: HttpStatusCode.InternalServerError,
+                    subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
+            }
+
+            if (insertResultSet.Rows.Count == 0)
+            {
+                // No record existed but nothing was inserted - indicates the create database policy
+                // was not satisfied.
+                throw new DataApiBuilderException(
+                    message: DataApiBuilderException.AUTHORIZATION_FAILURE,
+                    statusCode: HttpStatusCode.Forbidden,
+                    subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure);
+            }
+
+            return insertResultSet;
+        }
     }
 }

src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt

@@ -387,6 +387,23 @@
                 Action: Delete
               }
             ]
+          },
+          {
+            Role: database_policy_tester,
+            Actions: [
+              {
+                Action: Read
+              },
+              {
+                Action: Create
+              },
+              {
+                Action: Update,
+                Policy: {
+                  Database: @item.pieceid ne 1
+                }
+              }
+            ]
           }
         ],
         Relationships: {