From c278965ff0f1e0cc75b7e5fc0731557f04fd7410 Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 13 Oct 2026 05:10:18 +0000 Subject: [PATCH 1/2] Support contains, startswith and endswith in database policies and $filter Database policies and REST $filter only supported binary comparison operators. OData string functions parsed but failed at request time because ODataASTVisitor and ParameterAliasRewriter did not handle SingleValueFunctionCallNode. - SQL: translate contains/startswith/endswith to `column LIKE @param`. The search value (constant, bound claim, or concat of those) is resolved in C#, LIKE-escaped with the GraphQL filter's rules (now a shared helper) and bound as a single parameter. - Cosmos DB: map to the native CONTAINS/STARTSWITH/ENDSWITH. - ParameterAliasRewriter binds claim aliases inside function arguments and treats a function call as a predicate. - Unsupported shapes fail closed, including comparing a function's result (`startswith(...) eq true`). - MsSqlQueryBuilder adds the LIKE ESCAPE clause to insert, update, delete and upsert predicates, not only SELECT, since policies apply to those operations too. - ODataASTFieldVisitor traverses function arguments so referenced columns still go through column-level authorization. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FRDKmtDzv5kTiSFnUxh3ow --- src/Core/Models/GraphQLFilterParsers.cs | 19 +-- src/Core/Parsers/ODataASTCosmosVisitor.cs | 16 +++ src/Core/Parsers/ODataASTFieldVisitor.cs | 15 +++ src/Core/Parsers/ODataASTVisitor.cs | 19 +++ src/Core/Parsers/ODataStringFunctionHelper.cs | 125 ++++++++++++++++++ src/Core/Parsers/ParameterAliasRewriter.cs | 10 ++ src/Core/Resolvers/MsSqlQueryBuilder.cs | 14 +- .../DatabasePolicyClaimBindingUnitTests.cs | 116 ++++++++++++++++ .../UnitTests/ODataASTVisitorUnitTests.cs | 5 + ...ionalUpsertConcurrencyQueryBuilderTests.cs | 21 +++ 10 files changed, 339 insertions(+), 21 deletions(-) create mode 100644 src/Core/Parsers/ODataStringFunctionHelper.cs diff --git a/src/Core/Models/GraphQLFilterParsers.cs b/src/Core/Models/GraphQLFilterParsers.cs index e3346930cd..f4167744c2 100644 --- a/src/Core/Models/GraphQLFilterParsers.cs +++ b/src/Core/Models/GraphQLFilterParsers.cs @@ -5,6 +5,7 @@ using Azure.DataApiBuilder.Config.DatabasePrimitives; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; +using Azure.DataApiBuilder.Core.Parsers; using Azure.DataApiBuilder.Core.Resolvers; using Azure.DataApiBuilder.Core.Services; using Azure.DataApiBuilder.Core.Services.MetadataProviders; @@ -747,7 +748,7 @@ public static Predicate Parse( else { op = PredicateOperation.LIKE; - value = $"%{EscapeLikeString((string)value)}%"; + value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; lengthOverride = true; } @@ -760,19 +761,19 @@ public static Predicate Parse( else { op = PredicateOperation.NOT_LIKE; - value = $"%{EscapeLikeString((string)value)}%"; + value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; lengthOverride = true; } break; case "startsWith": op = PredicateOperation.LIKE; - value = $"{EscapeLikeString((string)value)}%"; + value = $"{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; lengthOverride = true; break; case "endsWith": op = PredicateOperation.LIKE; - value = $"%{EscapeLikeString((string)value)}"; + value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}"; lengthOverride = true; break; case "isNull": @@ -871,14 +872,4 @@ private static PredicateOperand GenerateRightOperand( return new PredicateOperand(processLiteral ? $"{processLiterals(value, column.ColumnName, lengthOverride)}" : value.ToString()); } - - private static string EscapeLikeString(string input) - { - input = input.Replace(@"\", @"\\"); - input = input.Replace(@"%", @"\%"); - input = input.Replace(@"[", @"\["); - input = input.Replace(@"]", @"\]"); - input = input.Replace(@"_", @"\_"); - return input; - } } diff --git a/src/Core/Parsers/ODataASTCosmosVisitor.cs b/src/Core/Parsers/ODataASTCosmosVisitor.cs index 1912972766..4757af8f84 100644 --- a/src/Core/Parsers/ODataASTCosmosVisitor.cs +++ b/src/Core/Parsers/ODataASTCosmosVisitor.cs @@ -45,6 +45,8 @@ public override string Visit(SingleValuePropertyAccessNode nodeIn) /// String concatenation of (left op right). public override string Visit(BinaryOperatorNode nodeIn) { + ODataStringFunctionHelper.ThrowIfFunctionIsCompared(nodeIn); + string left = nodeIn.Left.Accept(this); string right = nodeIn.Right.Accept(this); @@ -113,6 +115,20 @@ public override string Visit(ConvertNode nodeIn) return nodeIn.Source.Accept(this); } + /// + /// Represents visiting a SingleValueFunctionCallNode for the string functions + /// contains, startswith and endswith, which map to the Cosmos DB functions of the same name. + /// + /// The node visited. + /// String representing the function call. + public override string Visit(SingleValueFunctionCallNode nodeIn) + { + (SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); + string column = field.Accept(this); + string param = _queryStructure.MakeDbConnectionParam(value); + return $"{nodeIn.Name.ToUpperInvariant()}({column}, {param})"; + } + /// /// Represents visiting a ConstantNode, which is what /// holds a value in the AST. diff --git a/src/Core/Parsers/ODataASTFieldVisitor.cs b/src/Core/Parsers/ODataASTFieldVisitor.cs index 05b9501868..87916d341c 100644 --- a/src/Core/Parsers/ODataASTFieldVisitor.cs +++ b/src/Core/Parsers/ODataASTFieldVisitor.cs @@ -77,5 +77,20 @@ public class ODataASTFieldVisitor : QueryNodeVisitor { return nodeIn.Source.Accept(this); } + + /// + /// Represents visiting a SingleValueFunctionCallNode, such as contains. + /// Calls accept on each argument to keep traversing AST. + /// + /// The node visited. + public override object? Visit(SingleValueFunctionCallNode nodeIn) + { + foreach (QueryNode parameter in nodeIn.Parameters) + { + parameter.Accept(this); + } + + return null; + } } } diff --git a/src/Core/Parsers/ODataASTVisitor.cs b/src/Core/Parsers/ODataASTVisitor.cs index 1d074e365d..8a484bf9e5 100644 --- a/src/Core/Parsers/ODataASTVisitor.cs +++ b/src/Core/Parsers/ODataASTVisitor.cs @@ -34,6 +34,8 @@ public ODataASTVisitor(BaseSqlQueryStructure structure, ISqlMetadataProvider met /// String concatenation of (left op right). public override string Visit(BinaryOperatorNode nodeIn) { + ODataStringFunctionHelper.ThrowIfFunctionIsCompared(nodeIn); + // In order traversal but add parens to maintain order of logical operations string left = nodeIn.Left.Accept(this); string right = nodeIn.Right.Accept(this); @@ -109,6 +111,23 @@ public override string Visit(ConvertNode nodeIn) return nodeIn.Source.Accept(this); } + /// + /// Represents visiting a SingleValueFunctionCallNode for the string functions + /// contains, startswith and endswith. These are translated to a LIKE predicate whose + /// pattern is a single parameter, with LIKE wildcards in the search value escaped. + /// + /// The node visited. + /// String representing the LIKE predicate. + public override string Visit(SingleValueFunctionCallNode nodeIn) + { + (SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); + string column = field.Accept(this); + _metadataProvider.TryGetBackingColumn(_struct.EntityName, field.Property.Name, out string? backingColumnName); + string pattern = ODataStringFunctionHelper.GetLikePattern(nodeIn.Name, value); + string param = _struct.MakeDbConnectionParam(pattern, backingColumnName, lengthOverride: true); + return $"({column} LIKE {param})"; + } + /// /// Gets the value of the parameter cast as the type the edm model associated to this parameter /// diff --git a/src/Core/Parsers/ODataStringFunctionHelper.cs b/src/Core/Parsers/ODataStringFunctionHelper.cs new file mode 100644 index 0000000000..6cd6506a1d --- /dev/null +++ b/src/Core/Parsers/ODataStringFunctionHelper.cs @@ -0,0 +1,125 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.OData.UriParser; + +namespace Azure.DataApiBuilder.Core.Parsers +{ + /// + /// Shared handling for the OData string functions supported in $filter and database policies: + /// contains, startswith and endswith. The first argument must be a field and the second must + /// resolve to a non-null string, either a constant (including a bound claim value) or a + /// concat of such values. Folding concat here keeps the whole search value in one parameter. + /// + internal static class ODataStringFunctionHelper + { + public const string CONTAINS = "contains"; + public const string STARTSWITH = "startswith"; + public const string ENDSWITH = "endswith"; + public const string CONCAT = "concat"; + + /// + /// Validates a string function call and returns its field and resolved search value. + /// + /// The function call node. + /// The field being searched and the search value. + /// The function or its arguments are not supported. + public static (SingleValuePropertyAccessNode Field, string Value) GetFieldAndValue(SingleValueFunctionCallNode nodeIn) + { + if (nodeIn.Name is not (CONTAINS or STARTSWITH or ENDSWITH)) + { + throw new NotSupportedException($"The function '{nodeIn.Name}' is not supported."); + } + + QueryNode[] arguments = nodeIn.Parameters.ToArray(); + if (arguments.Length != 2 || Unwrap(arguments[0]) is not SingleValuePropertyAccessNode field) + { + throw new NotSupportedException($"The first argument of '{nodeIn.Name}' must be a field."); + } + + string value = FoldString(arguments[1]) + ?? throw new NotSupportedException($"The second argument of '{nodeIn.Name}' must be a non-null string value."); + + return (field, value); + } + + /// + /// String functions are only supported as conditions in their own right. Comparing their + /// result, as in startswith(title, 'A') eq true, is rejected rather than translated. + /// + /// The binary operator node. + /// A function call is an operand of a comparison. + public static void ThrowIfFunctionIsCompared(BinaryOperatorNode nodeIn) + { + if (nodeIn.OperatorKind is not (BinaryOperatorKind.And or BinaryOperatorKind.Or) && + (Unwrap(nodeIn.Left) is SingleValueFunctionCallNode || Unwrap(nodeIn.Right) is SingleValueFunctionCallNode)) + { + throw new NotSupportedException("A function call cannot be an operand of a comparison; use it directly as a condition."); + } + } + + /// + /// Builds a LIKE pattern for the function, escaping LIKE wildcards in the value + /// with a backslash. + /// + public static string GetLikePattern(string functionName, string value) + { + string escaped = EscapeLikeString(value); + return functionName switch + { + CONTAINS => $"%{escaped}%", + STARTSWITH => $"{escaped}%", + ENDSWITH => $"%{escaped}", + _ => throw new NotSupportedException($"The function '{functionName}' is not supported.") + }; + } + + /// + /// Escapes LIKE wildcard characters with a backslash. + /// + public static string EscapeLikeString(string input) + { + input = input.Replace(@"\", @"\\"); + input = input.Replace(@"%", @"\%"); + input = input.Replace(@"[", @"\["); + input = input.Replace(@"]", @"\]"); + input = input.Replace(@"_", @"\_"); + return input; + } + + /// + /// Resolves a node to a string when it is a string constant or a concat of string constants. + /// Returns null for anything else, including null values. + /// + private static string? FoldString(QueryNode node) + { + switch (Unwrap(node)) + { + case ConstantNode { Value: string value }: + return value; + case SingleValueFunctionCallNode { Name: CONCAT } concat: + QueryNode[] parts = concat.Parameters.ToArray(); + if (parts.Length != 2) + { + return null; + } + + string? left = FoldString(parts[0]); + string? right = FoldString(parts[1]); + return left is null || right is null ? null : left + right; + default: + return null; + } + } + + private static QueryNode Unwrap(QueryNode node) + { + while (node is ConvertNode convertNode) + { + node = convertNode.Source; + } + + return node; + } + } +} diff --git a/src/Core/Parsers/ParameterAliasRewriter.cs b/src/Core/Parsers/ParameterAliasRewriter.cs index 3aef5624ab..204e6811db 100644 --- a/src/Core/Parsers/ParameterAliasRewriter.cs +++ b/src/Core/Parsers/ParameterAliasRewriter.cs @@ -58,6 +58,15 @@ private SingleValueNode RewriteNode(SingleValueNode node, bool isPredicate) ParameterAliasNode aliasNode => RewriteAlias(aliasNode, isPredicate), ConstantNode constantNode => NormalizeBooleanPredicate(constantNode, isPredicate), SingleValuePropertyAccessNode propertyNode => NormalizeBooleanPredicate(propertyNode, isPredicate), + SingleValueFunctionCallNode functionNode => new SingleValueFunctionCallNode( + functionNode.Name, + functionNode.Parameters + .Select(parameter => parameter is SingleValueNode valueNode + ? RewriteNode(valueNode, isPredicate: false) + : throw new ODataException( + $"Database policy function argument '{parameter.Kind}' is not supported for typed claim binding.")) + .ToList(), + functionNode.TypeReference), _ => throw new ODataException( $"Database policy expression node '{node.Kind}' is not supported for typed claim binding.") }; @@ -129,6 +138,7 @@ private static bool IsPredicateExpression(SingleValueNode node) { BinaryOperatorNode => true, UnaryOperatorNode => true, + SingleValueFunctionCallNode => true, ConvertNode convertNode => IsPredicateExpression(convertNode.Source), _ => false }; diff --git a/src/Core/Resolvers/MsSqlQueryBuilder.cs b/src/Core/Resolvers/MsSqlQueryBuilder.cs index d689ae0717..9b1dd9e7d8 100644 --- a/src/Core/Resolvers/MsSqlQueryBuilder.cs +++ b/src/Core/Resolvers/MsSqlQueryBuilder.cs @@ -79,7 +79,7 @@ public string Build(SqlInsertStructure structure) string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; // Predicates by virtue of database policy for Create action. - string dbPolicypredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); + string dbPolicypredicates = AddEscapeToLikeClauses(JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create))); // Columns whose values are provided in the request body - to be inserted into the record. string insertColumns = Build(structure.InsertColumns); @@ -217,9 +217,9 @@ public string Build(SqlUpdateStructure structure) SourceDefinition sourceDefinition = structure.GetUnderlyingSourceDefinition(); bool isUpdateTriggerEnabled = sourceDefinition.IsUpdateDMLTriggerEnabled; string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; - string predicates = JoinPredicateStrings( + string predicates = AddEscapeToLikeClauses(JoinPredicateStrings( structure.GetDbPolicyForOperation(EntityActionOperation.Update), - Build(structure.Predicates)); + Build(structure.Predicates))); string columnsToBeReturned = MakeOutputColumns(structure.OutputColumns, isUpdateTriggerEnabled ? string.Empty : OutputQualifier.Inserted.ToString()); @@ -254,9 +254,9 @@ public string Build(SqlUpdateStructure structure) /// public string Build(SqlDeleteStructure structure) { - string predicates = JoinPredicateStrings( + string predicates = AddEscapeToLikeClauses(JoinPredicateStrings( structure.GetDbPolicyForOperation(EntityActionOperation.Delete), - Build(structure.Predicates)); + Build(structure.Predicates))); return $"DELETE FROM {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " + $"WHERE {predicates} "; @@ -280,7 +280,7 @@ public string Build(SqlUpsertQueryStructure structure) string pkPredicates = JoinPredicateStrings(Build(structure.Predicates)); // Predicates by virtue of PK + database policy. - string updatePredicates = JoinPredicateStrings(pkPredicates, structure.GetDbPolicyForOperation(EntityActionOperation.Update)); + string updatePredicates = AddEscapeToLikeClauses(JoinPredicateStrings(pkPredicates, structure.GetDbPolicyForOperation(EntityActionOperation.Update))); string updateOperations = Build(structure.UpdateOperations, ", "); string columnsToBeReturned = @@ -338,7 +338,7 @@ public string Build(SqlUpsertQueryStructure structure) string insertColumns = Build(structure.InsertColumns); // Predicates added by virtue of database policy for create operation. - string createPredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); + string createPredicates = AddEscapeToLikeClauses(JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create))); // Query to insert record (if there exists none for given PK). StringBuilder insertQuery = new($"INSERT INTO {tableName} ({insertColumns}) "); diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index fb423de2c7..0589fb4036 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -355,6 +355,122 @@ public void ResolvedPolicyClaimValues_AreImmutableSnapshots() Assert.ThrowsException(() => emptyValues.Add("@claim", "value")); } + /// + /// Verifies a delimited contains policy folds the claim and delimiters into one bound + /// parameter: a LIKE pattern for SQL and a CONTAINS search value for Cosmos DB. + /// + [TestMethod] + public void DelimitedContainsClaim_IsBoundAsSingleParameterAcrossSqlAndCosmos() + { + const string policy = "contains(@item.textCol, concat(concat('|', @claims.value), '|'))"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", "123456", ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual("([textCol] LIKE @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, "%|123456|%"); + Assert.AreEqual(-1, sqlStructure.Parameters["@param0"].Length); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual("CONTAINS(c.textCol, @param0)", cosmosPredicate); + AssertParameterValues(cosmosStructure, "|123456|"); + } + + /// + /// Verifies LIKE wildcards and quotes in a claim are escaped and stay inside the bound + /// parameter for each supported string function. + /// + [DataTestMethod] + [DataRow("contains", "%50\\%\\_\\[x\\]' or 1 eq 1%", DisplayName = "contains")] + [DataRow("startswith", "50\\%\\_\\[x\\]' or 1 eq 1%", DisplayName = "startswith")] + [DataRow("endswith", "%50\\%\\_\\[x\\]' or 1 eq 1", DisplayName = "endswith")] + public void StringFunctionClaim_EscapesLikeWildcards(string function, string expectedPattern) + { + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + $"{function}(@item.textCol, @claims.value)", + new Claim("value", "50%_[x]' or 1 eq 1", ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual("([textCol] LIKE @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, expectedPattern); + } + + /// + /// Verifies a string function combined with a comparison is not rewritten as a comparison + /// to Boolean true. + /// + [TestMethod] + public void ContainsClaim_CombinedWithComparison_RemainsPredicate() + { + const string policy = "@item.intCol ne 6 and contains(@item.textCol, @claims.value)"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", "abc", ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual( + "(([intCol] != @param0) AND ([textCol] LIKE @param1))", + sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, 6, "%abc%"); + } + + /// + /// Verifies unsupported string function shapes fail closed before any parameter is collected. + /// + [DataTestMethod] + [DataRow("contains(@claims.value, 'x')", "value", DisplayName = "Claim as searched value")] + [DataRow("contains(@item.textCol, @item.textCol)", "value", DisplayName = "Field as search value")] + [DataRow("contains(@item.textCol, @claims.value)", null, DisplayName = "Null claim")] + [DataRow("tolower(@item.textCol) eq @claims.value", "value", DisplayName = "Unsupported function")] + [DataRow("contains(@item.textCol, @claims.value) eq true", "value", DisplayName = "Function compared to a value")] + public void UnsupportedStringFunctionPolicy_FailsClosed(string policy, string? claimValue) + { + Claim claim = claimValue is null + ? new Claim("value", "null", JsonClaimValueTypes.JsonNull) + : new Claim("value", claimValue, ClaimValueTypes.String); + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext(policy, claim); + Mock metadataProvider = CreateMetadataProvider(); + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + + Assert.ThrowsException(() => + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object)); + Assert.AreEqual(0, sqlStructure.Parameters.Count); + } + private static FilterClause ResolveFilterClause( AuthorizationResolver resolver, DefaultHttpContext context, diff --git a/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs b/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs index 358947c0f8..8d324dc347 100644 --- a/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs +++ b/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs @@ -70,6 +70,11 @@ public static async Task SetupAsync(TestContext context) [DataRow("uuid_types eq 9A19103F-16F7-4668-BE54-9A1E7A4F7556", "([uuid_types] = @param1)", DisplayName = "Equate uuid(guid) types.")] [DataRow("time_types eq 10:23:54.9999999", "([time_types] = @param1)", DisplayName = "Equate time types.")] [DataRow("time_types eq null", "([time_types] IS NULL)", DisplayName = "Equate time types for null.")] + [DataRow("contains(string_types, 'he_l%o')", "([string_types] LIKE @param1)", DisplayName = "Contains string.")] + [DataRow("startswith(string_types, 'he')", "([string_types] LIKE @param1)", DisplayName = "Startswith string.")] + [DataRow("endswith(string_types, 'lo')", "([string_types] LIKE @param1)", DisplayName = "Endswith string.")] + [DataRow("contains(string_types, concat('|', concat('he', '|')))", "([string_types] LIKE @param1)", + DisplayName = "Contains concatenated string.")] [TestMethod] public void VisitorLeftFieldRightConstantFilterTest(string filterExp, string expectedPredicate) { diff --git a/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs b/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs index cc2000f6d7..094bb91294 100644 --- a/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs +++ b/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs @@ -161,6 +161,27 @@ public void UpdateOnlyFallbackUpsertsDoNotAcquireInsertSerializationLocks() Assert.IsFalse(dwSqlQuery.Contains("TABLOCKX", StringComparison.Ordinal), $"Update-only Data Warehouse SQL query should not acquire an insert serialization lock. Query: {dwSqlQuery}"); } + /// + /// SQL Server needs an explicit ESCAPE clause for the backslash-escaped LIKE patterns that string + /// functions in database policies produce, for mutations as well as reads. + /// + [TestMethod] + public void MsSqlUpsertAddsEscapeClauseToLikeInDatabasePolicy() + { + SqlUpsertQueryStructure structure = CreateUpsertStructure(DatabaseType.MSSQL, useCompositePrimaryKey: false); + structure.DbPolicyPredicatesForOperations[EntityActionOperation.Update] = "([title] LIKE @param8)"; + structure.DbPolicyPredicatesForOperations[EntityActionOperation.Create] = "([title] LIKE @param9)"; + + string query = new MsSqlQueryBuilder().Build(structure); + + Assert.IsTrue( + query.Contains(@"([title] LIKE @param8 ESCAPE '\')", StringComparison.Ordinal), + $"Expected the update policy LIKE to carry an ESCAPE clause. Query: {query}"); + Assert.IsTrue( + query.Contains(@"([title] LIKE @param9 ESCAPE '\')", StringComparison.Ordinal), + $"Expected the create policy LIKE to carry an ESCAPE clause. Query: {query}"); + } + private static void AssertAppearsBefore(string query, string first, string second) { int firstIndex = query.IndexOf(first, StringComparison.Ordinal); From 24ce54351b52f2a5817bec7430a8d7b3c18a28fe Mon Sep 17 00:00:00 2001 From: Richard Date: Fri, 13 Nov 2026 15:42:57 +1300 Subject: [PATCH 2/2] Address review feedback on string function policies - Render LIKE through IQueryBuilder.BuildLikePredicate so each dialect decides how to escape. SQL Server adds ESCAPE, DWSQL rejects the functions since it has no ESCAPE support yet. This replaces the regex wrapping added to the mutation builders. - Skip LIKE clauses that already have ESCAPE in the SQL Server read path regex. - Move LIKE escaping to SqlLikePattern so GraphQL filters no longer depend on the OData helper. Validate the function name once. - Reject empty search values, which matched every row. - Return a clear 403 when a non-string claim is used in a string function or concat. - Map Cosmos policy translation failures to AuthorizationCheckFailed and map Cosmos function names explicitly. - Resolve the backing column once in the LIKE visitor. --- src/Core/Models/GraphQLFilterParsers.cs | 9 +- src/Core/Parsers/ODataASTCosmosVisitor.cs | 12 +- src/Core/Parsers/ODataASTVisitor.cs | 30 ++--- src/Core/Parsers/ODataStringFunctionHelper.cs | 64 ++++------ .../Resolvers/AuthorizationPolicyHelpers.cs | 57 +++++++-- src/Core/Resolvers/DWSqlQueryBuilder.cs | 6 + src/Core/Resolvers/IQueryBuilder.cs | 5 + src/Core/Resolvers/MsSqlQueryBuilder.cs | 24 ++-- src/Core/Resolvers/SqlLikePattern.cs | 43 +++++++ .../DatabasePolicyClaimBindingUnitTests.cs | 110 +++++++++++++++++- .../UnitTests/ODataASTVisitorUnitTests.cs | 8 +- ...ionalUpsertConcurrencyQueryBuilderTests.cs | 21 ---- 12 files changed, 275 insertions(+), 114 deletions(-) create mode 100644 src/Core/Resolvers/SqlLikePattern.cs diff --git a/src/Core/Models/GraphQLFilterParsers.cs b/src/Core/Models/GraphQLFilterParsers.cs index f4167744c2..0476f236f1 100644 --- a/src/Core/Models/GraphQLFilterParsers.cs +++ b/src/Core/Models/GraphQLFilterParsers.cs @@ -5,7 +5,6 @@ using Azure.DataApiBuilder.Config.DatabasePrimitives; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; -using Azure.DataApiBuilder.Core.Parsers; using Azure.DataApiBuilder.Core.Resolvers; using Azure.DataApiBuilder.Core.Services; using Azure.DataApiBuilder.Core.Services.MetadataProviders; @@ -748,7 +747,7 @@ public static Predicate Parse( else { op = PredicateOperation.LIKE; - value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; + value = SqlLikePattern.Create(LikeMatch.Contains, (string)value); lengthOverride = true; } @@ -761,19 +760,19 @@ public static Predicate Parse( else { op = PredicateOperation.NOT_LIKE; - value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; + value = SqlLikePattern.Create(LikeMatch.Contains, (string)value); lengthOverride = true; } break; case "startsWith": op = PredicateOperation.LIKE; - value = $"{ODataStringFunctionHelper.EscapeLikeString((string)value)}%"; + value = SqlLikePattern.Create(LikeMatch.StartsWith, (string)value); lengthOverride = true; break; case "endsWith": op = PredicateOperation.LIKE; - value = $"%{ODataStringFunctionHelper.EscapeLikeString((string)value)}"; + value = SqlLikePattern.Create(LikeMatch.EndsWith, (string)value); lengthOverride = true; break; case "isNull": diff --git a/src/Core/Parsers/ODataASTCosmosVisitor.cs b/src/Core/Parsers/ODataASTCosmosVisitor.cs index 4757af8f84..72d20d1b97 100644 --- a/src/Core/Parsers/ODataASTCosmosVisitor.cs +++ b/src/Core/Parsers/ODataASTCosmosVisitor.cs @@ -123,10 +123,18 @@ public override string Visit(ConvertNode nodeIn) /// String representing the function call. public override string Visit(SingleValueFunctionCallNode nodeIn) { - (SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); + (LikeMatch match, SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); + string cosmosFunction = match switch + { + LikeMatch.Contains => "CONTAINS", + LikeMatch.StartsWith => "STARTSWITH", + LikeMatch.EndsWith => "ENDSWITH", + _ => throw new NotSupportedException($"The function '{nodeIn.Name}' is not supported.") + }; string column = field.Accept(this); string param = _queryStructure.MakeDbConnectionParam(value); - return $"{nodeIn.Name.ToUpperInvariant()}({column}, {param})"; + + return $"{cosmosFunction}({column}, {param})"; } /// diff --git a/src/Core/Parsers/ODataASTVisitor.cs b/src/Core/Parsers/ODataASTVisitor.cs index 8a484bf9e5..931227a809 100644 --- a/src/Core/Parsers/ODataASTVisitor.cs +++ b/src/Core/Parsers/ODataASTVisitor.cs @@ -73,13 +73,7 @@ public override string Visit(UnaryOperatorNode nodeIn) /// String representing the Field name public override string Visit(SingleValuePropertyAccessNode nodeIn) { - _metadataProvider.TryGetBackingColumn(_struct.EntityName, nodeIn.Property.Name, out string? backingColumnName); - if (_operation is EntityActionOperation.Create) - { - _struct.FieldsReferencedInDbPolicyForCreateAction.Add(backingColumnName!); - } - - return _metadataProvider.GetQueryBuilder().QuoteIdentifier(backingColumnName!); + return _metadataProvider.GetQueryBuilder().QuoteIdentifier(GetBackingColumn(nodeIn)); } /// @@ -120,12 +114,22 @@ public override string Visit(ConvertNode nodeIn) /// String representing the LIKE predicate. public override string Visit(SingleValueFunctionCallNode nodeIn) { - (SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); - string column = field.Accept(this); - _metadataProvider.TryGetBackingColumn(_struct.EntityName, field.Property.Name, out string? backingColumnName); - string pattern = ODataStringFunctionHelper.GetLikePattern(nodeIn.Name, value); - string param = _struct.MakeDbConnectionParam(pattern, backingColumnName, lengthOverride: true); - return $"({column} LIKE {param})"; + (LikeMatch match, SingleValuePropertyAccessNode field, string value) = ODataStringFunctionHelper.GetFieldAndValue(nodeIn); + IQueryBuilder queryBuilder = _metadataProvider.GetQueryBuilder(); + string backingColumnName = GetBackingColumn(field); + string param = _struct.MakeDbConnectionParam(SqlLikePattern.Create(match, value), backingColumnName, lengthOverride: true); + return $"({queryBuilder.BuildLikePredicate(queryBuilder.QuoteIdentifier(backingColumnName), param)})"; + } + + private string GetBackingColumn(SingleValuePropertyAccessNode nodeIn) + { + _metadataProvider.TryGetBackingColumn(_struct.EntityName, nodeIn.Property.Name, out string? backingColumnName); + if (_operation is EntityActionOperation.Create) + { + _struct.FieldsReferencedInDbPolicyForCreateAction.Add(backingColumnName!); + } + + return backingColumnName!; } /// diff --git a/src/Core/Parsers/ODataStringFunctionHelper.cs b/src/Core/Parsers/ODataStringFunctionHelper.cs index 6cd6506a1d..5059f4fcb0 100644 --- a/src/Core/Parsers/ODataStringFunctionHelper.cs +++ b/src/Core/Parsers/ODataStringFunctionHelper.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Azure.DataApiBuilder.Core.Resolvers; using Microsoft.OData.UriParser; namespace Azure.DataApiBuilder.Core.Parsers @@ -8,28 +9,31 @@ namespace Azure.DataApiBuilder.Core.Parsers /// /// Shared handling for the OData string functions supported in $filter and database policies: /// contains, startswith and endswith. The first argument must be a field and the second must - /// resolve to a non-null string, either a constant (including a bound claim value) or a + /// resolve to a non-empty string, either a constant (including a bound claim value) or a /// concat of such values. Folding concat here keeps the whole search value in one parameter. /// internal static class ODataStringFunctionHelper { - public const string CONTAINS = "contains"; - public const string STARTSWITH = "startswith"; - public const string ENDSWITH = "endswith"; - public const string CONCAT = "concat"; + private const string CONTAINS = "contains"; + private const string STARTSWITH = "startswith"; + private const string ENDSWITH = "endswith"; + private const string CONCAT = "concat"; /// - /// Validates a string function call and returns its field and resolved search value. + /// Validates a string function call and returns its match type, field and resolved search value. /// /// The function call node. - /// The field being searched and the search value. + /// The match type, the field being searched and the search value. /// The function or its arguments are not supported. - public static (SingleValuePropertyAccessNode Field, string Value) GetFieldAndValue(SingleValueFunctionCallNode nodeIn) + public static (LikeMatch Match, SingleValuePropertyAccessNode Field, string Value) GetFieldAndValue(SingleValueFunctionCallNode nodeIn) { - if (nodeIn.Name is not (CONTAINS or STARTSWITH or ENDSWITH)) + LikeMatch match = nodeIn.Name switch { - throw new NotSupportedException($"The function '{nodeIn.Name}' is not supported."); - } + CONTAINS => LikeMatch.Contains, + STARTSWITH => LikeMatch.StartsWith, + ENDSWITH => LikeMatch.EndsWith, + _ => throw new NotSupportedException($"The function '{nodeIn.Name}' is not supported.") + }; QueryNode[] arguments = nodeIn.Parameters.ToArray(); if (arguments.Length != 2 || Unwrap(arguments[0]) is not SingleValuePropertyAccessNode field) @@ -37,10 +41,13 @@ public static (SingleValuePropertyAccessNode Field, string Value) GetFieldAndVal throw new NotSupportedException($"The first argument of '{nodeIn.Name}' must be a field."); } - string value = FoldString(arguments[1]) - ?? throw new NotSupportedException($"The second argument of '{nodeIn.Name}' must be a non-null string value."); + string? value = FoldString(arguments[1]); + if (string.IsNullOrEmpty(value)) + { + throw new NotSupportedException($"The second argument of '{nodeIn.Name}' must be a non-empty string value."); + } - return (field, value); + return (match, field, value); } /// @@ -58,35 +65,6 @@ public static void ThrowIfFunctionIsCompared(BinaryOperatorNode nodeIn) } } - /// - /// Builds a LIKE pattern for the function, escaping LIKE wildcards in the value - /// with a backslash. - /// - public static string GetLikePattern(string functionName, string value) - { - string escaped = EscapeLikeString(value); - return functionName switch - { - CONTAINS => $"%{escaped}%", - STARTSWITH => $"{escaped}%", - ENDSWITH => $"%{escaped}", - _ => throw new NotSupportedException($"The function '{functionName}' is not supported.") - }; - } - - /// - /// Escapes LIKE wildcard characters with a backslash. - /// - public static string EscapeLikeString(string input) - { - input = input.Replace(@"\", @"\\"); - input = input.Replace(@"%", @"\%"); - input = input.Replace(@"[", @"\["); - input = input.Replace(@"]", @"\]"); - input = input.Replace(@"_", @"\_"); - return input; - } - /// /// Resolves a node to a string when it is a string constant or a concat of string constants. /// Returns null for anything else, including null values. diff --git a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index 408b0f30d4..1d7ec7dbc5 100644 --- a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs +++ b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Net; +using System.Text.RegularExpressions; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; using Azure.DataApiBuilder.Config.ObjectModel; @@ -11,6 +13,7 @@ using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; +using Microsoft.OData; using Microsoft.OData.UriParser; namespace Azure.DataApiBuilder.Core.Resolvers @@ -105,17 +108,13 @@ public static void ProcessAuthorizationPolicies( cosmosQueryStructure.TableCounter.Next(); fromClause = pathConfig.JoinStatement; - predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( - pathConfig.Alias, - cosmosQueryStructure)); + predicates = GetCosmosPolicyPredicate(filterClause, pathConfig.Alias, cosmosQueryStructure); existQuery = CosmosQueryBuilder.BuildExistsQueryForCosmos(fromClause, predicates); } else { - predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( - GetCosmosPolicyPathPrefix(pathConfig), - cosmosQueryStructure)); + predicates = GetCosmosPolicyPredicate(filterClause, GetCosmosPolicyPathPrefix(pathConfig), cosmosQueryStructure); } if (pathConfig.EntityName == entity.Key) @@ -137,6 +136,25 @@ public static void ProcessAuthorizationPolicies( } } + /// + /// Translates a policy to a Cosmos DB predicate. + /// + internal static string GetCosmosPolicyPredicate(FilterClause filterClause, string prefix, BaseQueryStructure queryStructure) + { + try + { + return filterClause.Expression.Accept(new ODataASTCosmosVisitor(prefix, queryStructure)); + } + catch (Exception ex) + { + throw new DataApiBuilderException( + message: "Policy query parameter is not well formed for Cosmos DB Policy Processing.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, + innerException: ex); + } + } + /// /// Builds the Cosmos document path used to qualify fields in a database policy. /// Root model paths do not have a column segment and must not end with a dot. @@ -221,16 +239,33 @@ private static List ProcessFilter( // Parse and save the values that are needed to later generate SQL query predicates // FilterClauseInDbPolicy is an Abstract Syntax Tree representing the parsed policy text. - return sqlMetadataProvider.GetODataParser().GetFilterClause( - filterQueryString: dbPolicyClause, - resourcePath: resourcePath, - customResolver: new ClaimsTypeDataUriResolver(claimValueNodes), - parameterAliasNodes: claimValueNodes); + try + { + return sqlMetadataProvider.GetODataParser().GetFilterClause( + filterQueryString: dbPolicyClause, + resourcePath: resourcePath, + customResolver: new ClaimsTypeDataUriResolver(claimValueNodes), + parameterAliasNodes: claimValueNodes); + } + catch (DataApiBuilderException ex) when (ex.InnerException is ODataException && UsesStringFunctionWithNonStringClaim(dbPolicy)) + { + throw new DataApiBuilderException( + message: "Claims used in contains, startswith, endswith or concat in a database policy must be strings.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, + innerException: ex); + } } return null; } + private static bool UsesStringFunctionWithNonStringClaim(ResolvedDatabasePolicy dbPolicy) + { + return Regex.IsMatch(dbPolicy.Policy, @"\b(contains|startswith|endswith|concat)\s*\(") && + dbPolicy.ClaimValues.Values.Any(value => value is not null and not string); + } + /// /// Resolves compound operations like Upsert,UpsertIncremental into the corresponding constituent elemental operations i.e. Create,Update. /// For simple operations, returns the operation itself. diff --git a/src/Core/Resolvers/DWSqlQueryBuilder.cs b/src/Core/Resolvers/DWSqlQueryBuilder.cs index f908c21d38..79a573667f 100644 --- a/src/Core/Resolvers/DWSqlQueryBuilder.cs +++ b/src/Core/Resolvers/DWSqlQueryBuilder.cs @@ -30,6 +30,12 @@ public override string QuoteIdentifier(string ident) return _builder.QuoteIdentifier(ident); } + /// + public string BuildLikePredicate(string column, string pattern) + { + throw new NotSupportedException("LIKE patterns with escaped wildcards are not supported for DWSQL."); + } + /// /// Builds the sql query that will return the json result for DW. /// Sample: Get title of all books. diff --git a/src/Core/Resolvers/IQueryBuilder.cs b/src/Core/Resolvers/IQueryBuilder.cs index f6a6222ddc..6db9bad395 100644 --- a/src/Core/Resolvers/IQueryBuilder.cs +++ b/src/Core/Resolvers/IQueryBuilder.cs @@ -91,6 +91,11 @@ public interface IQueryBuilder /// public string QuoteTableNameAsDBConnectionParam(string param); + /// + /// Builds a LIKE predicate for a pattern whose wildcards are escaped with a backslash. + /// + string BuildLikePredicate(string column, string pattern) => $"{column} LIKE {pattern}"; + public string BuildGetAutoentitiesQuery() => throw new NotSupportedException($"{GetType().Name} does not support Autoentities yet."); } } diff --git a/src/Core/Resolvers/MsSqlQueryBuilder.cs b/src/Core/Resolvers/MsSqlQueryBuilder.cs index 9b1dd9e7d8..0e8792fc11 100644 --- a/src/Core/Resolvers/MsSqlQueryBuilder.cs +++ b/src/Core/Resolvers/MsSqlQueryBuilder.cs @@ -64,11 +64,17 @@ public string Build(SqlQueryStructure structure) /// /// /// - private static string AddEscapeToLikeClauses(string predicate) + internal static string AddEscapeToLikeClauses(string predicate) { const string escapeClause = $" ESCAPE '{MSSQL_ESCAPE_CHAR}'"; // Regex to find LIKE clauses and append ESCAPE - return Regex.Replace(predicate, @"(LIKE\s+@[\w\d]+)", $"$1{escapeClause}", RegexOptions.IgnoreCase); + return Regex.Replace(predicate, @"(LIKE\s+@\w+)\b(?!\s+ESCAPE)", $"$1{escapeClause}", RegexOptions.IgnoreCase); + } + + /// + public string BuildLikePredicate(string column, string pattern) + { + return $"{column} LIKE {pattern} ESCAPE '{MSSQL_ESCAPE_CHAR}'"; } /// @@ -79,7 +85,7 @@ public string Build(SqlInsertStructure structure) string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; // Predicates by virtue of database policy for Create action. - string dbPolicypredicates = AddEscapeToLikeClauses(JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create))); + string dbPolicypredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); // Columns whose values are provided in the request body - to be inserted into the record. string insertColumns = Build(structure.InsertColumns); @@ -217,9 +223,9 @@ public string Build(SqlUpdateStructure structure) SourceDefinition sourceDefinition = structure.GetUnderlyingSourceDefinition(); bool isUpdateTriggerEnabled = sourceDefinition.IsUpdateDMLTriggerEnabled; string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; - string predicates = AddEscapeToLikeClauses(JoinPredicateStrings( + string predicates = JoinPredicateStrings( structure.GetDbPolicyForOperation(EntityActionOperation.Update), - Build(structure.Predicates))); + Build(structure.Predicates)); string columnsToBeReturned = MakeOutputColumns(structure.OutputColumns, isUpdateTriggerEnabled ? string.Empty : OutputQualifier.Inserted.ToString()); @@ -254,9 +260,9 @@ public string Build(SqlUpdateStructure structure) /// public string Build(SqlDeleteStructure structure) { - string predicates = AddEscapeToLikeClauses(JoinPredicateStrings( + string predicates = JoinPredicateStrings( structure.GetDbPolicyForOperation(EntityActionOperation.Delete), - Build(structure.Predicates))); + Build(structure.Predicates)); return $"DELETE FROM {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " + $"WHERE {predicates} "; @@ -280,7 +286,7 @@ public string Build(SqlUpsertQueryStructure structure) string pkPredicates = JoinPredicateStrings(Build(structure.Predicates)); // Predicates by virtue of PK + database policy. - string updatePredicates = AddEscapeToLikeClauses(JoinPredicateStrings(pkPredicates, structure.GetDbPolicyForOperation(EntityActionOperation.Update))); + string updatePredicates = JoinPredicateStrings(pkPredicates, structure.GetDbPolicyForOperation(EntityActionOperation.Update)); string updateOperations = Build(structure.UpdateOperations, ", "); string columnsToBeReturned = @@ -338,7 +344,7 @@ public string Build(SqlUpsertQueryStructure structure) string insertColumns = Build(structure.InsertColumns); // Predicates added by virtue of database policy for create operation. - string createPredicates = AddEscapeToLikeClauses(JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create))); + string createPredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); // Query to insert record (if there exists none for given PK). StringBuilder insertQuery = new($"INSERT INTO {tableName} ({insertColumns}) "); diff --git a/src/Core/Resolvers/SqlLikePattern.cs b/src/Core/Resolvers/SqlLikePattern.cs new file mode 100644 index 0000000000..2431c6463f --- /dev/null +++ b/src/Core/Resolvers/SqlLikePattern.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Azure.DataApiBuilder.Core.Resolvers +{ + /// + /// Where a search value must appear in a string. + /// + internal enum LikeMatch + { + Contains, + StartsWith, + EndsWith + } + + /// + /// Builds LIKE patterns with wildcards escaped by a backslash. + /// + internal static class SqlLikePattern + { + public static string Create(LikeMatch match, string value) + { + string escaped = Escape(value); + return match switch + { + LikeMatch.Contains => $"%{escaped}%", + LikeMatch.StartsWith => $"{escaped}%", + LikeMatch.EndsWith => $"%{escaped}", + _ => throw new ArgumentOutOfRangeException(nameof(match)) + }; + } + + public static string Escape(string input) + { + input = input.Replace(@"\", @"\\"); + input = input.Replace(@"%", @"\%"); + input = input.Replace(@"[", @"\["); + input = input.Replace(@"]", @"\]"); + input = input.Replace(@"_", @"\_"); + return input; + } + } +} diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index 0589fb4036..cd92737442 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -376,7 +376,7 @@ public void DelimitedContainsClaim_IsBoundAsSingleParameterAcrossSqlAndCosmos() resolver, metadataProvider.Object); - Assert.AreEqual("([textCol] LIKE @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + Assert.AreEqual("([textCol] LIKE @param0 ESCAPE '\\')", sqlStructure.GetDbPolicyForOperation(OPERATION)); AssertParameterValues(sqlStructure, "%|123456|%"); Assert.AreEqual(-1, sqlStructure.Parameters["@param0"].Length); @@ -412,10 +412,26 @@ public void StringFunctionClaim_EscapesLikeWildcards(string function, string exp resolver, metadataProvider.Object); - Assert.AreEqual("([textCol] LIKE @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + Assert.AreEqual("([textCol] LIKE @param0 ESCAPE '\\')", sqlStructure.GetDbPolicyForOperation(OPERATION)); AssertParameterValues(sqlStructure, expectedPattern); } + /// + /// Verifies the SQL Server read path adds ESCAPE to GraphQL LIKE predicates without + /// adding a second one to policy and $filter predicates that already have it. + /// + [DataTestMethod] + [DataRow(@"([title] LIKE @param10)", @"([title] LIKE @param10 ESCAPE '\')", DisplayName = "No ESCAPE")] + [DataRow(@"([title] LIKE @param10 ESCAPE '\')", @"([title] LIKE @param10 ESCAPE '\')", DisplayName = "Existing ESCAPE")] + [DataRow( + @"([textCol] LIKE @param0 ESCAPE '\') AND ([title] NOT LIKE @param1)", + @"([textCol] LIKE @param0 ESCAPE '\') AND ([title] NOT LIKE @param1 ESCAPE '\')", + DisplayName = "Mixed")] + public void MsSqlLikeEscape_IsAddedOnce(string predicate, string expected) + { + Assert.AreEqual(expected, MsSqlQueryBuilder.AddEscapeToLikeClauses(predicate)); + } + /// /// Verifies a string function combined with a comparison is not rewritten as a comparison /// to Boolean true. @@ -438,7 +454,7 @@ public void ContainsClaim_CombinedWithComparison_RemainsPredicate() metadataProvider.Object); Assert.AreEqual( - "(([intCol] != @param0) AND ([textCol] LIKE @param1))", + "(([intCol] != @param0) AND ([textCol] LIKE @param1 ESCAPE '\\'))", sqlStructure.GetDbPolicyForOperation(OPERATION)); AssertParameterValues(sqlStructure, 6, "%abc%"); } @@ -450,6 +466,9 @@ public void ContainsClaim_CombinedWithComparison_RemainsPredicate() [DataRow("contains(@claims.value, 'x')", "value", DisplayName = "Claim as searched value")] [DataRow("contains(@item.textCol, @item.textCol)", "value", DisplayName = "Field as search value")] [DataRow("contains(@item.textCol, @claims.value)", null, DisplayName = "Null claim")] + [DataRow("contains(@item.textCol, @claims.value)", "", DisplayName = "Empty claim")] + [DataRow("startswith(@item.textCol, @claims.value)", "", DisplayName = "Empty claim with startswith")] + [DataRow("contains(@item.textCol, '')", "value", DisplayName = "Empty constant")] [DataRow("tolower(@item.textCol) eq @claims.value", "value", DisplayName = "Unsupported function")] [DataRow("contains(@item.textCol, @claims.value) eq true", "value", DisplayName = "Function compared to a value")] public void UnsupportedStringFunctionPolicy_FailsClosed(string policy, string? claimValue) @@ -471,6 +490,84 @@ public void UnsupportedStringFunctionPolicy_FailsClosed(string policy, string? c Assert.AreEqual(0, sqlStructure.Parameters.Count); } + /// + /// Verifies rejected string function policies fail the authorization check for Cosmos DB. + /// + [DataTestMethod] + [DataRow("contains(@claims.value, 'x')", "value", DisplayName = "Claim as searched value")] + [DataRow("contains(@item.textCol, @item.textCol)", "value", DisplayName = "Field as search value")] + [DataRow("contains(@item.textCol, @claims.value)", null, DisplayName = "Null claim")] + [DataRow("contains(@item.textCol, @claims.value)", "", DisplayName = "Empty claim")] + [DataRow("contains(@item.textCol, @claims.value) eq true", "value", DisplayName = "Function compared to a value")] + public void UnsupportedStringFunctionPolicy_FailsAuthorizationForCosmos(string policy, string? claimValue) + { + Claim claim = claimValue is null + ? new Claim("value", "null", JsonClaimValueTypes.JsonNull) + : new Claim("value", claimValue, ClaimValueTypes.String); + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext(policy, claim); + Mock metadataProvider = CreateMetadataProvider(); + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + + DataApiBuilderException exception = Assert.ThrowsException(() => + AuthorizationPolicyHelpers.GetCosmosPolicyPredicate(filterClause, "c", cosmosStructure)); + + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); + Assert.AreEqual(0, cosmosStructure.Parameters.Count); + } + + /// + /// Verifies string functions are rejected for DWSQL. + /// + [TestMethod] + public void StringFunctionPolicy_IsRejectedForDwSql() + { + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + "not contains(@item.textCol, @claims.value)", + new Claim("value", "_", ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(DatabaseType.DWSQL); + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + + DataApiBuilderException exception = Assert.ThrowsException(() => + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object)); + + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); + } + + /// + /// Verifies a numeric claim in a string function gives a clear error. + /// + [DataTestMethod] + [DataRow("contains(@item.textCol, @claims.value)", ClaimValueTypes.Integer32, DisplayName = "Integer32 search value")] + [DataRow("contains(@item.textCol, @claims.value)", ClaimValueTypes.Integer64, DisplayName = "Integer64 search value")] + [DataRow("contains(@item.textCol, concat(concat('|', @claims.value), '|'))", ClaimValueTypes.Integer32, DisplayName = "Integer32 in concat")] + [DataRow("contains(@item.textCol, concat(concat('|', @claims.value), '|'))", ClaimValueTypes.Integer64, DisplayName = "Integer64 in concat")] + public void NumericClaimInStringFunction_FailsWithClearError(string policy, string claimType) + { + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", "123456", claimType)); + Mock metadataProvider = CreateMetadataProvider(); + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + + DataApiBuilderException exception = Assert.ThrowsException(() => + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object)); + + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); + StringAssert.Contains(exception.Message, "must be strings"); + Assert.AreEqual(0, sqlStructure.Parameters.Count); + } + private static FilterClause ResolveFilterClause( AuthorizationResolver resolver, DefaultHttpContext context, @@ -514,7 +611,7 @@ private static (AuthorizationResolver Resolver, DefaultHttpContext Context) Crea return (resolver, context); } - private static Mock CreateMetadataProvider() + private static Mock CreateMetadataProvider(DatabaseType databaseType = DatabaseType.MSSQL) { SourceDefinition sourceDefinition = new(); sourceDefinition.Columns.Add("id", new ColumnDefinition(typeof(int)) { DbType = DbType.Int32 }); @@ -536,8 +633,9 @@ private static Mock CreateMetadataProvider() metadataProvider.Setup(provider => provider.GetLinkingEntities()) .Returns(new Dictionary()); metadataProvider.Setup(provider => provider.GetSourceDefinition(ENTITY_NAME)).Returns(sourceDefinition); - metadataProvider.Setup(provider => provider.GetDatabaseType()).Returns(DatabaseType.MSSQL); - metadataProvider.Setup(provider => provider.GetQueryBuilder()).Returns(new MsSqlQueryBuilder()); + metadataProvider.Setup(provider => provider.GetDatabaseType()).Returns(databaseType); + metadataProvider.Setup(provider => provider.GetQueryBuilder()).Returns( + databaseType is DatabaseType.DWSQL ? new DwSqlQueryBuilder() : new MsSqlQueryBuilder()); string? exposedName; metadataProvider diff --git a/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs b/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs index 8d324dc347..448f8276a5 100644 --- a/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs +++ b/src/Service.Tests/UnitTests/ODataASTVisitorUnitTests.cs @@ -70,10 +70,10 @@ public static async Task SetupAsync(TestContext context) [DataRow("uuid_types eq 9A19103F-16F7-4668-BE54-9A1E7A4F7556", "([uuid_types] = @param1)", DisplayName = "Equate uuid(guid) types.")] [DataRow("time_types eq 10:23:54.9999999", "([time_types] = @param1)", DisplayName = "Equate time types.")] [DataRow("time_types eq null", "([time_types] IS NULL)", DisplayName = "Equate time types for null.")] - [DataRow("contains(string_types, 'he_l%o')", "([string_types] LIKE @param1)", DisplayName = "Contains string.")] - [DataRow("startswith(string_types, 'he')", "([string_types] LIKE @param1)", DisplayName = "Startswith string.")] - [DataRow("endswith(string_types, 'lo')", "([string_types] LIKE @param1)", DisplayName = "Endswith string.")] - [DataRow("contains(string_types, concat('|', concat('he', '|')))", "([string_types] LIKE @param1)", + [DataRow("contains(string_types, 'he_l%o')", "([string_types] LIKE @param1 ESCAPE '\\')", DisplayName = "Contains string.")] + [DataRow("startswith(string_types, 'he')", "([string_types] LIKE @param1 ESCAPE '\\')", DisplayName = "Startswith string.")] + [DataRow("endswith(string_types, 'lo')", "([string_types] LIKE @param1 ESCAPE '\\')", DisplayName = "Endswith string.")] + [DataRow("contains(string_types, concat('|', concat('he', '|')))", "([string_types] LIKE @param1 ESCAPE '\\')", DisplayName = "Contains concatenated string.")] [TestMethod] public void VisitorLeftFieldRightConstantFilterTest(string filterExp, string expectedPredicate) diff --git a/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs b/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs index 094bb91294..cc2000f6d7 100644 --- a/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs +++ b/src/Service.Tests/UnitTests/RelationalUpsertConcurrencyQueryBuilderTests.cs @@ -161,27 +161,6 @@ public void UpdateOnlyFallbackUpsertsDoNotAcquireInsertSerializationLocks() Assert.IsFalse(dwSqlQuery.Contains("TABLOCKX", StringComparison.Ordinal), $"Update-only Data Warehouse SQL query should not acquire an insert serialization lock. Query: {dwSqlQuery}"); } - /// - /// SQL Server needs an explicit ESCAPE clause for the backslash-escaped LIKE patterns that string - /// functions in database policies produce, for mutations as well as reads. - /// - [TestMethod] - public void MsSqlUpsertAddsEscapeClauseToLikeInDatabasePolicy() - { - SqlUpsertQueryStructure structure = CreateUpsertStructure(DatabaseType.MSSQL, useCompositePrimaryKey: false); - structure.DbPolicyPredicatesForOperations[EntityActionOperation.Update] = "([title] LIKE @param8)"; - structure.DbPolicyPredicatesForOperations[EntityActionOperation.Create] = "([title] LIKE @param9)"; - - string query = new MsSqlQueryBuilder().Build(structure); - - Assert.IsTrue( - query.Contains(@"([title] LIKE @param8 ESCAPE '\')", StringComparison.Ordinal), - $"Expected the update policy LIKE to carry an ESCAPE clause. Query: {query}"); - Assert.IsTrue( - query.Contains(@"([title] LIKE @param9 ESCAPE '\')", StringComparison.Ordinal), - $"Expected the create policy LIKE to carry an ESCAPE clause. Query: {query}"); - } - private static void AssertAppearsBefore(string query, string first, string second) { int firstIndex = query.IndexOf(first, StringComparison.Ordinal);