Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aaronburtle wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config-generators/mysql-commands.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 50 additions & 17 deletions src/Core/Resolvers/MySqlQueryBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -113,29 +120,55 @@ 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 whether a record already exists for the given primary key BEFORE attempting the
// update/insert. This count is surfaced as the first result set and is used by the query
// executor to distinguish an update from an insert and to detect database policy failures.
string countExistingRows =
$"SET @cnt := (SELECT COUNT(*) FROM {tableName} WHERE {pkPredicates}); " +
$"SELECT @cnt AS {QuoteIdentifier(COUNT_ROWS_WITH_GIVEN_PK)};";

// Update honoring the update database policy. When the policy is not satisfied, zero rows
// match and the subsequent select returns no rows.
string updateQuery =
$"UPDATE {tableName} " +
$"SET {Build(structure.UpdateOperations, ", ")} " +
", " + updates +
$" WHERE {updatePredicates}; " +
$"SET @ROWCOUNT=ROW_COUNT(); " +
$"SELECT " + select + $" WHERE @ROWCOUNT > 0;";
Comment on lines +135 to +150

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;";
Comment on lines +159 to +169

return sets + ";\n" + countExistingRows + updateQuery + insertQuery;
}
}

Expand Down
100 changes: 100 additions & 0 deletions src/Core/Resolvers/MySqlQueryExecutor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -187,5 +189,103 @@ 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: the number of records already present for the given primary key.
/// result set #2: the output of the UPDATE (non-empty only when a record was updated).
/// 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: 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 only when a record was actually updated
// (i.e. it existed and satisfied the update database policy).
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 record was updated - 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;
}
}
}
17 changes: 17 additions & 0 deletions src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,23 @@
Action: Delete
}
]
},
{
Role: database_policy_tester,
Actions: [
{
Action: Read
},
{
Action: Create
},
{
Action: Update,
Policy: {
Database: @item.pieceid ne 1
}
}
]
}
],
Relationships: {
Expand Down
40 changes: 22 additions & 18 deletions src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
@"
Expand Down Expand Up @@ -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();
}
Expand Down
Loading
Loading