diff --git a/config-generators/mysql-commands.txt b/config-generators/mysql-commands.txt index a6a8172f32..30f3f5e5a1 100644 --- a/config-generators/mysql-commands.txt +++ b/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 diff --git a/src/Core/Resolvers/MySqlQueryBuilder.cs b/src/Core/Resolvers/MySqlQueryBuilder.cs index e8300973a4..69f00a894d 100644 --- a/src/Core/Resolvers/MySqlQueryBuilder.cs +++ b/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"; + /// + /// 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. + /// + public const string COUNT_ROWS_WITH_GIVEN_PK = "cnt_rows_to_update"; + /// /// Adds database specific quotes to string identifier /// @@ -113,29 +120,69 @@ 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)); + + // Capture two facts about the pre-update state of the row, surfaced/consumed by the executor: + // - @cnt: whether a record exists for the given primary key. Used to distinguish an update + // from an insert (first result set). + // - @matched: whether a record exists that ALSO satisfies the update policy. Used to decide + // whether the update was authorized. + // These are *locking* reads (FOR UPDATE). On an existing row InnoDB takes a record lock; on a + // missing primary key it takes a gap lock, which prevents another concurrent transaction from + // inserting the same key until this transaction completes. This makes the insert-vs-update + // decision race-safe: two concurrent upserts for the same missing PK are serialized, and the + // second request observes the row the first inserted and cleanly resolves to an update instead + // of failing with a duplicate-key error. + // @matched is deliberately a *matched-row* count rather than ROW_COUNT() (the number of + // *changed* rows). ROW_COUNT() reports 0 for an authorized idempotent update (values already + // equal the row's current values) when the connection is opened with UseAffectedRows=true, + // which would otherwise be misread as a policy failure. Using a matched-row count keeps the + // authorization decision correct and independent of the connection-string options. + string countExistingRows = + $"SELECT COUNT(*) INTO @cnt FROM {tableName} WHERE {pkPredicates} FOR UPDATE; " + + $"SELECT COUNT(*) INTO @matched FROM {tableName} WHERE {updatePredicates} FOR UPDATE; " + + $"SELECT @cnt AS {QuoteIdentifier(COUNT_ROWS_WITH_GIVEN_PK)};"; + + // Update honoring the update database policy. The update output is emitted whenever a row + // matched the primary key + policy (@matched > 0) — regardless of whether any physical value + // actually changed — so that authorized idempotent updates still return the row (HTTP 200). + string updateQuery = + $"UPDATE {tableName} " + + $"SET {Build(structure.UpdateOperations, ", ")} " + + ", " + updates + + $" WHERE {updatePredicates}; " + + $"SELECT " + select + $" WHERE @matched > 0;"; if (structure.IsFallbackToUpdate) { - 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;"; + // Update-only path (e.g. autogenerated primary key): no insert is attempted. + return sets + ";\n" + countExistingRows + updateQuery; } else { - string insert = $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.Name)} ({Build(structure.InsertColumns)}) " + - $"VALUES ({string.Join(", ", (structure.Values))}) "; - - 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;"; + // Insert honoring the create database policy, but only when no record already exists for + // the given primary key (@cnt = 0). Gating the insert on @cnt = 0 ensures the update + // policy enforced above cannot be bypassed by falling through to an insert on an + // existing record. + string createPredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); + string insertQuery = + $"INSERT INTO {tableName} ({Build(structure.InsertColumns)}) " + + $"SELECT {string.Join(", ", structure.Values)} FROM DUAL " + + $"WHERE @cnt = 0 AND ({createPredicates}); " + + $"SET @ROWCOUNT=ROW_COUNT(); " + + $"SELECT {MakeUpsertSelections(structure)} WHERE @ROWCOUNT = 1;"; + + return sets + ";\n" + countExistingRows + updateQuery + insertQuery; } } diff --git a/src/Core/Resolvers/MySqlQueryExecutor.cs b/src/Core/Resolvers/MySqlQueryExecutor.cs index 670232b826..4af8bb589c 100644 --- a/src/Core/Resolvers/MySqlQueryExecutor.cs +++ b/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,104 @@ private bool IsDefaultAccessTokenValid() return _defaultAccessToken?.Token; } + + /// + /// Interprets the result sets produced by an upsert (PUT/PATCH) query built by + /// 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: the number of records already present for the given primary key. + /// 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). + /// + /// A DbDataReader. + /// The arguments to this handler - args[0] = primary key in pretty format, args[1] = entity name. + public override async Task GetMultipleResultSetsIfAnyAsync( + DbDataReader dbDataReader, List? args = null) + { + // Result set #1: count (0/1) of records already present for the given primary key. + 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; + } } } diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt index d3393a7b89..6b71ddd373 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt +++ b/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: { diff --git a/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertConcurrencyTests.cs b/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertConcurrencyTests.cs new file mode 100644 index 0000000000..65e66bc03b --- /dev/null +++ b/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertConcurrencyTests.cs @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.Http.Json; +using System.Text.Json; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Authorization; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using static Azure.DataApiBuilder.Core.AuthenticationHelpers.AppServiceAuthentication; + +namespace Azure.DataApiBuilder.Service.Tests.SqlTests.RestApiTests +{ + /// + /// Concurrency regression coverage for the MySQL upsert (PUT) path. The insert-vs-update decision is + /// made from a locking existence check (SELECT ... FOR UPDATE) which gap-locks a missing primary key, + /// so two concurrent upserts for the same initially-absent PK are serialized rather than both + /// attempting an insert. One request must create the record and the other must update it; neither may + /// fail with a duplicate-key / database-operation error. + /// + [TestClass, TestCategory(TestCategory.MYSQL)] + public class MySqlUpsertConcurrencyTests : RestApiTestBase + { + private const string EntityPath = "commodities"; + + #region Test Fixture Setup + + [ClassInitialize] + public static async Task SetupAsync(TestContext context) + { + DatabaseEngine = TestCategory.MYSQL; + await InitializeTestFixture(); + } + + [TestCleanup] + public async Task TestCleanup() + { + await ResetDbStateAsync(); + } + + #endregion + + public override string GetQuery(string key) + { + return string.Empty; + } + + /// + /// Two concurrent PUT requests targeting the same, initially-absent primary key must both succeed: + /// exactly one inserts (201 Created) and the other updates (200 OK). Neither request may fail with a + /// duplicate-key / database-operation error, which would occur if the insert-vs-update decision were + /// based on an unlocked pre-count. + /// + [TestMethod] + public async Task ConcurrentPutOnSameMissingPrimaryKeyResolvesCleanly() + { + // Primary key (0, 901) is absent at test start (not part of the seed data). + // categoryName must reference an existing comics.categoryName value (foreign key). + const string primaryKeyRoute = "categoryid/0/pieceid/901"; + string firstBody = @"{ ""categoryName"": ""SciFi"", ""piecesAvailable"": 1, ""piecesRequired"": 1 }"; + string secondBody = @"{ ""categoryName"": ""SciFi"", ""piecesAvailable"": 2, ""piecesRequired"": 2 }"; + + // Issue both requests concurrently. + Task firstRequest = SendPutAsync(primaryKeyRoute, firstBody); + Task secondRequest = SendPutAsync(primaryKeyRoute, secondBody); + + HttpResponseMessage[] responses = await Task.WhenAll(firstRequest, secondRequest); + + foreach (HttpResponseMessage response in responses) + { + Assert.IsTrue( + response.StatusCode is HttpStatusCode.OK or HttpStatusCode.Created, + $"Expected 200 OK or 201 Created but received {(int)response.StatusCode} ({response.StatusCode}). " + + $"Body: {await response.Content.ReadAsStringAsync()}"); + } + + int createdCount = responses.Count(response => response.StatusCode == HttpStatusCode.Created); + int okCount = responses.Count(response => response.StatusCode == HttpStatusCode.OK); + + Assert.AreEqual(1, createdCount, "Exactly one concurrent request should have created the record (201)."); + Assert.AreEqual(1, okCount, "Exactly one concurrent request should have updated the record (200)."); + } + + /// + /// Builds and sends an authenticated PUT request to the commodities entity. + /// + private static Task SendPutAsync(string primaryKeyRoute, string requestBody) + { + string endpoint = $"api/{EntityPath}/{primaryKeyRoute}"; + JsonElement requestBodyElement = JsonDocument.Parse(requestBody).RootElement.Clone(); + + HttpRequestMessage request = new(HttpMethod.Put, endpoint) + { + Content = JsonContent.Create(requestBodyElement) + }; + + // The MySQL test configuration uses the AppService EasyAuth provider. + request.Headers.Add( + AuthenticationOptions.CLIENT_PRINCIPAL_HEADER, + AuthTestHelper.CreateAppServiceEasyAuthToken( + roleClaimType: AuthenticationOptions.ROLE_CLAIM_TYPE, + additionalClaims: new List + { + new() { Typ = AuthenticationOptions.ROLE_CLAIM_TYPE, Val = "authenticated" } + })); + request.Headers.Add(AuthorizationResolver.CLIENT_ROLE_HEADER, "authenticated"); + + return HttpClient.SendAsync(request); + } + } +} diff --git a/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertUseAffectedRowsTests.cs b/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertUseAffectedRowsTests.cs new file mode 100644 index 0000000000..88b5202b42 --- /dev/null +++ b/src/Service.Tests/SqlTests/RestApiTests/MySqlUpsertUseAffectedRowsTests.cs @@ -0,0 +1,165 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Net; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Service.Exceptions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.SqlTests.RestApiTests +{ + /// + /// Regression coverage for the MySQL upsert (PUT/PATCH) path when the connection is opened with + /// UseAffectedRows=true (changed-row semantics). In that mode, an authorized idempotent update - + /// one whose submitted values equal the row's current values - reports ROW_COUNT() = 0. The upsert + /// authorization decision must therefore be based on whether a row matched the primary key + update + /// policy, not on the number of changed rows; otherwise an authorized idempotent PUT/PATCH would be + /// incorrectly rejected with 403 DatabasePolicyFailure. + /// + [TestClass, TestCategory(TestCategory.MYSQL)] + public class MySqlUpsertUseAffectedRowsTests : RestApiTestBase + { + protected static Dictionary _queryMap = new() + { + { + // Row (100, 99) as seeded: categoryName 'Historical', piecesAvailable 0, piecesRequired 0. + "IdempotentUpdate_ExistingRow", + @" + SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName, + 'piecesAvailable',piecesAvailable,'piecesRequired',piecesRequired) AS data + FROM ( + SELECT categoryid, pieceid, categoryName,piecesAvailable,piecesRequired + FROM " + _Composite_NonAutoGenPK_TableName + @" + WHERE categoryid = 100 AND pieceid = 99 AND categoryName ='Historical' AND piecesAvailable = 0 + AND piecesRequired = 0 AND pieceid != 1 + ) AS subq + " + } + }; + + #region Test Fixture Setup + + /// + /// Sets up the test fixture once per class, opening the MySQL connection with UseAffectedRows=true + /// so the tests exercise changed-row semantics. + /// + [ClassInitialize] + public static async Task SetupAsync(TestContext context) + { + DatabaseEngine = TestCategory.MYSQL; + await InitializeTestFixture(connectionStringOptions: "UseAffectedRows=true"); + } + + /// + /// Runs after every test to reset the database state. + /// + [TestCleanup] + public async Task TestCleanup() + { + await ResetDbStateAsync(); + } + + #endregion + + public override string GetQuery(string key) + { + return _queryMap[key]; + } + + /// + /// An authorized PUT that resubmits the row's existing values (idempotent) must succeed with 200, + /// even under UseAffectedRows=true where ROW_COUNT() would be 0. The row (100,99) satisfies the + /// update policy "@item.pieceid ne 1". + /// + [TestMethod] + public async Task PutOneIdempotentUpdateWithDatabasePolicyUsingAffectedRows() + { + string requestBody = @" + { + ""categoryName"": ""Historical"", + ""piecesAvailable"": 0, + ""piecesRequired"": 0 + }"; + + await SetupAndRunRestApiTest( + primaryKeyRoute: "categoryid/100/pieceid/99", + queryString: null, + entityNameOrPath: _Composite_NonAutoGenPK_EntityPath, + sqlQuery: GetQuery("IdempotentUpdate_ExistingRow"), + operationType: EntityActionOperation.Upsert, + requestBody: requestBody, + expectedStatusCode: HttpStatusCode.OK, + clientRoleHeader: "database_policy_tester" + ); + } + + /// + /// An authorized PATCH that resubmits a field's existing value (idempotent) must succeed with 200, + /// even under UseAffectedRows=true where ROW_COUNT() would be 0. + /// + [TestMethod] + public async Task PatchOneIdempotentUpdateWithDatabasePolicyUsingAffectedRows() + { + string requestBody = @" + { + ""piecesAvailable"": 0 + }"; + + await SetupAndRunRestApiTest( + primaryKeyRoute: "categoryid/100/pieceid/99", + queryString: null, + entityNameOrPath: _Composite_NonAutoGenPK_EntityPath, + sqlQuery: GetQuery("IdempotentUpdate_ExistingRow"), + operationType: EntityActionOperation.UpsertIncremental, + requestBody: requestBody, + expectedStatusCode: HttpStatusCode.OK, + clientRoleHeader: "database_policy_tester" + ); + } + + /// + /// An unauthorized update (targeting a row that violates the update policy "@item.pieceid ne 1") + /// must still be rejected with 403 under UseAffectedRows=true - the changed-row semantics must not + /// weaken policy enforcement. + /// + [TestMethod] + public async Task PatchOneUnauthorizedUpdateIsBlockedUsingAffectedRows() + { + string requestBody = @" + { + ""categoryName"": ""SciFi"", + ""piecesRequired"": 5, + ""piecesAvailable"": 2 + }"; + + await SetupAndRunRestApiTest( + primaryKeyRoute: "categoryid/0/pieceid/1", + queryString: null, + entityNameOrPath: _Composite_NonAutoGenPK_EntityPath, + operationType: EntityActionOperation.UpsertIncremental, + requestBody: requestBody, + sqlQuery: string.Empty, + exceptionExpected: true, + expectedErrorMessage: DataApiBuilderException.AUTHORIZATION_FAILURE, + expectedStatusCode: HttpStatusCode.Forbidden, + expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure.ToString(), + clientRoleHeader: "database_policy_tester" + ); + + // Verify the row was not modified: it must still match its original seed values + // (categoryName='', piecesAvailable=0, piecesRequired=0). + string unchangedRow = await GetDatabaseResultAsync( + "SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName, " + + "'piecesAvailable', piecesAvailable, 'piecesRequired', piecesRequired) AS data " + + "FROM " + _Composite_NonAutoGenPK_TableName + " " + + "WHERE categoryid = 0 AND pieceid = 1 AND categoryName = '' AND piecesAvailable = 0 AND piecesRequired = 0"); + + Assert.AreNotEqual( + "[]", + unchangedRow, + "The row (categoryid=0, pieceid=1) must remain unmodified after a PATCH blocked by the update policy."); + } + } +} diff --git a/src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs index 8751bf2183..fbf6a81eac 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs @@ -95,6 +95,19 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', ) AS subq " }, + { + "PatchOneUpdateWithDatabasePolicy", + @" + SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName, + 'piecesAvailable',piecesAvailable,'piecesRequired',piecesRequired) AS data + FROM ( + SELECT categoryid, pieceid, categoryName,piecesAvailable,piecesRequired + FROM " + _Composite_NonAutoGenPK_TableName + @" + WHERE categoryid = 100 AND pieceid = 99 AND categoryName ='Historical' AND piecesAvailable = 4 + AND piecesRequired = 0 AND pieceid != 1 + ) AS subq + " + }, { "PatchOne_Insert_Empty_Test", @" @@ -295,51 +308,42 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'piecesAvailabl }; #region overridden tests - [TestMethod] - [Ignore] - public override Task PatchOneInsertInViewTest() - { - throw new NotImplementedException(); - } + // Create-action database policies are only supported for MSSQL and DWSQL. Since MySQL does not + // support a database policy on the create action, the PATCH tests that rely on a create policy + // (insert path) remain unsupported here. The update-policy path is validated by + // PatchOneUpdateWithDatabasePolicy and PatchOneUpdateWithUnsatisfiedDatabasePolicy. [TestMethod] [Ignore] - public override Task PatchOneUpdateViewTest() - { - throw new NotImplementedException(); - } - - [TestMethod] - [Ignore] - public void PatchOneViewBadRequestTest() + public override Task PatchOneInsertWithDatabasePolicy() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PatchOneUpdateWithUnsatisfiedDatabasePolicy() + public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy() + public override Task PatchOneInsertInViewTest() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PatchOneUpdateWithDatabasePolicy() + public override Task PatchOneUpdateViewTest() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PatchOneInsertWithDatabasePolicy() + public void PatchOneViewBadRequestTest() { throw new NotImplementedException(); } diff --git a/src/Service.Tests/SqlTests/RestApiTests/Put/MySqlPutApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Put/MySqlPutApiTests.cs index 9ba465f240..0ab7cf7c89 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Put/MySqlPutApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Put/MySqlPutApiTests.cs @@ -3,7 +3,10 @@ using System; using System.Collections.Generic; +using System.Net; using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace Azure.DataApiBuilder.Service.Tests.SqlTests.RestApiTests.Put @@ -97,6 +100,19 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', ) AS subq " }, + { + "PutOneUpdateWithDatabasePolicy", + @" + SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName, + 'piecesAvailable',piecesAvailable,'piecesRequired',piecesRequired) AS data + FROM ( + SELECT categoryid, pieceid, categoryName,piecesAvailable,piecesRequired + FROM " + _Composite_NonAutoGenPK_TableName + @" + WHERE categoryid = 100 AND pieceid = 99 AND categoryName ='SciFi' AND piecesAvailable = 4 + AND piecesRequired = 5 AND pieceid != 1 + ) AS subq + " + }, { "PutOne_Update_NullOutMissingField_Test", @" @@ -359,13 +375,10 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'piecesAvailabl #region overridden tests - [TestMethod] - [Ignore] - public override Task PutOneUpdateWithDatabasePolicy() - { - throw new NotImplementedException(); - } - + // Create-action database policies are only supported for MSSQL and DWSQL. Since MySQL does not + // support a database policy on the create action, the PUT tests that rely on a create policy + // (insert path) remain unsupported here. The update-policy path is validated by + // PutOneUpdateWithDatabasePolicy. [TestMethod] [Ignore] public override Task PutOneInsertWithDatabasePolicy() @@ -375,42 +388,42 @@ public override Task PutOneInsertWithDatabasePolicy() [TestMethod] [Ignore] - public override Task PutOneInsertInViewTest() + public override Task PutOneWithUnsatisfiedDatabasePolicy() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PutOneUpdateViewTest() + public override Task PutOneInsertInTableWithFieldsInDbPolicyNotPresentInBody() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public void PutOneInViewBadRequest(string expectedErrorMessage) + public override Task PutOneInsertInViewTest() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public void PutOneUpdateNonNullableDefaultFieldMissingFromJsonBodyTest() + public override Task PutOneUpdateViewTest() { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PutOneWithUnsatisfiedDatabasePolicy() + public void PutOneInViewBadRequest(string expectedErrorMessage) { throw new NotImplementedException(); } [TestMethod] [Ignore] - public override Task PutOneInsertInTableWithFieldsInDbPolicyNotPresentInBody() + public void PutOneUpdateNonNullableDefaultFieldMissingFromJsonBodyTest() { throw new NotImplementedException(); } @@ -457,5 +470,52 @@ public override string GetUniqueDbErrorMessage() { return "Column 'piecesRequired' cannot be null"; } + + /// + /// MySQL-specific negative PUT test for the update database policy. A PUT targeting an existing row + /// that violates the update policy ("@item.pieceid ne 1") must be rejected with 403 + /// DatabasePolicyFailure, and the targeted row must be left unmodified. + /// The inherited is skipped for MySQL + /// because it also exercises a create-action database policy, which MySQL does not support; this test + /// provides the denied-update coverage for the PUT path. + /// + [TestMethod] + public async Task PutOneUpdateWithUnsatisfiedDatabasePolicyIsBlocked() + { + // Row (categoryid=0, pieceid=1) exists in the seed with categoryName='', piecesAvailable=0, + // piecesRequired=0. pieceid=1 violates the update policy, so the PUT must be denied. + string requestBody = @" + { + ""categoryName"": ""SciFi"", + ""piecesRequired"": 2, + ""piecesAvailable"": 2 + }"; + + await SetupAndRunRestApiTest( + primaryKeyRoute: "categoryid/0/pieceid/1", + queryString: null, + entityNameOrPath: _Composite_NonAutoGenPK_EntityPath, + operationType: EntityActionOperation.Upsert, + requestBody: requestBody, + sqlQuery: string.Empty, + exceptionExpected: true, + expectedErrorMessage: DataApiBuilderException.AUTHORIZATION_FAILURE, + expectedStatusCode: HttpStatusCode.Forbidden, + expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure.ToString(), + clientRoleHeader: "database_policy_tester" + ); + + // Verify the row was not modified: it must still match its original seed values. + string unchangedRow = await GetDatabaseResultAsync( + "SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName, " + + "'piecesAvailable', piecesAvailable, 'piecesRequired', piecesRequired) AS data " + + "FROM " + _Composite_NonAutoGenPK_TableName + " " + + "WHERE categoryid = 0 AND pieceid = 1 AND categoryName = '' AND piecesAvailable = 0 AND piecesRequired = 0"); + + Assert.AreNotEqual( + "[]", + unchangedRow, + "The row (categoryid=0, pieceid=1) must remain unmodified after a PUT blocked by the update policy."); + } } } diff --git a/src/Service.Tests/SqlTests/SqlTestBase.cs b/src/Service.Tests/SqlTests/SqlTestBase.cs index f0bbd3258a..4b755eb5a2 100644 --- a/src/Service.Tests/SqlTests/SqlTestBase.cs +++ b/src/Service.Tests/SqlTests/SqlTestBase.cs @@ -84,13 +84,29 @@ public abstract class SqlTestBase protected async static Task InitializeTestFixture( List customQueries = null, List customEntities = null, - bool isRestBodyStrict = true) + bool isRestBodyStrict = true, + string connectionStringOptions = null) { TestHelper.SetupDatabaseEnvironment(DatabaseEngine); // Get the base config file from disk RuntimeConfig runtimeConfig = SqlTestHelper.SetupRuntimeConfig(); + // Append additional connection-string options for the test fixture (e.g. "UseAffectedRows=true"), + // when specified. This allows tests to exercise connection-string dependent behavior. + if (!string.IsNullOrEmpty(connectionStringOptions)) + { + string baseConnectionString = runtimeConfig.DataSource.ConnectionString; + string separator = baseConnectionString.TrimEnd().EndsWith(";") ? string.Empty : ";"; + runtimeConfig = runtimeConfig with + { + DataSource = runtimeConfig.DataSource with + { + ConnectionString = baseConnectionString + separator + connectionStringOptions + } + }; + } + // Setting the rest.request-body-strict flag as per the test fixtures. if (!isRestBodyStrict) {