From f6d2c4a2c3943b48fee2135e02c6f56d16af6b57 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Wed, 5 Aug 2026 12:53:27 -0700 Subject: [PATCH 1/8] safely inject claims --- src/Auth/IAuthorizationResolver.cs | 7 +- src/Auth/ResolvedDatabasePolicy.cs | 23 ++++ .../Authorization/AuthorizationResolver.cs | 128 ++++++++++++------ src/Core/Parsers/ClaimsTypeDataUriResolver.cs | 23 ++++ src/Core/Parsers/FilterParser.cs | 15 +- src/Core/Parsers/ODataASTCosmosVisitor.cs | 7 +- .../Resolvers/AuthorizationPolicyHelpers.cs | 27 ++-- .../AuthorizationResolverUnitTests.cs | 116 ++++++++++------ .../REST/RestAuthorizationHandlerUnitTests.cs | 1 + .../DatabasePolicyClaimBindingUnitTests.cs | 85 ++++++++++++ .../UnitTests/DwSqlQueryBuilderUpsertTests.cs | 2 +- 11 files changed, 334 insertions(+), 100 deletions(-) create mode 100644 src/Auth/ResolvedDatabasePolicy.cs create mode 100644 src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs diff --git a/src/Auth/IAuthorizationResolver.cs b/src/Auth/IAuthorizationResolver.cs index 3a961ece4d..72ec1983fc 100644 --- a/src/Auth/IAuthorizationResolver.cs +++ b/src/Auth/IAuthorizationResolver.cs @@ -80,8 +80,11 @@ public interface IAuthorizationResolver /// Role defined in client role header. /// Operation type: Create, Read, Update, Delete. /// Contains token claims of the authenticated user used in policy evaluation. - /// Returns the parsed policy, if successfully processed, or an exception otherwise. - public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); + /// + /// Returns policy text containing parameter aliases together with the typed claim values + /// that must be bound to those aliases, or an exception when processing fails. + /// + public ResolvedDatabasePolicy ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); /// /// Get list of roles defined for entity within runtime configuration.. This is applicable for GraphQL when creating authorization diff --git a/src/Auth/ResolvedDatabasePolicy.cs b/src/Auth/ResolvedDatabasePolicy.cs new file mode 100644 index 0000000000..83ffefe104 --- /dev/null +++ b/src/Auth/ResolvedDatabasePolicy.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Azure.DataApiBuilder.Auth; + +/// +/// A database authorization policy whose claim references have been replaced by +/// OData parameter aliases. Claim values remain separate from the policy text so +/// they can be injected into the parsed OData AST as typed constants. +/// +/// Policy text containing OData parameter aliases. +/// Typed claim values keyed by their parameter alias. +public sealed record ResolvedDatabasePolicy( + string Policy, + IReadOnlyDictionary ClaimValues) +{ + /// + /// Represents an operation without a database authorization policy. + /// + public static ResolvedDatabasePolicy Empty { get; } = new( + string.Empty, + new Dictionary()); +} diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 205dc3d646..39a6671f42 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Globalization; using System.Net; using System.Security.Claims; using System.Text.Json; @@ -205,13 +206,13 @@ public bool AreColumnsAllowedForOperation(string entityName, string roleName, En } /// - public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) + public ResolvedDatabasePolicy ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) { string dBpolicyWithClaimTypes = GetDBPolicyForRequest(entityName, roleName, operation); if (string.IsNullOrWhiteSpace(dBpolicyWithClaimTypes)) { - return string.Empty; + return ResolvedDatabasePolicy.Empty; } return GetPolicyWithClaimValues(dBpolicyWithClaimTypes, GetAllAuthenticatedUserClaims(httpContext)); @@ -759,26 +760,37 @@ public static Dictionary> GetAllAuthenticatedUserClaims(Http } /// - /// Helper method to substitute all the claimTypes(denoted with @claims.claimType) in - /// the policy string with their corresponding claimValues. + /// Replaces all claim references (denoted with @claims.claimType) in the policy + /// with OData parameter aliases and returns their typed values separately. + /// Claim values must never be inserted into URI text because URI parsing can decode + /// percent-encoded syntax after string escaping has already occurred. /// /// The policy to be processed. /// Dictionary holding all the claims available in the request. - /// Processed policy with claim values substituted for claim types. + /// Policy text containing aliases and the typed values bound to those aliases. /// - private static string GetPolicyWithClaimValues(string policy, Dictionary> claimsInRequestContext) + private static ResolvedDatabasePolicy GetPolicyWithClaimValues(string policy, Dictionary> claimsInRequestContext) { // Regex used to extract all claimTypes in policy. It finds all the substrings which are // of the form @claims.*** where *** contains characters from a-zA-Z0-9._ . string claimCharsRgx = @"@claims\.[a-zA-Z0-9_\.]*"; - // Find all the claimTypes from the policy + Dictionary claimValues = new(); + int claimIndex = 0; + + // Replace claim references with inert OData aliases. The raw values remain out of + // the policy URI and are later injected directly into the parsed AST. string processedPolicy = Regex.Replace(policy, claimCharsRgx, - (claimTypeMatch) => GetClaimValueFromClaim(claimTypeMatch, claimsInRequestContext)); + (claimTypeMatch) => + { + string claimAlias = $"@dabClaim{claimIndex++}"; + claimValues.Add(claimAlias, GetClaimValueFromClaim(claimTypeMatch, claimsInRequestContext)); + return claimAlias; + }); // Remove occurrences of @item. directives processedPolicy = processedPolicy.Replace(FIELD_PREFIX, ""); - return processedPolicy; + return new ResolvedDatabasePolicy(processedPolicy, claimValues); } /// @@ -786,9 +798,9 @@ private static string GetPolicyWithClaimValues(string policy, Dictionary /// The claimType present in policy with a prefix of @claims.. /// Dictionary populated with all the user claims. - /// The claim value of the first claim whose claimType matches 'claimTypeMatch'. + /// The typed value of the first claim whose claimType matches 'claimTypeMatch'. /// Throws exception when the user does not possess the given claim. - private static string GetClaimValueFromClaim(Match claimTypeMatch, Dictionary> claimsInRequestContext) + private static object? GetClaimValueFromClaim(Match claimTypeMatch, Dictionary> claimsInRequestContext) { // Gets from @claims. string claimType = claimTypeMatch.Value.ToString().Substring(CLAIM_PREFIX.Length); @@ -815,13 +827,12 @@ private static string GetClaimValueFromClaim(Match claimTypeMatch, Dictionary - /// Using the input parameter claim, returns the primitive literal from claim.Value: - /// e.g. @claims.idp (string) resolves as 'azuread' + /// Using the input parameter claim, returns the typed primitive value from claim.Value: + /// e.g. @claims.idp (string) resolves as azuread /// e.g. @claims.iat (int) resolves as 1537231048 /// e.g. @claims.email_verified (boolean) resolves as true - /// To adhere with OData 4.01 ABNF construction rules (Section 7: Literal Data Values) - /// - Primitive string literals in URLS must be enclosed within single quotes. - /// - Other primitive types are represented as plain values and do not require single quotes. + /// Values are returned as CLR primitives so the policy parser can bind them as typed + /// OData AST constants without serializing them into URI text. /// Note: With many access token issuers, token claims are strings or string representations /// of other data types such as dates and GUIDs. /// Note: System.Security.Claim.ValueType defaults to ClaimValueTypes.String if the code calling @@ -834,7 +845,7 @@ private static string GetClaimValueFromClaim(Match claimTypeMatch, Dictionary /// /// - private static string GetClaimValue(Claim claim) + private static object? GetClaimValue(Claim claim) { /* An example Claim object: * claim.Type: "user_email" @@ -842,33 +853,64 @@ private static string GetClaimValue(Claim claim) * claim.ValueType: "http://www.w3.org/2001/XMLSchema#string" */ - switch (claim.ValueType) - { - case ClaimValueTypes.String: - // Escape embedded single quotes per OData 4.01 ABNF (Section 7: Literal Data Values) - // by doubling them. This prevents an attacker-influenced claim value from breaking - // out of the string literal and injecting additional OData predicates into the - // database authorization policy expression. - // See: http://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt - return $"'{claim.Value.Replace("'", "''")}'"; - case ClaimValueTypes.Boolean: - case ClaimValueTypes.Integer: - case ClaimValueTypes.Integer32: - case ClaimValueTypes.Integer64: - case ClaimValueTypes.UInteger32: - case ClaimValueTypes.UInteger64: - case ClaimValueTypes.Double: - return $"{claim.Value}"; - case JsonClaimValueTypes.JsonNull: - return $"null"; - default: - // One of the claims in the request had unsupported data type. - throw new DataApiBuilderException( - message: $"The claim value for claim: {claim.Type} belonging to the user has an unsupported data type.", - statusCode: HttpStatusCode.Forbidden, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType - ); + try + { + switch (claim.ValueType) + { + case ClaimValueTypes.String: + return claim.Value; + case ClaimValueTypes.Boolean: + return bool.Parse(claim.Value); + case ClaimValueTypes.Integer: + return ParseIntegerClaimValue(claim.Value); + case ClaimValueTypes.Integer32: + return int.Parse(claim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture); + case ClaimValueTypes.Integer64: + return long.Parse(claim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture); + case ClaimValueTypes.UInteger32: + return (long)uint.Parse(claim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture); + case ClaimValueTypes.UInteger64: + return (decimal)ulong.Parse(claim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture); + case ClaimValueTypes.Double: + return double.Parse(claim.Value, NumberStyles.Float, CultureInfo.InvariantCulture); + case JsonClaimValueTypes.JsonNull: + return null; + default: + // One of the claims in the request had unsupported data type. + throw CreateUnsupportedClaimValueException(claim); + } + } + catch (Exception ex) when (ex is FormatException || ex is OverflowException) + { + throw CreateUnsupportedClaimValueException(claim, ex); + } + } + + private static DataApiBuilderException CreateUnsupportedClaimValueException(Claim claim, Exception? innerException = null) + { + return new DataApiBuilderException( + message: $"The claim value for claim: {claim.Type} belonging to the user has an unsupported data type.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType, + innerException: innerException); + } + + /// + /// Parses an XML Schema integer claim into the narrowest OData-supported CLR integer type. + /// + private static object ParseIntegerClaimValue(string value) + { + if (int.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out int intValue)) + { + return intValue; + } + + if (long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out long longValue)) + { + return longValue; } + + return decimal.Parse(value, NumberStyles.Integer, CultureInfo.InvariantCulture); } /// diff --git a/src/Core/Parsers/ClaimsTypeDataUriResolver.cs b/src/Core/Parsers/ClaimsTypeDataUriResolver.cs index c8ef174ecd..d1fdf6dc40 100644 --- a/src/Core/Parsers/ClaimsTypeDataUriResolver.cs +++ b/src/Core/Parsers/ClaimsTypeDataUriResolver.cs @@ -15,6 +15,13 @@ namespace Azure.DataApiBuilder.Core.Parsers /// public class ClaimsTypeDataUriResolver : ODataUriResolver { + private readonly IReadOnlyDictionary _claimValueNodes; + + public ClaimsTypeDataUriResolver(IReadOnlyDictionary? claimValueNodes = null) + { + _claimValueNodes = claimValueNodes ?? new Dictionary(); + } + /// /// Between two nodes in the filter clause, determine the: /// - PrimaryOperand: Node representing an OData EDM model object and has Kind == QueryNodeKind.SingleValuePropertyAccess. @@ -27,6 +34,9 @@ public class ClaimsTypeDataUriResolver : ODataUriResolver /// type reference for the result BinaryOperatorNode. public override void PromoteBinaryOperandTypes(BinaryOperatorKind binaryOperatorKind, ref SingleValueNode leftNode, ref SingleValueNode rightNode, out IEdmTypeReference typeReference) { + ResolveClaimAlias(ref leftNode); + ResolveClaimAlias(ref rightNode); + if (leftNode.TypeReference.PrimitiveKind() != rightNode.TypeReference.PrimitiveKind()) { if ((leftNode.Kind == QueryNodeKind.SingleValuePropertyAccess) && (rightNode is ConstantNode)) @@ -48,6 +58,19 @@ public override void PromoteBinaryOperandTypes(BinaryOperatorKind binaryOperator base.PromoteBinaryOperandTypes(binaryOperatorKind, ref leftNode, ref rightNode, out typeReference); } + /// + /// Replaces a policy parameter alias with its typed claim constant before OData + /// performs type promotion. The claim value therefore never enters URI text. + /// + private void ResolveClaimAlias(ref SingleValueNode node) + { + if (node is ParameterAliasNode aliasNode && + _claimValueNodes.TryGetValue(aliasNode.Alias, out SingleValueNode? claimValueNode)) + { + node = claimValueNode; + } + } + /// /// Uses type specific parsers to attempt converting the supplied node to a new ConstantNode of type targetType /// when the supplied node's type differs from the target's type. diff --git a/src/Core/Parsers/FilterParser.cs b/src/Core/Parsers/FilterParser.cs index c9cfc1eb53..dbad26ba20 100644 --- a/src/Core/Parsers/FilterParser.cs +++ b/src/Core/Parsers/FilterParser.cs @@ -39,8 +39,13 @@ public void BuildModel(DocumentNode graphQLSchemaRoot) /// Represents the $filter part of the query string /// Represents the resource path, in our case the entity name. /// ODataUriResolver resolving different kinds of Uri parsing context. + /// Typed AST values for parameter aliases referenced by the filter. /// An AST FilterClause that represents the filter portion of the WHERE clause. - public FilterClause GetFilterClause(string filterQueryString, string resourcePath, ODataUriResolver? customResolver = null) + public FilterClause GetFilterClause( + string filterQueryString, + string resourcePath, + ODataUriResolver? customResolver = null, + IReadOnlyDictionary? parameterAliasNodes = null) { if (_model is null) { @@ -60,6 +65,14 @@ public FilterClause GetFilterClause(string filterQueryString, string resourcePat parser.Resolver = customResolver; } + if (parameterAliasNodes is not null) + { + foreach ((string alias, SingleValueNode valueNode) in parameterAliasNodes) + { + parser.ParameterAliasNodes.Add(alias, valueNode); + } + } + return parser.ParseFilter(); } catch (ODataException e) diff --git a/src/Core/Parsers/ODataASTCosmosVisitor.cs b/src/Core/Parsers/ODataASTCosmosVisitor.cs index 1fca34d624..f32a60c1a5 100644 --- a/src/Core/Parsers/ODataASTCosmosVisitor.cs +++ b/src/Core/Parsers/ODataASTCosmosVisitor.cs @@ -12,14 +12,17 @@ namespace Azure.DataApiBuilder.Core.Parsers internal class ODataASTCosmosVisitor : QueryNodeVisitor { private string _prefix; + private readonly Func _makeDbConnectionParam; /// /// Constructor for the visitor to append prefix to the column names which would be the path from container to the column /// /// - public ODataASTCosmosVisitor(string prefix) + /// Creates a bound Cosmos DB query parameter. + public ODataASTCosmosVisitor(string prefix, Func makeDbConnectionParam) { this._prefix = prefix; + _makeDbConnectionParam = makeDbConnectionParam; } /// @@ -123,7 +126,7 @@ public override string Visit(ConstantNode nodeIn) return "NULL"; } - return $"'{nodeIn.Value}'"; + return _makeDbConnectionParam(nodeIn.Value); } /// diff --git a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index 58f4d1461d..093e8eefeb 100644 --- a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs +++ b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs @@ -105,13 +105,17 @@ public static void ProcessAuthorizationPolicies( cosmosQueryStructure.TableCounter.Next(); fromClause = pathConfig.JoinStatement; - predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor(pathConfig.Alias)); + predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( + pathConfig.Alias, + value => cosmosQueryStructure.MakeDbConnectionParam(value))); existQuery = CosmosQueryBuilder.BuildExistsQueryForCosmos(fromClause, predicates); } else { - predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor($"{pathConfig.Path}.{pathConfig.ColumnName}")); + predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( + $"{pathConfig.Path}.{pathConfig.ColumnName}", + value => cosmosQueryStructure.MakeDbConnectionParam(value))); } if (pathConfig.EntityName == entity.Key) @@ -160,7 +164,7 @@ private static List ProcessFilter( List filterClauses = new(); foreach (EntityActionOperation elementalOperation in elementalOperations) { - string dbQueryPolicy = authorizationResolver.ProcessDBPolicy( + ResolvedDatabasePolicy dbQueryPolicy = authorizationResolver.ProcessDBPolicy( entityName, clientRoleHeader, elementalOperation, @@ -179,31 +183,36 @@ private static List ProcessFilter( } /// - /// Given a dbPolicyClause string, appends the string formatting needed to be processed by ODataParser + /// Appends the filter query formatting to a resolved database policy and parses it with ODataParser. /// - /// string representation of a processed database authorization policy. + /// Database authorization policy text and separately bound claim values. /// Name of the entity. /// Name of the schema. e.g. `dbo` for MsSql. /// Provides helper method to process ODataFilterClause. public static FilterClause? GetDBPolicyClauseForQueryStructure( - string dbPolicyClause, + ResolvedDatabasePolicy dbPolicy, string entityName, string resourcePath, ISqlMetadataProvider sqlMetadataProvider) { - if (!string.IsNullOrEmpty(dbPolicyClause)) + if (!string.IsNullOrEmpty(dbPolicy.Policy)) { + Dictionary claimValueNodes = dbPolicy.ClaimValues.ToDictionary( + claimValue => claimValue.Key, + claimValue => (SingleValueNode)new ConstantNode(claimValue.Value)); + // Since dbPolicy is nothing but filters to be added by virtue of database policy, we prefix it with // ?$filter= so that it conforms with the format followed by other filter predicates. // This enables the ODataVisitor helpers to parse the policy text properly. - dbPolicyClause = $"?{RequestParser.FILTER_URL}={dbPolicyClause}"; + string dbPolicyClause = $"?{RequestParser.FILTER_URL}={dbPolicy.Policy}"; // 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()); + customResolver: new ClaimsTypeDataUriResolver(claimValueNodes), + parameterAliasNodes: claimValueNodes); } return null; diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index a40839f8e8..c61031f722 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1286,14 +1286,14 @@ public void AreColumnsAllowedForOperationWithRoleWithDifferentCasing( /// The policy which is expected to be generated after parsing. [DataTestMethod] [DataRow("@claims.user_email ne @item.col1 and @claims.contact_no eq @item.col2 and not(@claims.name eq @item.col3)", - "'xyz@microsoft.com' ne col1 and 1234 eq col2 and not('Aaron' eq col3)", + "@dabClaim0 ne col1 and @dabClaim1 eq col2 and not(@dabClaim2 eq col3)", DisplayName = "Valid policy parsing test for string and int64 claimvaluetypes.")] [DataRow("(@claims.isemployee eq @item.col1 and @item.col2 ne @claims.user_email) or" + - "('David' ne @item.col3 and @claims.contact_no ne @item.col3)", "(true eq col1 and col2 ne 'xyz@microsoft.com') or" + - "('David' ne col3 and 1234 ne col3)", DisplayName = "Valid policy parsing test for constant string and int64 claimvaluetypes.")] + "('David' ne @item.col3 and @claims.contact_no ne @item.col3)", "(@dabClaim0 eq col1 and col2 ne @dabClaim1) or" + + "('David' ne col3 and @dabClaim2 ne col3)", DisplayName = "Valid policy parsing test for constant string and int64 claimvaluetypes.")] [DataRow("(@item.rating gt @claims.emprating) and (@claims.isemployee eq true)", - "(rating gt 4.2) and (true eq true)", DisplayName = "Valid policy parsing test for double and boolean claimvaluetypes.")] - [DataRow("@item.rating eq @claims.emprating)", "rating eq 4.2)", DisplayName = "Valid policy parsing test for double claimvaluetype.")] + "(rating gt @dabClaim0) and (@dabClaim1 eq true)", DisplayName = "Valid policy parsing test for double and boolean claimvaluetypes.")] + [DataRow("@item.rating eq @claims.emprating)", "rating eq @dabClaim0)", DisplayName = "Valid policy parsing test for double claimvaluetype.")] public void ParseValidDbPolicy(string policy, string expectedParsedPolicy) { RuntimeConfig runtimeConfig = InitRuntimeConfig( @@ -1317,40 +1317,33 @@ public void ParseValidDbPolicy(string policy, string expectedParsedPolicy) context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); - Assert.AreEqual(parsedPolicy, expectedParsedPolicy); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + Assert.AreEqual(parsedPolicy.Policy, expectedParsedPolicy); } /// - /// Validates that single quote characters embedded in a string-typed claim value are - /// escaped (doubled) per OData 4.01 ABNF when substituted into a database authorization - /// policy. Without escaping, an attacker who can influence a referenced JWT claim could - /// break out of the string literal and inject additional OData predicates - bypassing - /// row-level authorization. The substituted claim must remain enclosed in a single - /// string literal regardless of its contents. + /// Validates that a string claim is kept out of database authorization policy text and + /// associated with an inert OData parameter alias. This prevents claim contents from + /// being reinterpreted as policy syntax during URI parsing. /// /// The raw claim value (as it appears in the JWT) to substitute. - /// The parsed policy after safe substitution. [DataTestMethod] [DataRow( "alice' or 1 eq 1 or '", - "col1 eq 'alice'' or 1 eq 1 or '''", - DisplayName = "Injection attempt with OR predicate is neutralized by escaping single quotes")] + DisplayName = "Literal quote injection remains outside policy text")] [DataRow( "O'Brien", - "col1 eq 'O''Brien'", - DisplayName = "Legitimate single-quote-bearing value (e.g. surname) is safely escaped")] + DisplayName = "Legitimate single-quote-bearing value remains unchanged")] [DataRow( - "''", - "col1 eq ''''''", - DisplayName = "Value composed solely of single quotes is fully escaped")] + "alice%27 or 1 eq 1 or %27", + DisplayName = "Encoded quote injection remains outside policy text")] [DataRow( - "no quotes here", - "col1 eq 'no quotes here'", - DisplayName = "Value without single quotes is unchanged aside from enclosing quotes")] - public void DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection( - string claimValue, - string expectedParsedPolicy) + "alice%2527 or 1 eq 1 or %2527", + DisplayName = "Double-encoded quote injection remains outside policy text")] + [DataRow( + "50% complete", + DisplayName = "Legitimate percent characters remain unchanged")] + public void DbPolicy_StringClaim_UsesTypedParameterAlias(string claimValue) { const string policyDefinition = "@item.col1 eq @claims.userId"; @@ -1370,9 +1363,10 @@ public void DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection( context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); - Assert.AreEqual(expectedParsedPolicy, parsedPolicy); + Assert.AreEqual("col1 eq @dabClaim0", parsedPolicy.Policy); + Assert.AreEqual(claimValue, parsedPolicy.ClaimValues["@dabClaim0"]); } /// @@ -1403,11 +1397,7 @@ public void DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection( #pragma warning restore format public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimValue, bool supportedValueType) { - // To adhere with OData 4 ABNF construction rules (Section 7: Literal Data Values) - // - Primitive string literals in URLS must be enclosed within single quotes. - // - http://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt - string odataClaimValue = (claimValueType == ClaimValueTypes.String) ? "'" + claimValue + "'" : claimValue; - string expectedPolicy = odataClaimValue + " eq col1"; + string expectedPolicy = "@dabClaim0 eq col1"; string policyDefinition = "@claims.testClaim eq @item.col1"; RuntimeConfig runtimeConfig = InitRuntimeConfig( @@ -1431,9 +1421,21 @@ public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimVa try { - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); Assert.IsTrue(supportedValueType); - Assert.AreEqual(expectedPolicy, parsedPolicy); + Assert.AreEqual(expectedPolicy, parsedPolicy.Policy); + + object? typedClaimValue = parsedPolicy.ClaimValues["@dabClaim0"]; + if (claimValueType == JsonClaimValueTypes.JsonNull) + { + Assert.IsNull(typedClaimValue); + } + else + { + Assert.AreEqual( + claimValue.ToLowerInvariant(), + Convert.ToString(typedClaimValue, System.Globalization.CultureInfo.InvariantCulture)?.ToLowerInvariant()); + } } catch (DataApiBuilderException ex) { @@ -1446,6 +1448,34 @@ public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimVa } } + /// + /// A claim whose value does not match its declared primitive type must fail before + /// policy parsing and must never be interpreted as OData syntax. + /// + [TestMethod] + public void DbPolicy_MalformedPrimitiveClaim_FailsClosed() + { + RuntimeConfig runtimeConfig = InitRuntimeConfig( + entityName: TEST_ENTITY, + roleName: TEST_ROLE, + operation: TEST_OPERATION, + includedCols: new HashSet { "col1" }, + databasePolicy: "@claims.testClaim eq @item.col1"); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + Mock context = new(); + ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE); + identity.AddClaim(new Claim("testClaim", "1 or 1 eq 1", ClaimValueTypes.Integer)); + context.Setup(x => x.User).Returns(new ClaimsPrincipal(identity)); + context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); + + DataApiBuilderException exception = Assert.ThrowsException(() => + authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); + + Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType, exception.SubStatusCode); + } + /// /// Test to validate that we are correctly throwing an appropriate exception when the user request /// lacks a claim required by the policy. @@ -1531,11 +1561,13 @@ public void ParsePolicyWithDuplicateUserClaims() context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); // Act - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); // Assert - string expectedPolicy = $"'profile' eq col2 and '1111' eq col3"; - Assert.AreEqual(expected: expectedPolicy, actual: parsedPolicy); + string expectedPolicy = "@dabClaim0 eq col2 and @dabClaim1 eq col3"; + Assert.AreEqual(expected: expectedPolicy, actual: parsedPolicy.Policy); + Assert.AreEqual("profile", parsedPolicy.ClaimValues["@dabClaim0"]); + Assert.AreEqual("1111", parsedPolicy.ClaimValues["@dabClaim1"]); } // Indirectly tests the AuthorizationResolver private method: @@ -1583,15 +1615,15 @@ public void GetDBPolicyTest( context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(clientRole); - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); string errorMessage = "TryProcessDBPolicy returned unexpected value."; if (expectPolicy) { - Assert.AreEqual(actual: parsedPolicy, expected: policy, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy.Policy, expected: policy, message: errorMessage); } else { - Assert.AreEqual(actual: parsedPolicy, expected: string.Empty, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy.Policy, expected: string.Empty, message: errorMessage); } } diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index d6e7d55bfe..69f71368dd 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -104,6 +104,7 @@ public void TestWildcardPolicyResolvesToEmpty(string httpMethod) roleName: "admin", operation: RestService.HttpVerbToOperations(httpVerbName: httpMethod), httpContext: httpContext) + .Policy ); } diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs new file mode 100644 index 0000000000..417aa87e64 --- /dev/null +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using Azure.DataApiBuilder.Core.Parsers; +using Microsoft.OData.Edm; +using Microsoft.OData.UriParser; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Azure.DataApiBuilder.Service.Tests.UnitTests +{ + /// + /// Tests that database policy claim values are bound as typed AST constants and + /// never interpreted as URI/OData syntax. + /// + [TestClass] + public class DatabasePolicyClaimBindingUnitTests + { + /// + /// Verifies that attacker-influenced claim values never enter the policy URI. The OData + /// parser must see one equality expression whose right operand is the original claim value, + /// regardless of percent encoding, and Cosmos DB must receive that value as a parameter. + /// + [DataTestMethod] + [DataRow("alice%27 or 1 eq 1 or %27", DisplayName = "Percent-encoded quote")] + [DataRow("alice%2527 or 1 eq 1 or %2527", DisplayName = "Double-encoded quote")] + [DataRow("alice%252527 or 1 eq 1 or %27", DisplayName = "Mixed nested encodings")] + [DataRow("alice' or 1 eq 1 or '", DisplayName = "Literal quote")] + [DataRow("50% complete", DisplayName = "Legitimate percent character")] + public void DatabasePolicyClaimValue_IsTypedAstConstantAndCosmosParameter(string claimValue) + { + const string claimAlias = "@dabClaim0"; + Dictionary claimValueNodes = new() + { + [claimAlias] = new ConstantNode(claimValue) + }; + + ODataUriParser parser = new( + BuildModel(), + new Uri($"Entities/?$filter=name eq {claimAlias}", UriKind.Relative)) + { + Resolver = new ClaimsTypeDataUriResolver(claimValueNodes) + }; + + foreach ((string alias, SingleValueNode valueNode) in claimValueNodes) + { + parser.ParameterAliasNodes.Add(alias, valueNode); + } + + FilterClause clause = parser.ParseFilter(); + BinaryOperatorNode comparison = (BinaryOperatorNode)clause.Expression; + Assert.AreEqual(BinaryOperatorKind.Equal, comparison.OperatorKind); + Assert.IsInstanceOfType(comparison.Right); + Assert.AreEqual(claimValue, ((ConstantNode)comparison.Right).Value); + + List parameterValues = new(); + string predicate = clause.Expression.Accept(new ODataASTCosmosVisitor( + "c", + value => + { + parameterValues.Add(value); + return "@param1"; + })); + + Assert.AreEqual("(c.name = @param1)", predicate); + CollectionAssert.AreEqual(new object?[] { claimValue }, parameterValues); + } + + private static IEdmModel BuildModel() + { + EdmModel model = new(); + EdmEntityType entityType = new("Dab", "Entity"); + EdmStructuralProperty id = entityType.AddStructuralProperty("id", EdmPrimitiveTypeKind.Int32, false); + entityType.AddKeys(id); + entityType.AddStructuralProperty("name", EdmPrimitiveTypeKind.String, true); + model.AddElement(entityType); + + EdmEntityContainer container = new("Dab", "Container"); + container.AddEntitySet("Entities", entityType); + model.AddElement(container); + return model; + } + } +} diff --git a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs index 3d70162832..8a8a5f2341 100644 --- a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs +++ b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs @@ -144,7 +144,7 @@ private static SqlUpsertQueryStructure CreateUpsertStructure() Mock authorizationResolver = new(); authorizationResolver .Setup(x => x.ProcessDBPolicy(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(string.Empty); + .Returns(ResolvedDatabasePolicy.Empty); RuntimeConfigProvider runtimeConfigProvider = TestHelper.GetRuntimeConfigProvider(TestHelper.GetRuntimeConfigLoader()); Mock metadataProviderFactory = new(); From 170c2825df7823330f99f39c48d88c092d2ccc90 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Wed, 5 Aug 2026 14:17:06 -0700 Subject: [PATCH 2/8] fix some missing operator paths --- src/Auth/IAuthorizationResolver.cs | 12 +- src/Auth/IResolvedDatabasePolicyProvider.cs | 28 ++ src/Auth/ResolvedDatabasePolicy.cs | 35 +- .../Authorization/AuthorizationResolver.cs | 43 +- src/Core/Parsers/FilterParser.cs | 5 +- src/Core/Parsers/ODataASTCosmosVisitor.cs | 11 +- src/Core/Parsers/ParameterAliasRewriter.cs | 86 ++++ .../Resolvers/AuthorizationPolicyHelpers.cs | 51 +- src/Core/Resolvers/CosmosQueryStructure.cs | 8 - .../AuthorizationResolverUnitTests.cs | 65 ++- .../REST/RestAuthorizationHandlerUnitTests.cs | 3 +- .../DatabasePolicyClaimBindingUnitTests.cs | 445 ++++++++++++++++-- .../UnitTests/DwSqlQueryBuilderUpsertTests.cs | 7 +- 13 files changed, 700 insertions(+), 99 deletions(-) create mode 100644 src/Auth/IResolvedDatabasePolicyProvider.cs create mode 100644 src/Core/Parsers/ParameterAliasRewriter.cs diff --git a/src/Auth/IAuthorizationResolver.cs b/src/Auth/IAuthorizationResolver.cs index 72ec1983fc..27e3b330eb 100644 --- a/src/Auth/IAuthorizationResolver.cs +++ b/src/Auth/IAuthorizationResolver.cs @@ -80,11 +80,13 @@ public interface IAuthorizationResolver /// Role defined in client role header. /// Operation type: Create, Read, Update, Delete. /// Contains token claims of the authenticated user used in policy evaluation. - /// - /// Returns policy text containing parameter aliases together with the typed claim values - /// that must be bound to those aliases, or an exception when processing fails. - /// - public ResolvedDatabasePolicy ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); + /// Returns the parsed policy, if safely representable as text, or an exception otherwise. + /// + /// Claim-bearing policies cannot be represented safely by this string-only contract. + /// Use instead. + /// + [Obsolete("Use IResolvedDatabasePolicyProvider.ResolveDBPolicy to keep claim values separate from policy text.")] + public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); /// /// Get list of roles defined for entity within runtime configuration.. This is applicable for GraphQL when creating authorization diff --git a/src/Auth/IResolvedDatabasePolicyProvider.cs b/src/Auth/IResolvedDatabasePolicyProvider.cs new file mode 100644 index 0000000000..f816b15d99 --- /dev/null +++ b/src/Auth/IResolvedDatabasePolicyProvider.cs @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.DataApiBuilder.Config.ObjectModel; +using Microsoft.AspNetCore.Http; + +namespace Azure.DataApiBuilder.Auth; + +/// +/// Resolves database authorization policies while keeping claim values separate +/// from policy text. +/// +public interface IResolvedDatabasePolicyProvider +{ + /// + /// Resolves claim references to parameter aliases and returns their typed values separately. + /// + /// Entity from the request. + /// Role defined in the client role header. + /// Operation type: Create, Read, Update, Delete. + /// Contains the authenticated user's token claims. + /// The policy text and typed claim values to bind to it. + public ResolvedDatabasePolicy ResolveDBPolicy( + string entityName, + string roleName, + EntityActionOperation operation, + HttpContext httpContext); +} diff --git a/src/Auth/ResolvedDatabasePolicy.cs b/src/Auth/ResolvedDatabasePolicy.cs index 83ffefe104..1e4e8a2676 100644 --- a/src/Auth/ResolvedDatabasePolicy.cs +++ b/src/Auth/ResolvedDatabasePolicy.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.ObjectModel; + namespace Azure.DataApiBuilder.Auth; /// @@ -8,16 +10,37 @@ namespace Azure.DataApiBuilder.Auth; /// OData parameter aliases. Claim values remain separate from the policy text so /// they can be injected into the parsed OData AST as typed constants. /// -/// Policy text containing OData parameter aliases. -/// Typed claim values keyed by their parameter alias. -public sealed record ResolvedDatabasePolicy( - string Policy, - IReadOnlyDictionary ClaimValues) +public sealed record ResolvedDatabasePolicy { + /// + /// Initializes a resolved database policy and takes an immutable snapshot of its claim values. + /// + /// Policy text containing OData parameter aliases. + /// Typed claim values keyed by their parameter alias. + public ResolvedDatabasePolicy(string policy, IReadOnlyDictionary claimValues) + { + ArgumentNullException.ThrowIfNull(policy); + ArgumentNullException.ThrowIfNull(claimValues); + + Policy = policy; + ClaimValues = new ReadOnlyDictionary( + new Dictionary(claimValues, StringComparer.Ordinal)); + } + + /// + /// Policy text containing OData parameter aliases. + /// + public string Policy { get; } + + /// + /// Immutable snapshot of typed claim values keyed by parameter alias. + /// + public IReadOnlyDictionary ClaimValues { get; } + /// /// Represents an operation without a database authorization policy. /// public static ResolvedDatabasePolicy Empty { get; } = new( string.Empty, - new Dictionary()); + new ReadOnlyDictionary(new Dictionary())); } diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 39a6671f42..4b896354f0 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -25,7 +25,7 @@ namespace Azure.DataApiBuilder.Core.Authorization; /// Authorization stages that require passing before a request is executed /// against a database. /// -public class AuthorizationResolver : IAuthorizationResolver +public class AuthorizationResolver : IAuthorizationResolver, IResolvedDatabasePolicyProvider { private readonly RuntimeConfigProvider _runtimeConfigProvider; private readonly IMetadataProviderFactory _metadataProviderFactory; @@ -206,7 +206,29 @@ public bool AreColumnsAllowedForOperation(string entityName, string roleName, En } /// - public ResolvedDatabasePolicy ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) + [Obsolete("Use IResolvedDatabasePolicyProvider.ResolveDBPolicy to keep claim values separate from policy text.")] + public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) + { + string dbPolicy = GetDBPolicyForRequest(entityName, roleName, operation); + + if (string.IsNullOrWhiteSpace(dbPolicy)) + { + return string.Empty; + } + + if (dbPolicy.Contains(CLAIM_PREFIX, StringComparison.Ordinal)) + { + throw new DataApiBuilderException( + message: "Claim-bearing database policies require typed claim binding.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed); + } + + return dbPolicy.Replace(FIELD_PREFIX, string.Empty); + } + + /// + public ResolvedDatabasePolicy ResolveDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) { string dBpolicyWithClaimTypes = GetDBPolicyForRequest(entityName, roleName, operation); @@ -872,7 +894,7 @@ private static ResolvedDatabasePolicy GetPolicyWithClaimValues(string policy, Di case ClaimValueTypes.UInteger64: return (decimal)ulong.Parse(claim.Value, NumberStyles.Integer, CultureInfo.InvariantCulture); case ClaimValueTypes.Double: - return double.Parse(claim.Value, NumberStyles.Float, CultureInfo.InvariantCulture); + return ParseFiniteDoubleClaimValue(claim.Value); case JsonClaimValueTypes.JsonNull: return null; default: @@ -886,6 +908,21 @@ private static ResolvedDatabasePolicy GetPolicyWithClaimValues(string policy, Di } } + /// + /// Parses a floating-point claim and rejects values that database providers cannot + /// represent consistently, including NaN and positive or negative infinity. + /// + private static double ParseFiniteDoubleClaimValue(string value) + { + double parsedValue = double.Parse(value, NumberStyles.Float, CultureInfo.InvariantCulture); + if (!double.IsFinite(parsedValue)) + { + throw new FormatException("The floating-point claim value must be finite."); + } + + return parsedValue; + } + private static DataApiBuilderException CreateUnsupportedClaimValueException(Claim claim, Exception? innerException = null) { return new DataApiBuilderException( diff --git a/src/Core/Parsers/FilterParser.cs b/src/Core/Parsers/FilterParser.cs index dbad26ba20..f261a6e197 100644 --- a/src/Core/Parsers/FilterParser.cs +++ b/src/Core/Parsers/FilterParser.cs @@ -73,7 +73,10 @@ public FilterClause GetFilterClause( } } - return parser.ParseFilter(); + FilterClause filterClause = parser.ParseFilter(); + return parameterAliasNodes is null + ? filterClause + : new ParameterAliasRewriter(parameterAliasNodes).Rewrite(filterClause); } catch (ODataException e) { diff --git a/src/Core/Parsers/ODataASTCosmosVisitor.cs b/src/Core/Parsers/ODataASTCosmosVisitor.cs index f32a60c1a5..1912972766 100644 --- a/src/Core/Parsers/ODataASTCosmosVisitor.cs +++ b/src/Core/Parsers/ODataASTCosmosVisitor.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Azure.DataApiBuilder.Core.Resolvers; using Microsoft.OData.UriParser; /// @@ -12,17 +13,17 @@ namespace Azure.DataApiBuilder.Core.Parsers internal class ODataASTCosmosVisitor : QueryNodeVisitor { private string _prefix; - private readonly Func _makeDbConnectionParam; + private readonly BaseQueryStructure _queryStructure; /// /// Constructor for the visitor to append prefix to the column names which would be the path from container to the column /// /// - /// Creates a bound Cosmos DB query parameter. - public ODataASTCosmosVisitor(string prefix, Func makeDbConnectionParam) + /// Stores bound Cosmos DB query parameters. + public ODataASTCosmosVisitor(string prefix, BaseQueryStructure queryStructure) { this._prefix = prefix; - _makeDbConnectionParam = makeDbConnectionParam; + _queryStructure = queryStructure; } /// @@ -126,7 +127,7 @@ public override string Visit(ConstantNode nodeIn) return "NULL"; } - return _makeDbConnectionParam(nodeIn.Value); + return _queryStructure.MakeDbConnectionParam(nodeIn.Value); } /// diff --git a/src/Core/Parsers/ParameterAliasRewriter.cs b/src/Core/Parsers/ParameterAliasRewriter.cs new file mode 100644 index 0000000000..cfcef05dba --- /dev/null +++ b/src/Core/Parsers/ParameterAliasRewriter.cs @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.OData; +using Microsoft.OData.Edm; +using Microsoft.OData.UriParser; + +namespace Azure.DataApiBuilder.Core.Parsers; + +/// +/// Replaces OData parameter aliases with their typed AST values after parsing. +/// The resolver still supplies aliases during binary type promotion; this pass +/// also covers aliases in unary and root Boolean expressions. +/// +internal sealed class ParameterAliasRewriter +{ + private readonly IReadOnlyDictionary _parameterAliasNodes; + + public ParameterAliasRewriter(IReadOnlyDictionary parameterAliasNodes) + { + _parameterAliasNodes = parameterAliasNodes; + } + + /// + /// Rewrites all supported nodes in a filter clause and normalizes bare Boolean + /// values into comparisons that are valid SQL predicates across providers. + /// + public FilterClause Rewrite(FilterClause filterClause) + { + SingleValueNode expression = RewriteNode(filterClause.Expression, isPredicate: true); + return new FilterClause(expression, filterClause.RangeVariable); + } + + private SingleValueNode RewriteNode(SingleValueNode node, bool isPredicate) + { + return node switch + { + BinaryOperatorNode binaryNode => RewriteBinaryOperator(binaryNode), + UnaryOperatorNode unaryNode => new UnaryOperatorNode( + unaryNode.OperatorKind, + RewriteNode(unaryNode.Operand, isPredicate: true)), + ConvertNode convertNode => NormalizeBooleanPredicate( + new ConvertNode( + RewriteNode(convertNode.Source, isPredicate: false), + convertNode.TypeReference), + isPredicate), + ParameterAliasNode aliasNode => RewriteAlias(aliasNode, isPredicate), + ConstantNode constantNode => NormalizeBooleanPredicate(constantNode, isPredicate), + SingleValuePropertyAccessNode propertyNode => NormalizeBooleanPredicate(propertyNode, isPredicate), + _ => throw new ODataException( + $"Database policy expression node '{node.Kind}' is not supported for typed claim binding.") + }; + } + + private SingleValueNode RewriteBinaryOperator(BinaryOperatorNode node) + { + bool operandsArePredicates = node.OperatorKind is BinaryOperatorKind.And or BinaryOperatorKind.Or; + return new BinaryOperatorNode( + node.OperatorKind, + RewriteNode(node.Left, operandsArePredicates), + RewriteNode(node.Right, operandsArePredicates)); + } + + private SingleValueNode RewriteAlias(ParameterAliasNode aliasNode, bool isPredicate) + { + if (!_parameterAliasNodes.TryGetValue(aliasNode.Alias, out SingleValueNode? valueNode)) + { + throw new ODataException($"No value was supplied for database policy parameter alias '{aliasNode.Alias}'."); + } + + return RewriteNode(valueNode, isPredicate); + } + + private static SingleValueNode NormalizeBooleanPredicate(SingleValueNode node, bool isPredicate) + { + if (!isPredicate || node.TypeReference?.PrimitiveKind() is not EdmPrimitiveTypeKind.Boolean) + { + return node; + } + + return new BinaryOperatorNode( + BinaryOperatorKind.Equal, + node, + new ConstantNode(true)); + } +} diff --git a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index 093e8eefeb..7bfa9c8b5e 100644 --- a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs +++ b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs @@ -107,7 +107,7 @@ public static void ProcessAuthorizationPolicies( fromClause = pathConfig.JoinStatement; predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( pathConfig.Alias, - value => cosmosQueryStructure.MakeDbConnectionParam(value))); + cosmosQueryStructure)); existQuery = CosmosQueryBuilder.BuildExistsQueryForCosmos(fromClause, predicates); } @@ -115,7 +115,7 @@ public static void ProcessAuthorizationPolicies( { predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( $"{pathConfig.Path}.{pathConfig.ColumnName}", - value => cosmosQueryStructure.MakeDbConnectionParam(value))); + cosmosQueryStructure)); } if (pathConfig.EntityName == entity.Key) @@ -164,11 +164,12 @@ private static List ProcessFilter( List filterClauses = new(); foreach (EntityActionOperation elementalOperation in elementalOperations) { - ResolvedDatabasePolicy dbQueryPolicy = authorizationResolver.ProcessDBPolicy( - entityName, - clientRoleHeader, - elementalOperation, - context); + ResolvedDatabasePolicy dbQueryPolicy = ResolveDatabasePolicy( + authorizationResolver, + entityName, + clientRoleHeader, + elementalOperation, + context); FilterClause? filterClause = GetDBPolicyClauseForQueryStructure( dbQueryPolicy, @@ -182,6 +183,42 @@ private static List ProcessFilter( return filterClauses; } + /// + /// Uses typed claim binding when supported. Legacy authorization resolver implementations + /// remain compatible for static policies, but claim-bearing policies fail closed because + /// the legacy string contract cannot keep claim data separate from policy syntax. + /// + private static ResolvedDatabasePolicy ResolveDatabasePolicy( + IAuthorizationResolver authorizationResolver, + string entityName, + string roleName, + EntityActionOperation operation, + HttpContext context) + { + if (authorizationResolver is IResolvedDatabasePolicyProvider resolvedPolicyProvider) + { + return resolvedPolicyProvider.ResolveDBPolicy(entityName, roleName, operation, context); + } + + string policy = authorizationResolver.GetDBPolicyForRequest(entityName, roleName, operation); + if (string.IsNullOrWhiteSpace(policy)) + { + return ResolvedDatabasePolicy.Empty; + } + + if (policy.Contains(AuthorizationResolver.CLAIM_PREFIX, StringComparison.Ordinal)) + { + throw new DataApiBuilderException( + message: "The authorization resolver does not support typed database policy claims.", + statusCode: System.Net.HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed); + } + + return new ResolvedDatabasePolicy( + policy.Replace(AuthorizationResolver.FIELD_PREFIX, string.Empty), + new Dictionary()); + } + /// /// Appends the filter query formatting to a resolved database policy and parses it with ODataParser. /// diff --git a/src/Core/Resolvers/CosmosQueryStructure.cs b/src/Core/Resolvers/CosmosQueryStructure.cs index 29c435d955..897345260a 100644 --- a/src/Core/Resolvers/CosmosQueryStructure.cs +++ b/src/Core/Resolvers/CosmosQueryStructure.cs @@ -67,14 +67,6 @@ public CosmosQueryStructure( Init(parameters); } - /// - public override string MakeDbConnectionParam(object? value, string? columnName = null, bool lengthOverride = false) - { - string encodedParamName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(encodedParamName, new(value)); - return encodedParamName; - } - private static IEnumerable GenerateQueryColumns(SelectionSetNode selectionSet, DocumentNode document, string tableName) { foreach (ISelectionNode selectionNode in selectionSet.Selections) diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index c61031f722..93ef5a68a6 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1317,7 +1317,7 @@ public void ParseValidDbPolicy(string policy, string expectedParsedPolicy) context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); - ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); Assert.AreEqual(parsedPolicy.Policy, expectedParsedPolicy); } @@ -1363,12 +1363,46 @@ public void DbPolicy_StringClaim_UsesTypedParameterAlias(string claimValue) context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); - ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); Assert.AreEqual("col1 eq @dabClaim0", parsedPolicy.Policy); Assert.AreEqual(claimValue, parsedPolicy.ClaimValues["@dabClaim0"]); } + /// + /// The legacy public API keeps its string return type for source and binary compatibility, + /// but fails closed because claim values cannot be represented safely in policy text. + /// + [TestMethod] + public void DbPolicy_LegacyStringContractWithClaims_FailsClosed() + { + RuntimeConfig runtimeConfig = InitRuntimeConfig( + entityName: TEST_ENTITY, + roleName: TEST_ROLE, + operation: TEST_OPERATION, + includedCols: new HashSet { "col1" }, + databasePolicy: "@item.col1 eq @claims.userId"); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + Mock context = new(); + ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE); + identity.AddClaim(new Claim("userId", "alice", ClaimValueTypes.String)); + context.Setup(x => x.User).Returns(new ClaimsPrincipal(identity)); + context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); + + Assert.AreEqual( + typeof(string), + typeof(IAuthorizationResolver).GetMethod(nameof(IAuthorizationResolver.ProcessDBPolicy))!.ReturnType); + + #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. + DataApiBuilderException exception = Assert.ThrowsException(() => + authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); + #pragma warning restore CS0618 + + Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); + } + /// /// Tests authorization policy processing mechanism by validating value type compatibility /// of claims present in HttpContext.User.Claims. @@ -1421,7 +1455,7 @@ public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimVa try { - ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); Assert.IsTrue(supportedValueType); Assert.AreEqual(expectedPolicy, parsedPolicy.Policy); @@ -1452,8 +1486,13 @@ public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimVa /// A claim whose value does not match its declared primitive type must fail before /// policy parsing and must never be interpreted as OData syntax. /// - [TestMethod] - public void DbPolicy_MalformedPrimitiveClaim_FailsClosed() + [DataTestMethod] + [DataRow(ClaimValueTypes.Integer, "1 or 1 eq 1", DisplayName = "Malformed integer")] + [DataRow(ClaimValueTypes.Double, "NaN", DisplayName = "NaN")] + [DataRow(ClaimValueTypes.Double, "Infinity", DisplayName = "Positive infinity")] + [DataRow(ClaimValueTypes.Double, "-Infinity", DisplayName = "Negative infinity")] + [DataRow(ClaimValueTypes.Double, "1e9999", DisplayName = "Exponent overflow")] + public void DbPolicy_InvalidPrimitiveClaim_FailsClosed(string claimValueType, string claimValue) { RuntimeConfig runtimeConfig = InitRuntimeConfig( entityName: TEST_ENTITY, @@ -1465,12 +1504,12 @@ public void DbPolicy_MalformedPrimitiveClaim_FailsClosed() Mock context = new(); ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE); - identity.AddClaim(new Claim("testClaim", "1 or 1 eq 1", ClaimValueTypes.Integer)); + identity.AddClaim(new Claim("testClaim", claimValue, claimValueType)); context.Setup(x => x.User).Returns(new ClaimsPrincipal(identity)); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); DataApiBuilderException exception = Assert.ThrowsException(() => - authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); + authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); Assert.AreEqual(DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType, exception.SubStatusCode); @@ -1509,7 +1548,7 @@ public void ParseInvalidDbPolicyWithUserNotPossessingAllClaims(string policy) try { - authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); } catch (DataApiBuilderException ex) { @@ -1561,7 +1600,7 @@ public void ParsePolicyWithDuplicateUserClaims() context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); // Act - ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); + ResolvedDatabasePolicy parsedPolicy = authZResolver.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object); // Assert string expectedPolicy = "@dabClaim0 eq col2 and @dabClaim1 eq col3"; @@ -1615,15 +1654,17 @@ public void GetDBPolicyTest( context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(clientRole); - ResolvedDatabasePolicy parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); + #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. + string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); + #pragma warning restore CS0618 string errorMessage = "TryProcessDBPolicy returned unexpected value."; if (expectPolicy) { - Assert.AreEqual(actual: parsedPolicy.Policy, expected: policy, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy, expected: policy, message: errorMessage); } else { - Assert.AreEqual(actual: parsedPolicy.Policy, expected: string.Empty, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy, expected: string.Empty, message: errorMessage); } } diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index 69f71368dd..26264741e0 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -99,13 +99,14 @@ public void TestWildcardPolicyResolvesToEmpty(string httpMethod) AuthorizationResolver authorizationResolver = SetupAuthResolverWithWildcardOperation(); HttpContext httpContext = CreateHttpContext(httpMethod: httpMethod, clientRole: "admin"); + #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. Assert.AreEqual(expected: string.Empty, actual: authorizationResolver.ProcessDBPolicy( entityName: AuthorizationHelpers.TEST_ENTITY, roleName: "admin", operation: RestService.HttpVerbToOperations(httpVerbName: httpMethod), httpContext: httpContext) - .Policy ); + #pragma warning restore CS0618 } /// diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index 417aa87e64..6c7856de40 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -3,24 +3,41 @@ using System; using System.Collections.Generic; +using System.Data; +using System.Linq; +using System.Net; +using System.Security.Claims; +using Azure.DataApiBuilder.Auth; +using Azure.DataApiBuilder.Config.DatabasePrimitives; +using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.Authorization; using Azure.DataApiBuilder.Core.Parsers; -using Microsoft.OData.Edm; +using Azure.DataApiBuilder.Core.Resolvers; +using Azure.DataApiBuilder.Core.Services; +using Azure.DataApiBuilder.Service.Exceptions; +using Azure.DataApiBuilder.Service.Tests.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.IdentityModel.JsonWebTokens; using Microsoft.OData.UriParser; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; namespace Azure.DataApiBuilder.Service.Tests.UnitTests { /// - /// Tests that database policy claim values are bound as typed AST constants and - /// never interpreted as URI/OData syntax. + /// Tests the complete database-policy claim binding path from an authenticated claim + /// through OData AST creation and SQL/Cosmos query parameter collection. /// [TestClass] public class DatabasePolicyClaimBindingUnitTests { + private const string ENTITY_NAME = AuthorizationHelpers.TEST_ENTITY; + private const string ROLE_NAME = AuthorizationHelpers.TEST_ROLE; + private const EntityActionOperation OPERATION = EntityActionOperation.Read; + /// - /// Verifies that attacker-influenced claim values never enter the policy URI. The OData - /// parser must see one equality expression whose right operand is the original claim value, - /// regardless of percent encoding, and Cosmos DB must receive that value as a parameter. + /// Verifies that encoded and literal syntax remains claim data throughout the complete + /// SQL and Cosmos policy pipelines. /// [DataTestMethod] [DataRow("alice%27 or 1 eq 1 or %27", DisplayName = "Percent-encoded quote")] @@ -28,58 +45,394 @@ public class DatabasePolicyClaimBindingUnitTests [DataRow("alice%252527 or 1 eq 1 or %27", DisplayName = "Mixed nested encodings")] [DataRow("alice' or 1 eq 1 or '", DisplayName = "Literal quote")] [DataRow("50% complete", DisplayName = "Legitimate percent character")] - public void DatabasePolicyClaimValue_IsTypedAstConstantAndCosmosParameter(string claimValue) + public void StringClaim_RemainsBoundParameterAcrossSqlAndCosmos(string claimValue) + { + const string policy = "@item.textCol eq @claims.value"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", claimValue, ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual("([textCol] = @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, claimValue); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual("(c.textCol = @param0)", cosmosPredicate); + AssertParameterValues(cosmosStructure, claimValue); + } + + /// + /// Verifies aliases in root and unary Boolean positions are replaced throughout the AST + /// and generate executable, parameterized predicates for both SQL and Cosmos DB. + /// + [DataTestMethod] + [DataRow("@claims.value", "true", "(@param0 = @param1)", DisplayName = "Root Boolean claim")] + [DataRow("not @claims.value", "false", "(NOT (@param0 = @param1) )", DisplayName = "Unary Boolean claim")] + public void BooleanClaim_InRootOrUnaryPosition_IsResolvedAcrossSqlAndCosmos( + string policy, + string claimValue, + string expectedPredicate) + { + bool expectedValue = bool.Parse(claimValue); + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", claimValue, ClaimValueTypes.Boolean)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual(expectedPredicate, sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, expectedValue, true); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual(expectedPredicate, cosmosPredicate); + AssertParameterValues(cosmosStructure, expectedValue, true); + } + + /// + /// Verifies aliases remain resolved when Boolean predicates are nested under logical operators. + /// + [TestMethod] + public void BooleanClaims_InNestedLogicalExpression_AreResolvedAcrossSqlAndCosmos() + { + const string policy = "@claims.first and not @claims.second"; + const string expectedPredicate = "((@param0 = @param1) AND (NOT (@param2 = @param3) ))"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("first", "true", ClaimValueTypes.Boolean), + new Claim("second", "false", ClaimValueTypes.Boolean)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual(expectedPredicate, sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, true, true, false, true); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual(expectedPredicate, cosmosPredicate); + AssertParameterValues(cosmosStructure, true, true, false, true); + } + + /// + /// Verifies existing authorization resolver implementations remain usable for static policies. + /// + [TestMethod] + public void LegacyAuthorizationResolver_StaticPolicy_RemainsSupported() + { + Mock resolver = new(); + resolver + .Setup(instance => instance.GetDBPolicyForRequest(ENTITY_NAME, ROLE_NAME, OPERATION)) + .Returns("@item.intCol eq 42"); + Mock metadataProvider = CreateMetadataProvider(); + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver.Object); + DefaultHttpContext context = new(); + context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; + + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver.Object, + metadataProvider.Object); + + Assert.AreEqual("([intCol] = @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, 42); + } + + /// + /// Verifies existing string-only resolver implementations fail closed for claim-bearing policies. + /// + [TestMethod] + public void LegacyAuthorizationResolver_ClaimPolicy_FailsClosed() { - const string claimAlias = "@dabClaim0"; - Dictionary claimValueNodes = new() + Mock resolver = new(); + resolver + .Setup(instance => instance.GetDBPolicyForRequest(ENTITY_NAME, ROLE_NAME, OPERATION)) + .Returns("@item.textCol eq @claims.value"); + Mock metadataProvider = CreateMetadataProvider(); + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver.Object); + DefaultHttpContext context = new(); + context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; + + DataApiBuilderException exception = Assert.ThrowsException(() => + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver.Object, + metadataProvider.Object)); + + Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); + Assert.AreEqual(0, sqlStructure.Parameters.Count); + } + + /// + /// Verifies string claims are promoted to the target column's numeric type before + /// SQL and Cosmos parameters are created. + /// + [TestMethod] + public void StringClaim_IsPromotedToNumericColumnType() + { + const string policy = "@item.intCol eq @claims.value"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", "42", ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual("([intCol] = @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, 42); + Assert.AreEqual(DbType.Int32, sqlStructure.Parameters["@param0"].DbType); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual("(c.intCol = @param0)", cosmosPredicate); + AssertParameterValues(cosmosStructure, 42); + } + + /// + /// Verifies null claims remain typed null AST constants and do not create provider parameters. + /// + [TestMethod] + public void NullClaim_ProducesNullPredicateWithoutParameter() + { + const string policy = "@item.textCol eq @claims.value"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", "null", JsonClaimValueTypes.JsonNull)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual("([textCol] IS NULL)", sqlStructure.GetDbPolicyForOperation(OPERATION)); + Assert.AreEqual(0, sqlStructure.Parameters.Count); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual("(c.textCol IS NULL)", cosmosPredicate); + Assert.AreEqual(0, cosmosStructure.Parameters.Count); + } + + /// + /// Verifies non-finite floating-point claims fail before either query structure can + /// collect a provider parameter. + /// + [DataTestMethod] + [DataRow("NaN")] + [DataRow("Infinity")] + [DataRow("-Infinity")] + [DataRow("1e9999")] + public void NonFiniteDoubleClaim_FailsBeforeParameterCollection(string claimValue) + { + const string policy = "@item.doubleCol eq @claims.value"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("value", claimValue, ClaimValueTypes.Double)); + 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.UnsupportedClaimValueType, exception.SubStatusCode); + Assert.AreEqual(0, sqlStructure.Parameters.Count); + } + + /// + /// Verifies resolved policies own a read-only snapshot, including the shared empty value. + /// + [TestMethod] + public void ResolvedPolicyClaimValues_AreImmutableSnapshots() + { + Dictionary source = new() { ["@claim"] = "original" }; + ResolvedDatabasePolicy policy = new("value eq @claim", source); + source["@claim"] = "modified"; + + Assert.AreEqual("original", policy.ClaimValues["@claim"]); + Assert.IsInstanceOfType>(ResolvedDatabasePolicy.Empty.ClaimValues); + IDictionary emptyValues = (IDictionary)ResolvedDatabasePolicy.Empty.ClaimValues; + Assert.ThrowsException(() => emptyValues.Add("@claim", "value")); + } + + private static FilterClause ResolveFilterClause( + AuthorizationResolver resolver, + DefaultHttpContext context, + ISqlMetadataProvider metadataProvider) + { + ResolvedDatabasePolicy resolvedPolicy = resolver.ResolveDBPolicy( + ENTITY_NAME, + ROLE_NAME, + OPERATION, + context); + + return AuthorizationPolicyHelpers.GetDBPolicyClauseForQueryStructure( + resolvedPolicy, + ENTITY_NAME, + $"{ENTITY_NAME}.{metadataProvider.EntityToDatabaseObject[ENTITY_NAME].FullName}", + metadataProvider)!; + } + + private static (AuthorizationResolver Resolver, DefaultHttpContext Context) CreateAuthorizationContext( + string policy, + params Claim[] claims) + { + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + entityName: ENTITY_NAME, + roleName: ROLE_NAME, + operation: OPERATION, + databasePolicy: policy); + AuthorizationResolver resolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + ClaimsIdentity identity = new( + claims, + authenticationType: "TestAuth", + nameType: ClaimTypes.Name, + roleType: ClaimTypes.Role); + DefaultHttpContext context = new() { - [claimAlias] = new ConstantNode(claimValue) + User = new ClaimsPrincipal(identity) }; + context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; - ODataUriParser parser = new( - BuildModel(), - new Uri($"Entities/?$filter=name eq {claimAlias}", UriKind.Relative)) + return (resolver, context); + } + + private static Mock CreateMetadataProvider() + { + SourceDefinition sourceDefinition = new(); + sourceDefinition.Columns.Add("id", new ColumnDefinition(typeof(int)) { DbType = DbType.Int32 }); + sourceDefinition.Columns.Add("flag", new ColumnDefinition(typeof(bool)) { DbType = DbType.Boolean }); + sourceDefinition.Columns.Add("textCol", new ColumnDefinition(typeof(string)) { DbType = DbType.String }); + sourceDefinition.Columns.Add("intCol", new ColumnDefinition(typeof(int)) { DbType = DbType.Int32 }); + sourceDefinition.Columns.Add("doubleCol", new ColumnDefinition(typeof(double)) { DbType = DbType.Double }); + sourceDefinition.PrimaryKey.Add("id"); + + DatabaseObject databaseObject = new DatabaseTable(schemaName: "dbo", tableName: "PolicyTable"); + Dictionary entities = new() { - Resolver = new ClaimsTypeDataUriResolver(claimValueNodes) + [ENTITY_NAME] = databaseObject }; - foreach ((string alias, SingleValueNode valueNode) in claimValueNodes) + Mock metadataProvider = new(); + metadataProvider.SetupGet(provider => provider.EntityToDatabaseObject).Returns(entities); + metadataProvider.Setup(provider => provider.GetEntityNamesAndDbObjects()).Returns(entities); + 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()); + + string? exposedName; + metadataProvider + .Setup(provider => provider.TryGetExposedColumnName(It.IsAny(), It.IsAny(), out exposedName)) + .Callback(new ColumnNameCallback((string _, string column, out string? name) => name = column)) + .Returns(true); + + string? backingName; + metadataProvider + .Setup(provider => provider.TryGetBackingColumn(It.IsAny(), It.IsAny(), out backingName)) + .Callback(new ColumnNameCallback((string _, string column, out string? name) => name = column)) + .Returns(true); + + ODataParser parser = new(); + parser.BuildModel(metadataProvider.Object); + metadataProvider.Setup(provider => provider.GetODataParser()).Returns(parser); + return metadataProvider; + } + + private static void AssertParameterValues(BaseQueryStructure structure, params object?[] expectedValues) + { + object?[] actualValues = structure.Parameters.Values + .Select(parameter => parameter.Value) + .ToArray(); + CollectionAssert.AreEqual(expectedValues, actualValues); + } + + private delegate void ColumnNameCallback(string entity, string column, out string? name); + + private sealed class TestSqlQueryStructure : BaseSqlQueryStructure + { + public TestSqlQueryStructure( + ISqlMetadataProvider metadataProvider, + IAuthorizationResolver authorizationResolver) + : base( + metadataProvider, + authorizationResolver, + gQLFilterParser: null!, + entityName: ENTITY_NAME) { - parser.ParameterAliasNodes.Add(alias, valueNode); } - - FilterClause clause = parser.ParseFilter(); - BinaryOperatorNode comparison = (BinaryOperatorNode)clause.Expression; - Assert.AreEqual(BinaryOperatorKind.Equal, comparison.OperatorKind); - Assert.IsInstanceOfType(comparison.Right); - Assert.AreEqual(claimValue, ((ConstantNode)comparison.Right).Value); - - List parameterValues = new(); - string predicate = clause.Expression.Accept(new ODataASTCosmosVisitor( - "c", - value => - { - parameterValues.Add(value); - return "@param1"; - })); - - Assert.AreEqual("(c.name = @param1)", predicate); - CollectionAssert.AreEqual(new object?[] { claimValue }, parameterValues); } - private static IEdmModel BuildModel() + private sealed class TestQueryStructure : BaseQueryStructure { - EdmModel model = new(); - EdmEntityType entityType = new("Dab", "Entity"); - EdmStructuralProperty id = entityType.AddStructuralProperty("id", EdmPrimitiveTypeKind.Int32, false); - entityType.AddKeys(id); - entityType.AddStructuralProperty("name", EdmPrimitiveTypeKind.String, true); - model.AddElement(entityType); - - EdmEntityContainer container = new("Dab", "Container"); - container.AddEntitySet("Entities", entityType); - model.AddElement(container); - return model; + public TestQueryStructure( + ISqlMetadataProvider metadataProvider, + IAuthorizationResolver authorizationResolver) + : base( + metadataProvider, + authorizationResolver, + gQLFilterParser: null!, + entityName: ENTITY_NAME) + { + } } } } diff --git a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs index 8a8a5f2341..5e4dc4a270 100644 --- a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs +++ b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs @@ -139,12 +139,9 @@ private static SqlUpsertQueryStructure CreateUpsertStructure() => _columnMapping.TryGetValue(field, out column))) .Returns((string entity, string field, string? column) => _columnMapping.ContainsKey(field)); - // The update policy is injected directly onto the structure, so the resolver only needs - // to return an empty policy (no throw) during construction. + // The update policy is injected directly onto the structure, so this test does not + // invoke the resolver while constructing the query structure. Mock authorizationResolver = new(); - authorizationResolver - .Setup(x => x.ProcessDBPolicy(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(ResolvedDatabasePolicy.Empty); RuntimeConfigProvider runtimeConfigProvider = TestHelper.GetRuntimeConfigProvider(TestHelper.GetRuntimeConfigLoader()); Mock metadataProviderFactory = new(); From 118417bf2211839efd447e6e4919a0f66cfef6e8 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Wed, 5 Aug 2026 15:30:59 -0700 Subject: [PATCH 3/8] fix bugs --- .../Authorization/AuthorizationResolver.cs | 6 +- src/Core/Parsers/ClaimsTypeDataUriResolver.cs | 17 +++-- src/Core/Parsers/FilterParser.cs | 4 +- src/Core/Parsers/ParameterAliasRewriter.cs | 20 +++++- .../AuthorizationResolverUnitTests.cs | 3 + .../DatabasePolicyClaimBindingUnitTests.cs | 67 +++++++++++++++++++ 6 files changed, 108 insertions(+), 9 deletions(-) diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 4b896354f0..62cdfef745 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -925,8 +925,12 @@ private static double ParseFiniteDoubleClaimValue(string value) private static DataApiBuilderException CreateUnsupportedClaimValueException(Claim claim, Exception? innerException = null) { + string message = innerException is null + ? $"The claim value for claim: {claim.Type} belonging to the user has an unsupported data type." + : $"The claim value for claim: {claim.Type} belonging to the user is invalid for its declared data type."; + return new DataApiBuilderException( - message: $"The claim value for claim: {claim.Type} belonging to the user has an unsupported data type.", + message: message, statusCode: HttpStatusCode.Forbidden, subStatusCode: DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType, innerException: innerException); diff --git a/src/Core/Parsers/ClaimsTypeDataUriResolver.cs b/src/Core/Parsers/ClaimsTypeDataUriResolver.cs index d1fdf6dc40..f8f213dfa2 100644 --- a/src/Core/Parsers/ClaimsTypeDataUriResolver.cs +++ b/src/Core/Parsers/ClaimsTypeDataUriResolver.cs @@ -37,19 +37,26 @@ public override void PromoteBinaryOperandTypes(BinaryOperatorKind binaryOperator ResolveClaimAlias(ref leftNode); ResolveClaimAlias(ref rightNode); - if (leftNode.TypeReference.PrimitiveKind() != rightNode.TypeReference.PrimitiveKind()) + EdmPrimitiveTypeKind? leftPrimitiveKind = leftNode.TypeReference?.PrimitiveKind(); + EdmPrimitiveTypeKind? rightPrimitiveKind = rightNode.TypeReference?.PrimitiveKind(); + + if (leftPrimitiveKind != rightPrimitiveKind) { - if ((leftNode.Kind == QueryNodeKind.SingleValuePropertyAccess) && (rightNode is ConstantNode)) + if (leftPrimitiveKind.HasValue && + leftNode.Kind == QueryNodeKind.SingleValuePropertyAccess && + rightNode is ConstantNode) { TryConvertNodeToTargetType( - targetType: leftNode.TypeReference.PrimitiveKind(), + targetType: leftPrimitiveKind.Value, operandToConvert: ref rightNode ); } - else if (rightNode.Kind == QueryNodeKind.SingleValuePropertyAccess && leftNode is ConstantNode) + else if (rightPrimitiveKind.HasValue && + rightNode.Kind == QueryNodeKind.SingleValuePropertyAccess && + leftNode is ConstantNode) { TryConvertNodeToTargetType( - targetType: rightNode.TypeReference.PrimitiveKind(), + targetType: rightPrimitiveKind.Value, operandToConvert: ref leftNode ); } diff --git a/src/Core/Parsers/FilterParser.cs b/src/Core/Parsers/FilterParser.cs index f261a6e197..e79e559ee5 100644 --- a/src/Core/Parsers/FilterParser.cs +++ b/src/Core/Parsers/FilterParser.cs @@ -65,7 +65,7 @@ public FilterClause GetFilterClause( parser.Resolver = customResolver; } - if (parameterAliasNodes is not null) + if (parameterAliasNodes is { Count: > 0 }) { foreach ((string alias, SingleValueNode valueNode) in parameterAliasNodes) { @@ -74,7 +74,7 @@ public FilterClause GetFilterClause( } FilterClause filterClause = parser.ParseFilter(); - return parameterAliasNodes is null + return parameterAliasNodes is not { Count: > 0 } ? filterClause : new ParameterAliasRewriter(parameterAliasNodes).Rewrite(filterClause); } diff --git a/src/Core/Parsers/ParameterAliasRewriter.cs b/src/Core/Parsers/ParameterAliasRewriter.cs index cfcef05dba..22595cb7bb 100644 --- a/src/Core/Parsers/ParameterAliasRewriter.cs +++ b/src/Core/Parsers/ParameterAliasRewriter.cs @@ -73,7 +73,9 @@ private SingleValueNode RewriteAlias(ParameterAliasNode aliasNode, bool isPredic private static SingleValueNode NormalizeBooleanPredicate(SingleValueNode node, bool isPredicate) { - if (!isPredicate || node.TypeReference?.PrimitiveKind() is not EdmPrimitiveTypeKind.Boolean) + if (!isPredicate || + node.TypeReference?.PrimitiveKind() is not EdmPrimitiveTypeKind.Boolean || + IsPredicateExpression(node)) { return node; } @@ -83,4 +85,20 @@ private static SingleValueNode NormalizeBooleanPredicate(SingleValueNode node, b node, new ConstantNode(true)); } + + /// + /// Returns whether a Boolean node already represents a predicate rather than a bare value. + /// OData can wrap comparison predicates in one or more conversion nodes when binding logical + /// operators. Such predicates must not be rewritten as "predicate eq true", which is invalid SQL. + /// + private static bool IsPredicateExpression(SingleValueNode node) + { + return node switch + { + BinaryOperatorNode => true, + UnaryOperatorNode => true, + ConvertNode convertNode => IsPredicateExpression(convertNode.Source), + _ => false + }; + } } diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 93ef5a68a6..0639c03451 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1513,6 +1513,9 @@ public void DbPolicy_InvalidPrimitiveClaim_FailsClosed(string claimValueType, st Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); Assert.AreEqual(DataApiBuilderException.SubStatusCodes.UnsupportedClaimValueType, exception.SubStatusCode); + Assert.AreEqual( + "The claim value for claim: testClaim belonging to the user is invalid for its declared data type.", + exception.Message); } /// diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index 6c7856de40..1c29dac4fc 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -145,6 +145,73 @@ public void BooleanClaims_InNestedLogicalExpression_AreResolvedAcrossSqlAndCosmo AssertParameterValues(cosmosStructure, true, true, false, true); } + /// + /// Verifies ordinary comparison predicates are not rewritten as comparisons to Boolean true. + /// + [TestMethod] + public void StaticComparisonPolicy_RemainsValidAcrossSqlAndCosmos() + { + const string policy = "@item.intCol ne 6 and @item.doubleCol gt 0"; + const string expectedSqlPredicate = "(([intCol] != @param0) AND ([doubleCol] > @param1))"; + const string expectedCosmosPredicate = "((c.intCol != @param0) AND (c.doubleCol > @param1))"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext(policy); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual(expectedSqlPredicate, sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, 6, 0d); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual(expectedCosmosPredicate, cosmosPredicate); + AssertParameterValues(cosmosStructure, 6, 0d); + } + + /// + /// Verifies a bare Boolean claim can be combined with an ordinary comparison without + /// rewriting the comparison predicate as "predicate equals true". + /// + [TestMethod] + public void BooleanClaim_CombinedWithComparison_OnlyNormalizesBareClaim() + { + const string policy = "@item.intCol ne 6 and @claims.allowed"; + const string expectedSqlPredicate = "(([intCol] != @param0) AND (@param1 = @param2))"; + const string expectedCosmosPredicate = "((c.intCol != @param0) AND (@param1 = @param2))"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + policy, + new Claim("allowed", "true", ClaimValueTypes.Boolean)); + Mock metadataProvider = CreateMetadataProvider(); + + TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver); + AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( + OPERATION, + sqlStructure, + context, + resolver, + metadataProvider.Object); + + Assert.AreEqual(expectedSqlPredicate, sqlStructure.GetDbPolicyForOperation(OPERATION)); + AssertParameterValues(sqlStructure, 6, true, true); + + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + string cosmosPredicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor("c", cosmosStructure)); + + Assert.AreEqual(expectedCosmosPredicate, cosmosPredicate); + AssertParameterValues(cosmosStructure, 6, true, true); + } + /// /// Verifies existing authorization resolver implementations remain usable for static policies. /// From 60f660d103d739db0a52dc98886cf55c40500de5 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Wed, 5 Aug 2026 16:51:35 -0700 Subject: [PATCH 4/8] remove obsolete method --- src/Auth/IAuthorizationResolver.cs | 14 ++--- src/Auth/IResolvedDatabasePolicyProvider.cs | 28 ---------- src/Auth/ResolvedDatabasePolicy.cs | 20 +++---- .../Authorization/AuthorizationResolver.cs | 24 +------- .../Resolvers/AuthorizationPolicyHelpers.cs | 39 +------------ .../AuthorizationResolverUnitTests.cs | 46 ++-------------- .../REST/RestAuthorizationHandlerUnitTests.cs | 9 ++- .../DabCacheServiceIntegrationTests.cs | 14 +++++ .../DatabasePolicyClaimBindingUnitTests.cs | 55 ------------------- .../UnitTests/DwSqlQueryBuilderUpsertTests.cs | 10 +++- 10 files changed, 47 insertions(+), 212 deletions(-) delete mode 100644 src/Auth/IResolvedDatabasePolicyProvider.cs diff --git a/src/Auth/IAuthorizationResolver.cs b/src/Auth/IAuthorizationResolver.cs index 27e3b330eb..c990611f9a 100644 --- a/src/Auth/IAuthorizationResolver.cs +++ b/src/Auth/IAuthorizationResolver.cs @@ -72,21 +72,15 @@ public interface IAuthorizationResolver public string GetDBPolicyForRequest(string entityName, string roleName, EntityActionOperation operation); /// - /// Retrieves the policy of an operation within an entity's role entry - /// within the permissions section of the runtime config, and tries to process - /// the policy. + /// Resolves claim references in a database policy to parameter aliases and + /// returns their typed values separately from the policy text. /// /// Entity from request. /// Role defined in client role header. /// Operation type: Create, Read, Update, Delete. /// Contains token claims of the authenticated user used in policy evaluation. - /// Returns the parsed policy, if safely representable as text, or an exception otherwise. - /// - /// Claim-bearing policies cannot be represented safely by this string-only contract. - /// Use instead. - /// - [Obsolete("Use IResolvedDatabasePolicyProvider.ResolveDBPolicy to keep claim values separate from policy text.")] - public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); + /// The policy text and typed claim values to bind to it. + public ResolvedDatabasePolicy ResolveDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext); /// /// Get list of roles defined for entity within runtime configuration.. This is applicable for GraphQL when creating authorization diff --git a/src/Auth/IResolvedDatabasePolicyProvider.cs b/src/Auth/IResolvedDatabasePolicyProvider.cs deleted file mode 100644 index f816b15d99..0000000000 --- a/src/Auth/IResolvedDatabasePolicyProvider.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Azure.DataApiBuilder.Config.ObjectModel; -using Microsoft.AspNetCore.Http; - -namespace Azure.DataApiBuilder.Auth; - -/// -/// Resolves database authorization policies while keeping claim values separate -/// from policy text. -/// -public interface IResolvedDatabasePolicyProvider -{ - /// - /// Resolves claim references to parameter aliases and returns their typed values separately. - /// - /// Entity from the request. - /// Role defined in the client role header. - /// Operation type: Create, Read, Update, Delete. - /// Contains the authenticated user's token claims. - /// The policy text and typed claim values to bind to it. - public ResolvedDatabasePolicy ResolveDBPolicy( - string entityName, - string roleName, - EntityActionOperation operation, - HttpContext httpContext); -} diff --git a/src/Auth/ResolvedDatabasePolicy.cs b/src/Auth/ResolvedDatabasePolicy.cs index 1e4e8a2676..75494b19f8 100644 --- a/src/Auth/ResolvedDatabasePolicy.cs +++ b/src/Auth/ResolvedDatabasePolicy.cs @@ -12,6 +12,16 @@ namespace Azure.DataApiBuilder.Auth; /// public sealed record ResolvedDatabasePolicy { + /// + /// Policy text containing OData parameter aliases. + /// + public string Policy { get; } + + /// + /// Immutable snapshot of typed claim values keyed by parameter alias. + /// + public IReadOnlyDictionary ClaimValues { get; } + /// /// Initializes a resolved database policy and takes an immutable snapshot of its claim values. /// @@ -27,16 +37,6 @@ public ResolvedDatabasePolicy(string policy, IReadOnlyDictionary(claimValues, StringComparer.Ordinal)); } - /// - /// Policy text containing OData parameter aliases. - /// - public string Policy { get; } - - /// - /// Immutable snapshot of typed claim values keyed by parameter alias. - /// - public IReadOnlyDictionary ClaimValues { get; } - /// /// Represents an operation without a database authorization policy. /// diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 62cdfef745..fd0da59393 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -25,7 +25,7 @@ namespace Azure.DataApiBuilder.Core.Authorization; /// Authorization stages that require passing before a request is executed /// against a database. /// -public class AuthorizationResolver : IAuthorizationResolver, IResolvedDatabasePolicyProvider +public class AuthorizationResolver : IAuthorizationResolver { private readonly RuntimeConfigProvider _runtimeConfigProvider; private readonly IMetadataProviderFactory _metadataProviderFactory; @@ -205,28 +205,6 @@ public bool AreColumnsAllowedForOperation(string entityName, string roleName, En return false; } - /// - [Obsolete("Use IResolvedDatabasePolicyProvider.ResolveDBPolicy to keep claim values separate from policy text.")] - public string ProcessDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) - { - string dbPolicy = GetDBPolicyForRequest(entityName, roleName, operation); - - if (string.IsNullOrWhiteSpace(dbPolicy)) - { - return string.Empty; - } - - if (dbPolicy.Contains(CLAIM_PREFIX, StringComparison.Ordinal)) - { - throw new DataApiBuilderException( - message: "Claim-bearing database policies require typed claim binding.", - statusCode: HttpStatusCode.Forbidden, - subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed); - } - - return dbPolicy.Replace(FIELD_PREFIX, string.Empty); - } - /// public ResolvedDatabasePolicy ResolveDBPolicy(string entityName, string roleName, EntityActionOperation operation, HttpContext httpContext) { diff --git a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index 7bfa9c8b5e..f2efc35cf9 100644 --- a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs +++ b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs @@ -164,8 +164,7 @@ private static List ProcessFilter( List filterClauses = new(); foreach (EntityActionOperation elementalOperation in elementalOperations) { - ResolvedDatabasePolicy dbQueryPolicy = ResolveDatabasePolicy( - authorizationResolver, + ResolvedDatabasePolicy dbQueryPolicy = authorizationResolver.ResolveDBPolicy( entityName, clientRoleHeader, elementalOperation, @@ -183,42 +182,6 @@ private static List ProcessFilter( return filterClauses; } - /// - /// Uses typed claim binding when supported. Legacy authorization resolver implementations - /// remain compatible for static policies, but claim-bearing policies fail closed because - /// the legacy string contract cannot keep claim data separate from policy syntax. - /// - private static ResolvedDatabasePolicy ResolveDatabasePolicy( - IAuthorizationResolver authorizationResolver, - string entityName, - string roleName, - EntityActionOperation operation, - HttpContext context) - { - if (authorizationResolver is IResolvedDatabasePolicyProvider resolvedPolicyProvider) - { - return resolvedPolicyProvider.ResolveDBPolicy(entityName, roleName, operation, context); - } - - string policy = authorizationResolver.GetDBPolicyForRequest(entityName, roleName, operation); - if (string.IsNullOrWhiteSpace(policy)) - { - return ResolvedDatabasePolicy.Empty; - } - - if (policy.Contains(AuthorizationResolver.CLAIM_PREFIX, StringComparison.Ordinal)) - { - throw new DataApiBuilderException( - message: "The authorization resolver does not support typed database policy claims.", - statusCode: System.Net.HttpStatusCode.Forbidden, - subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed); - } - - return new ResolvedDatabasePolicy( - policy.Replace(AuthorizationResolver.FIELD_PREFIX, string.Empty), - new Dictionary()); - } - /// /// Appends the filter query formatting to a resolved database policy and parses it with ODataParser. /// diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 0639c03451..e5896cf6ce 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1369,40 +1369,6 @@ public void DbPolicy_StringClaim_UsesTypedParameterAlias(string claimValue) Assert.AreEqual(claimValue, parsedPolicy.ClaimValues["@dabClaim0"]); } - /// - /// The legacy public API keeps its string return type for source and binary compatibility, - /// but fails closed because claim values cannot be represented safely in policy text. - /// - [TestMethod] - public void DbPolicy_LegacyStringContractWithClaims_FailsClosed() - { - RuntimeConfig runtimeConfig = InitRuntimeConfig( - entityName: TEST_ENTITY, - roleName: TEST_ROLE, - operation: TEST_OPERATION, - includedCols: new HashSet { "col1" }, - databasePolicy: "@item.col1 eq @claims.userId"); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - - Mock context = new(); - ClaimsIdentity identity = new(TEST_AUTHENTICATION_TYPE, TEST_CLAIMTYPE_NAME, AuthenticationOptions.ROLE_CLAIM_TYPE); - identity.AddClaim(new Claim("userId", "alice", ClaimValueTypes.String)); - context.Setup(x => x.User).Returns(new ClaimsPrincipal(identity)); - context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(TEST_ROLE); - - Assert.AreEqual( - typeof(string), - typeof(IAuthorizationResolver).GetMethod(nameof(IAuthorizationResolver.ProcessDBPolicy))!.ReturnType); - - #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. - DataApiBuilderException exception = Assert.ThrowsException(() => - authZResolver.ProcessDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); - #pragma warning restore CS0618 - - Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); - Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); - } - /// /// Tests authorization policy processing mechanism by validating value type compatibility /// of claims present in HttpContext.User.Claims. @@ -1614,7 +1580,7 @@ public void ParsePolicyWithDuplicateUserClaims() // Indirectly tests the AuthorizationResolver private method: // GetDBPolicyForRequest(string entityName, string roleName, string operation) - // by calling public method TryProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object) + // by calling public method ResolveDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object) // The result of executing that method will determine whether execution behaves as expected. // When string.Empty is returned, // then no policy is found for the provided entity, role, and operation combination, therefore, @@ -1657,17 +1623,15 @@ public void GetDBPolicyTest( context.Setup(x => x.User).Returns(principal); context.Setup(x => x.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]).Returns(clientRole); - #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. - string parsedPolicy = authZResolver.ProcessDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); - #pragma warning restore CS0618 - string errorMessage = "TryProcessDBPolicy returned unexpected value."; + ResolvedDatabasePolicy parsedPolicy = authZResolver.ResolveDBPolicy(TEST_ENTITY, clientRole, requestOperation, context.Object); + string errorMessage = "ResolveDBPolicy returned unexpected value."; if (expectPolicy) { - Assert.AreEqual(actual: parsedPolicy, expected: policy, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy.Policy, expected: policy, message: errorMessage); } else { - Assert.AreEqual(actual: parsedPolicy, expected: string.Empty, message: errorMessage); + Assert.AreEqual(actual: parsedPolicy.Policy, expected: string.Empty, message: errorMessage); } } diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index 26264741e0..8db19814d0 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -99,14 +99,13 @@ public void TestWildcardPolicyResolvesToEmpty(string httpMethod) AuthorizationResolver authorizationResolver = SetupAuthResolverWithWildcardOperation(); HttpContext httpContext = CreateHttpContext(httpMethod: httpMethod, clientRole: "admin"); - #pragma warning disable CS0618 // Explicitly validates the preserved legacy API contract. - Assert.AreEqual(expected: string.Empty, actual: authorizationResolver.ProcessDBPolicy( + ResolvedDatabasePolicy resolvedPolicy = authorizationResolver.ResolveDBPolicy( entityName: AuthorizationHelpers.TEST_ENTITY, roleName: "admin", operation: RestService.HttpVerbToOperations(httpVerbName: httpMethod), - httpContext: httpContext) - ); - #pragma warning restore CS0618 + httpContext: httpContext); + + Assert.AreEqual(expected: string.Empty, actual: resolvedPolicy.Policy); } /// diff --git a/src/Service.Tests/Caching/DabCacheServiceIntegrationTests.cs b/src/Service.Tests/Caching/DabCacheServiceIntegrationTests.cs index 3bf6e37012..2bcb97a931 100644 --- a/src/Service.Tests/Caching/DabCacheServiceIntegrationTests.cs +++ b/src/Service.Tests/Caching/DabCacheServiceIntegrationTests.cs @@ -717,6 +717,13 @@ private static Mock CreateMockSqlQueryStructure(string entity .Returns(entityToDatabaseObject); Mock mockMetadataProviderFactory = new(); Mock mockAuthorizationResolver = new(); + mockAuthorizationResolver + .Setup(resolver => resolver.ResolveDBPolicy( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(ResolvedDatabasePolicy.Empty); Mock mockRestRequestContext = new( entityName, new DatabaseTable()); @@ -817,6 +824,13 @@ private static SqlQueryEngine CreateQueryEngine(DabCacheService cache, string qu Mock mockMetadataProviderFactory = new(); Mock mockHttpContextAccessor = new(); Mock mockAuthorizationResolver = new(); + mockAuthorizationResolver + .Setup(resolver => resolver.ResolveDBPolicy( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(ResolvedDatabasePolicy.Empty); Mock> mockLogger = new(); Mock mockRuntimeConfigProvider = CreateMockRuntimeConfigProvider(entityName); Mock mockFilterParser = new(mockRuntimeConfigProvider.Object, mockMetadataProviderFactory.Object); diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index 1c29dac4fc..70d6622177 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Data; using System.Linq; -using System.Net; using System.Security.Claims; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config.DatabasePrimitives; @@ -212,60 +211,6 @@ public void BooleanClaim_CombinedWithComparison_OnlyNormalizesBareClaim() AssertParameterValues(cosmosStructure, 6, true, true); } - /// - /// Verifies existing authorization resolver implementations remain usable for static policies. - /// - [TestMethod] - public void LegacyAuthorizationResolver_StaticPolicy_RemainsSupported() - { - Mock resolver = new(); - resolver - .Setup(instance => instance.GetDBPolicyForRequest(ENTITY_NAME, ROLE_NAME, OPERATION)) - .Returns("@item.intCol eq 42"); - Mock metadataProvider = CreateMetadataProvider(); - TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver.Object); - DefaultHttpContext context = new(); - context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; - - AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( - OPERATION, - sqlStructure, - context, - resolver.Object, - metadataProvider.Object); - - Assert.AreEqual("([intCol] = @param0)", sqlStructure.GetDbPolicyForOperation(OPERATION)); - AssertParameterValues(sqlStructure, 42); - } - - /// - /// Verifies existing string-only resolver implementations fail closed for claim-bearing policies. - /// - [TestMethod] - public void LegacyAuthorizationResolver_ClaimPolicy_FailsClosed() - { - Mock resolver = new(); - resolver - .Setup(instance => instance.GetDBPolicyForRequest(ENTITY_NAME, ROLE_NAME, OPERATION)) - .Returns("@item.textCol eq @claims.value"); - Mock metadataProvider = CreateMetadataProvider(); - TestSqlQueryStructure sqlStructure = new(metadataProvider.Object, resolver.Object); - DefaultHttpContext context = new(); - context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; - - DataApiBuilderException exception = Assert.ThrowsException(() => - AuthorizationPolicyHelpers.ProcessAuthorizationPolicies( - OPERATION, - sqlStructure, - context, - resolver.Object, - metadataProvider.Object)); - - Assert.AreEqual(HttpStatusCode.Forbidden, exception.StatusCode); - Assert.AreEqual(DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed, exception.SubStatusCode); - Assert.AreEqual(0, sqlStructure.Parameters.Count); - } - /// /// Verifies string claims are promoted to the target column's numeric type before /// SQL and Cosmos parameters are created. diff --git a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs index 5e4dc4a270..ac5de6109a 100644 --- a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs +++ b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs @@ -139,9 +139,15 @@ private static SqlUpsertQueryStructure CreateUpsertStructure() => _columnMapping.TryGetValue(field, out column))) .Returns((string entity, string field, string? column) => _columnMapping.ContainsKey(field)); - // The update policy is injected directly onto the structure, so this test does not - // invoke the resolver while constructing the query structure. + // The update policy is injected directly onto the structure after construction. Mock authorizationResolver = new(); + authorizationResolver + .Setup(x => x.ResolveDBPolicy( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(ResolvedDatabasePolicy.Empty); RuntimeConfigProvider runtimeConfigProvider = TestHelper.GetRuntimeConfigProvider(TestHelper.GetRuntimeConfigLoader()); Mock metadataProviderFactory = new(); From 766e603012a28350a96c5c7c528a60dc1c8978a6 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Fri, 7 Aug 2026 13:53:17 -0700 Subject: [PATCH 5/8] Add end-to-end tests for policy claim binding --- config-generators/cosmosdb_nosql-commands.txt | 1 + config-generators/dwsql-commands.txt | 1 + config-generators/mssql-commands.txt | 1 + config-generators/mysql-commands.txt | 1 + config-generators/postgresql-commands.txt | 1 + src/Auth/ResolvedDatabasePolicy.cs | 14 ++--- src/Core/Parsers/ParameterAliasRewriter.cs | 32 ++++++++++ .../AuthorizationResolverUnitTests.cs | 10 ++-- .../CosmosTests/QueryFilterTests.cs | 52 ++++++++++++++++ .../RestApiTests/Find/FindApiTestBase.cs | 60 +++++++++++++++++++ .../dab-config.CosmosDb_NoSql.json | 11 ++++ src/Service.Tests/dab-config.DwSql.json | 17 ++++++ src/Service.Tests/dab-config.MsSql.json | 17 ++++++ src/Service.Tests/dab-config.MySql.json | 17 ++++++ src/Service.Tests/dab-config.PostgreSql.json | 17 ++++++ 15 files changed, 240 insertions(+), 12 deletions(-) diff --git a/config-generators/cosmosdb_nosql-commands.txt b/config-generators/cosmosdb_nosql-commands.txt index 392a3a7b78..51e4e5b364 100644 --- a/config-generators/cosmosdb_nosql-commands.txt +++ b/config-generators/cosmosdb_nosql-commands.txt @@ -4,6 +4,7 @@ update PlanetAlias --config "dab-config.CosmosDb_NoSql.json" --permissions "anon update PlanetAlias --config "dab-config.CosmosDb_NoSql.json" --permissions "authenticated:create,read,update,delete" update PlanetAlias --config "dab-config.CosmosDb_NoSql.json" --permissions "limited-read-role:read" update PlanetAlias --config "dab-config.CosmosDb_NoSql.json" --permissions "item-level-permission-role:read" +update PlanetAlias --config "dab-config.CosmosDb_NoSql.json" --permissions "claim_policy_tester:read" --policy-database "@item.name eq @claims.userId" add Character --config "dab-config.CosmosDb_NoSql.json" --source "graphqldb.planet" --permissions "anonymous:create,read,update,delete" --graphql "Character:Characters" update Character --config "dab-config.CosmosDb_NoSql.json" --permissions "item-level-permission-role:read" add Star --config "dab-config.CosmosDb_NoSql.json" --source "graphqldb.planet" --permissions "anonymous:create,read,update,delete" --graphql "Star:Stars" diff --git a/config-generators/dwsql-commands.txt b/config-generators/dwsql-commands.txt index 1728dfd25a..e58eac3b2f 100644 --- a/config-generators/dwsql-commands.txt +++ b/config-generators/dwsql-commands.txt @@ -101,6 +101,7 @@ update Book --config "dab-config.DwSql.json" --permissions "policy_tester_08:cre update Book --config "dab-config.DwSql.json" --permissions "policy_tester_08:update" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.DwSql.json" --permissions "policy_tester_08:delete" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.DwSql.json" --permissions "policy_tester_08:read" --fields.include "*" +update Book --config "dab-config.DwSql.json" --permissions "claim_policy_tester:read" --fields.include "*" --policy-database "@item.title eq @claims.userId" update Book --config "dab-config.DwSql.json" --permissions "test_role_with_noread:create,update,delete" update Book --config "dab-config.DwSql.json" --permissions "test_role_with_excluded_fields:create,update,delete" update Book --config "dab-config.DwSql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "publisher_id" diff --git a/config-generators/mssql-commands.txt b/config-generators/mssql-commands.txt index cfa338be32..277b9878f0 100644 --- a/config-generators/mssql-commands.txt +++ b/config-generators/mssql-commands.txt @@ -124,6 +124,7 @@ update Book --config "dab-config.MsSql.json" --permissions "policy_tester_08:cre update Book --config "dab-config.MsSql.json" --permissions "policy_tester_08:update" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.MsSql.json" --permissions "policy_tester_08:delete" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.MsSql.json" --permissions "policy_tester_08:read" --fields.include "*" +update Book --config "dab-config.MsSql.json" --permissions "claim_policy_tester:read" --fields.include "*" --policy-database "@item.title eq @claims.userId" update Book --config "dab-config.MsSql.json" --permissions "test_role_with_noread:create,update,delete" update Book --config "dab-config.MsSql.json" --permissions "test_role_with_excluded_fields:create,update,delete" update Book --config "dab-config.MsSql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "publisher_id" diff --git a/config-generators/mysql-commands.txt b/config-generators/mysql-commands.txt index 30f3f5e5a1..1b796a0412 100644 --- a/config-generators/mysql-commands.txt +++ b/config-generators/mysql-commands.txt @@ -96,6 +96,7 @@ update Book --config "dab-config.MySql.json" --permissions "policy_tester_08:cre update Book --config "dab-config.MySql.json" --permissions "policy_tester_08:update" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.MySql.json" --permissions "policy_tester_08:delete" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.MySql.json" --permissions "policy_tester_08:read" --fields.include "*" +update Book --config "dab-config.MySql.json" --permissions "claim_policy_tester:read" --fields.include "*" --policy-database "@item.title eq @claims.userId" update Book --config "dab-config.MySql.json" --permissions "test_role_with_noread:create,update,delete" update Book --config "dab-config.MySql.json" --permissions "test_role_with_excluded_fields:create,update,delete" update Book --config "dab-config.MySql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "publisher_id" diff --git a/config-generators/postgresql-commands.txt b/config-generators/postgresql-commands.txt index cf0d8dc1fa..3b4f0863e6 100644 --- a/config-generators/postgresql-commands.txt +++ b/config-generators/postgresql-commands.txt @@ -98,6 +98,7 @@ update Book --config "dab-config.PostgreSql.json" --permissions "policy_tester_0 update Book --config "dab-config.PostgreSql.json" --permissions "policy_tester_08:update" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.PostgreSql.json" --permissions "policy_tester_08:delete" --fields.include "*" --policy-database "@item.id eq 9" update Book --config "dab-config.PostgreSql.json" --permissions "policy_tester_08:read" --fields.include "*" +update Book --config "dab-config.PostgreSql.json" --permissions "claim_policy_tester:read" --fields.include "*" --policy-database "@item.title eq @claims.userId" update Book --config "dab-config.PostgreSql.json" --permissions "test_role_with_noread:create,update,delete" update Book --config "dab-config.PostgreSql.json" --permissions "test_role_with_excluded_fields:create,update,delete" update Book --config "dab-config.PostgreSql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "publisher_id" diff --git a/src/Auth/ResolvedDatabasePolicy.cs b/src/Auth/ResolvedDatabasePolicy.cs index 75494b19f8..08ad51edd9 100644 --- a/src/Auth/ResolvedDatabasePolicy.cs +++ b/src/Auth/ResolvedDatabasePolicy.cs @@ -22,6 +22,13 @@ public sealed record ResolvedDatabasePolicy /// public IReadOnlyDictionary ClaimValues { get; } + /// + /// Represents an operation without a database authorization policy. + /// + public static ResolvedDatabasePolicy Empty { get; } = new( + string.Empty, + new ReadOnlyDictionary(new Dictionary())); + /// /// Initializes a resolved database policy and takes an immutable snapshot of its claim values. /// @@ -36,11 +43,4 @@ public ResolvedDatabasePolicy(string policy, IReadOnlyDictionary( new Dictionary(claimValues, StringComparer.Ordinal)); } - - /// - /// Represents an operation without a database authorization policy. - /// - public static ResolvedDatabasePolicy Empty { get; } = new( - string.Empty, - new ReadOnlyDictionary(new Dictionary())); } diff --git a/src/Core/Parsers/ParameterAliasRewriter.cs b/src/Core/Parsers/ParameterAliasRewriter.cs index 22595cb7bb..3aef5624ab 100644 --- a/src/Core/Parsers/ParameterAliasRewriter.cs +++ b/src/Core/Parsers/ParameterAliasRewriter.cs @@ -16,6 +16,10 @@ internal sealed class ParameterAliasRewriter { private readonly IReadOnlyDictionary _parameterAliasNodes; + /// + /// Initializes a rewriter with the typed values available for policy parameter aliases. + /// + /// Typed AST values keyed by parameter alias. public ParameterAliasRewriter(IReadOnlyDictionary parameterAliasNodes) { _parameterAliasNodes = parameterAliasNodes; @@ -31,6 +35,13 @@ public FilterClause Rewrite(FilterClause filterClause) return new FilterClause(expression, filterClause.RangeVariable); } + /// + /// Recursively replaces aliases and normalizes Boolean values according to whether the + /// current node is used as a predicate or as an operand value. + /// + /// The AST node to rewrite. + /// Whether the node occupies a Boolean predicate position. + /// The rewritten AST node. private SingleValueNode RewriteNode(SingleValueNode node, bool isPredicate) { return node switch @@ -52,6 +63,12 @@ private SingleValueNode RewriteNode(SingleValueNode node, bool isPredicate) }; } + /// + /// Rewrites both operands of a binary operator, treating operands of logical operators + /// as predicates and operands of comparison operators as values. + /// + /// The binary operator node to rewrite. + /// A binary operator node containing the rewritten operands. private SingleValueNode RewriteBinaryOperator(BinaryOperatorNode node) { bool operandsArePredicates = node.OperatorKind is BinaryOperatorKind.And or BinaryOperatorKind.Or; @@ -61,6 +78,14 @@ private SingleValueNode RewriteBinaryOperator(BinaryOperatorNode node) RewriteNode(node.Right, operandsArePredicates)); } + /// + /// Replaces a parameter alias with its supplied typed value and applies any + /// context-specific Boolean normalization to that value. + /// + /// The parameter alias to resolve. + /// Whether the alias occupies a Boolean predicate position. + /// The rewritten typed value for the alias. + /// Thrown when no value was supplied for the alias. private SingleValueNode RewriteAlias(ParameterAliasNode aliasNode, bool isPredicate) { if (!_parameterAliasNodes.TryGetValue(aliasNode.Alias, out SingleValueNode? valueNode)) @@ -71,6 +96,13 @@ private SingleValueNode RewriteAlias(ParameterAliasNode aliasNode, bool isPredic return RewriteNode(valueNode, isPredicate); } + /// + /// Converts a bare Boolean value used as a predicate into an explicit equality with + /// , while preserving expressions that are already predicates. + /// + /// The node to normalize. + /// Whether the node occupies a Boolean predicate position. + /// The original node or an explicit Boolean equality predicate. private static SingleValueNode NormalizeBooleanPredicate(SingleValueNode node, bool isPredicate) { if (!isPredicate || diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index e5896cf6ce..1ab41a7c1a 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1453,11 +1453,11 @@ public void DbPolicy_ClaimValueTypeParsing(string claimValueType, string claimVa /// policy parsing and must never be interpreted as OData syntax. /// [DataTestMethod] - [DataRow(ClaimValueTypes.Integer, "1 or 1 eq 1", DisplayName = "Malformed integer")] - [DataRow(ClaimValueTypes.Double, "NaN", DisplayName = "NaN")] - [DataRow(ClaimValueTypes.Double, "Infinity", DisplayName = "Positive infinity")] - [DataRow(ClaimValueTypes.Double, "-Infinity", DisplayName = "Negative infinity")] - [DataRow(ClaimValueTypes.Double, "1e9999", DisplayName = "Exponent overflow")] + [DataRow(ClaimValueTypes.Integer, "1 or 1 eq 1", DisplayName = "Invalid primitive claim fails: malformed integer")] + [DataRow(ClaimValueTypes.Double, "NaN", DisplayName = "Invalid primitive claim fails: NaN")] + [DataRow(ClaimValueTypes.Double, "Infinity", DisplayName = "Invalid primitive claim fails: positive infinity")] + [DataRow(ClaimValueTypes.Double, "-Infinity", DisplayName = "Invalid primitive claim fails: negative infinity")] + [DataRow(ClaimValueTypes.Double, "1e9999", DisplayName = "Invalid primitive claim fails: exponent overflow")] public void DbPolicy_InvalidPrimitiveClaim_FailsClosed(string claimValueType, string claimValue) { RuntimeConfig runtimeConfig = InitRuntimeConfig( diff --git a/src/Service.Tests/CosmosTests/QueryFilterTests.cs b/src/Service.Tests/CosmosTests/QueryFilterTests.cs index 636988331c..eb4f4c0fcc 100644 --- a/src/Service.Tests/CosmosTests/QueryFilterTests.cs +++ b/src/Service.Tests/CosmosTests/QueryFilterTests.cs @@ -6,6 +6,7 @@ using System.Text.Json; using System.Threading.Tasks; using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.AuthenticationHelpers; using Azure.DataApiBuilder.Core.Resolvers; using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.Azure.Cosmos; @@ -977,6 +978,57 @@ public async Task TestQueryFilterFieldAuth_Only_AuthorizedArrayItem() ValidateResults(actual.GetProperty("items"), expected.RootElement, false); } + /// + /// Sends real authenticated GraphQL requests through the Cosmos DB policy pipeline and + /// verifies that percent-encoded quote syntax in a claim remains a bound query value. + /// + [TestMethod] + public async Task TestStringClaimWithEncodedQuotes_RemainsPolicyData() + { + JsonElement matchingRows = await ExecuteStringClaimPolicyQueryAsync("Mars"); + Assert.AreEqual(1, matchingRows.GetArrayLength()); + Assert.AreEqual("Mars", matchingRows[0].GetProperty("name").GetString()); + + JsonElement injectionAttemptRows = await ExecuteStringClaimPolicyQueryAsync( + "Mars%27 or 1 eq 1 or %27"); + Assert.AreEqual(0, injectionAttemptRows.GetArrayLength(), + "The encoded claim value must remain a bound value and must not become OData policy syntax."); + } + + /// + /// Executes a Planet query as a role whose read policy compares the name to the userId claim. + /// + private async Task ExecuteStringClaimPolicyQueryAsync(string userId) + { + const string clientRole = "claim_policy_tester"; + AppServiceClaim roleClaim = new() + { + Typ = ClaimTypes.Role, + Val = clientRole + }; + AppServiceClaim userIdClaim = new() + { + Typ = StaticWebAppsAuthentication.USER_ID_CLAIM, + Val = userId + }; + string gqlQuery = @"{ + planets(first: 10) { + items { + name + } + } + }"; + + JsonElement response = await ExecuteGraphQLRequestAsync( + queryName: _graphQLQueryName, + query: gqlQuery, + authToken: AuthTestHelper.CreateAppServiceEasyAuthToken( + additionalClaims: [roleClaim, userIdClaim]), + clientRoleHeader: clientRole); + + return response.GetProperty("items"); + } + #region Field Level Auth /// /// Tests that the field level query filter succeeds requests when filter fields are authorized diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index c216b4349c..a440bc4eca 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -3,12 +3,18 @@ using System.Linq; using System.Net; +using System.Net.Http; +using System.Security.Claims; +using System.Text.Json; using System.Threading.Tasks; using System.Web; using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Core.AuthenticationHelpers; +using Azure.DataApiBuilder.Core.Authorization; using Azure.DataApiBuilder.Core.Resolvers; using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.VisualStudio.TestTools.UnitTesting; +using static Azure.DataApiBuilder.Core.AuthenticationHelpers.AppServiceAuthentication; namespace Azure.DataApiBuilder.Service.Tests.SqlTests.RestApiTests.Find { @@ -112,6 +118,60 @@ await SetupAndRunRestApiTest( ); } + /// + /// Sends real authenticated REST requests through the database-policy pipeline and verifies + /// that a string claim is treated as data after URI processing. This inherited test runs + /// against MSSQL, PostgreSQL, MySQL, and DWSQL in their respective CI jobs. + /// + [TestMethod] + public async Task FindMany_StringClaimWithEncodedQuotes_RemainsPolicyData() + { + JsonElement matchingRows = await ExecuteStringClaimPolicyRequestAsync("Policy-Test-01"); + Assert.AreEqual(1, matchingRows.GetArrayLength()); + Assert.AreEqual(9, matchingRows[0].GetProperty("id").GetInt32()); + Assert.AreEqual("Policy-Test-01", matchingRows[0].GetProperty("title").GetString()); + + JsonElement injectionAttemptRows = await ExecuteStringClaimPolicyRequestAsync( + "Policy-Test-01%27 or 1 eq 1 or %27"); + Assert.AreEqual(0, injectionAttemptRows.GetArrayLength(), + "The encoded claim value must remain a bound value and must not become OData policy syntax."); + } + + /// + /// Executes a Book query as a role whose read policy compares the title to the userId claim. + /// + private static async Task ExecuteStringClaimPolicyRequestAsync(string userId) + { + const string clientRole = "claim_policy_tester"; + AppServiceClaim roleClaim = new() + { + Typ = AuthenticationOptions.ROLE_CLAIM_TYPE, + Val = clientRole + }; + AppServiceClaim userIdClaim = new() + { + Typ = StaticWebAppsAuthentication.USER_ID_CLAIM, + Val = userId + }; + + using HttpRequestMessage request = new(HttpMethod.Get, "api/Book?$select=id,title"); + request.Headers.Add(AuthorizationResolver.CLIENT_ROLE_HEADER, clientRole); + request.Headers.Add( + AuthenticationOptions.CLIENT_PRINCIPAL_HEADER, + AuthTestHelper.CreateAppServiceEasyAuthToken( + roleClaimType: AuthenticationOptions.ROLE_CLAIM_TYPE, + additionalClaims: [roleClaim, userIdClaim])); + + using HttpResponseMessage response = await HttpClient.SendAsync(request); + string responseBody = await response.Content.ReadAsStringAsync(); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode, responseBody); + + using JsonDocument responseDocument = JsonDocument.Parse(responseBody); + return responseDocument.RootElement + .GetProperty(SqlTestHelper.jsonResultTopLevelKey) + .Clone(); + } + /// /// Tests the REST API to validate that unique unicode /// characters work in queries. diff --git a/src/Service.Tests/dab-config.CosmosDb_NoSql.json b/src/Service.Tests/dab-config.CosmosDb_NoSql.json index c52c5e61c8..a34fe90cb9 100644 --- a/src/Service.Tests/dab-config.CosmosDb_NoSql.json +++ b/src/Service.Tests/dab-config.CosmosDb_NoSql.json @@ -118,6 +118,17 @@ "action": "read" } ] + }, + { + "role": "claim_policy_tester", + "actions": [ + { + "action": "read", + "policy": { + "database": "@item.name eq @claims.userId" + } + } + ] } ] }, diff --git a/src/Service.Tests/dab-config.DwSql.json b/src/Service.Tests/dab-config.DwSql.json index a5eec531b8..58cb15459b 100644 --- a/src/Service.Tests/dab-config.DwSql.json +++ b/src/Service.Tests/dab-config.DwSql.json @@ -819,6 +819,23 @@ } ] }, + { + "role": "claim_policy_tester", + "actions": [ + { + "action": "read", + "fields": { + "exclude": [], + "include": [ + "*" + ] + }, + "policy": { + "database": "@item.title eq @claims.userId" + } + } + ] + }, { "role": "test_role_with_noread", "actions": [ diff --git a/src/Service.Tests/dab-config.MsSql.json b/src/Service.Tests/dab-config.MsSql.json index 77bfd8b3fd..670f390d4c 100644 --- a/src/Service.Tests/dab-config.MsSql.json +++ b/src/Service.Tests/dab-config.MsSql.json @@ -875,6 +875,23 @@ } ] }, + { + "role": "claim_policy_tester", + "actions": [ + { + "action": "read", + "fields": { + "exclude": [], + "include": [ + "*" + ] + }, + "policy": { + "database": "@item.title eq @claims.userId" + } + } + ] + }, { "role": "test_role_with_noread", "actions": [ diff --git a/src/Service.Tests/dab-config.MySql.json b/src/Service.Tests/dab-config.MySql.json index d866369e1b..1ec164007d 100644 --- a/src/Service.Tests/dab-config.MySql.json +++ b/src/Service.Tests/dab-config.MySql.json @@ -738,6 +738,23 @@ } ] }, + { + "role": "claim_policy_tester", + "actions": [ + { + "action": "read", + "fields": { + "exclude": [], + "include": [ + "*" + ] + }, + "policy": { + "database": "@item.title eq @claims.userId" + } + } + ] + }, { "role": "test_role_with_noread", "actions": [ diff --git a/src/Service.Tests/dab-config.PostgreSql.json b/src/Service.Tests/dab-config.PostgreSql.json index 86bed1a352..7f79ba1784 100644 --- a/src/Service.Tests/dab-config.PostgreSql.json +++ b/src/Service.Tests/dab-config.PostgreSql.json @@ -774,6 +774,23 @@ } ] }, + { + "role": "claim_policy_tester", + "actions": [ + { + "action": "read", + "fields": { + "exclude": [], + "include": [ + "*" + ] + }, + "policy": { + "database": "@item.title eq @claims.userId" + } + } + ] + }, { "role": "test_role_with_noread", "actions": [ From ad7e688489b9292c8a24080de0b99f370a63cc36 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Fri, 7 Aug 2026 14:33:27 -0700 Subject: [PATCH 6/8] Fix policy claim snapshots and formatting --- ...estReadingRuntimeConfigForCosmos.verified.txt | 11 +++++++++++ ...TestReadingRuntimeConfigForMsSql.verified.txt | 16 ++++++++++++++++ ...TestReadingRuntimeConfigForMySql.verified.txt | 16 ++++++++++++++++ ...eadingRuntimeConfigForPostgreSql.verified.txt | 16 ++++++++++++++++ .../RestApiTests/Find/FindApiTestBase.cs | 1 - 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForCosmos.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForCosmos.verified.txt index 09c2586351..4738ac6a8d 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForCosmos.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForCosmos.verified.txt @@ -130,6 +130,17 @@ Action: Read } ] + }, + { + Role: claim_policy_tester, + Actions: [ + { + Action: Read, + Policy: { + Database: @item.name eq @claims.userId + } + } + ] } ] } diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt index d239ce2c26..3c3b8224e4 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt @@ -848,6 +848,22 @@ } ] }, + { + Role: claim_policy_tester, + Actions: [ + { + Action: Read, + Fields: { + Include: [ + * + ] + }, + Policy: { + Database: @item.title eq @claims.userId + } + } + ] + }, { Role: test_role_with_noread, Actions: [ diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt index 6b71ddd373..6d8ad1d11e 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt @@ -730,6 +730,22 @@ } ] }, + { + Role: claim_policy_tester, + Actions: [ + { + Action: Read, + Fields: { + Include: [ + * + ] + }, + Policy: { + Database: @item.title eq @claims.userId + } + } + ] + }, { Role: test_role_with_noread, Actions: [ diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt index a07b41b777..100ffd0214 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt @@ -749,6 +749,22 @@ } ] }, + { + Role: claim_policy_tester, + Actions: [ + { + Action: Read, + Fields: { + Include: [ + * + ] + }, + Policy: { + Database: @item.title eq @claims.userId + } + } + ] + }, { Role: test_role_with_noread, Actions: [ diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index a440bc4eca..8a4f2ced15 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Net; using System.Net.Http; -using System.Security.Claims; using System.Text.Json; using System.Threading.Tasks; using System.Web; From 2d563c3a8c1c0a2d570a1a6a5cca9d35a5369497 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Fri, 7 Aug 2026 15:59:55 -0700 Subject: [PATCH 7/8] Fix Cosmos policy parsing for model aliases --- src/Core/Parsers/EdmModelBuilder.cs | 17 ++++++++- .../CosmosTests/QueryFilterTests.cs | 2 + .../UnitTests/EdmModelBuilderTests.cs | 38 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/Core/Parsers/EdmModelBuilder.cs b/src/Core/Parsers/EdmModelBuilder.cs index 3c6c247dfd..4ca7a5c84a 100644 --- a/src/Core/Parsers/EdmModelBuilder.cs +++ b/src/Core/Parsers/EdmModelBuilder.cs @@ -4,6 +4,7 @@ using Azure.DataApiBuilder.Config.DatabasePrimitives; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Services; +using Azure.DataApiBuilder.Service.GraphQLBuilder; using HotChocolate.Language; using Microsoft.OData.Edm; @@ -54,7 +55,8 @@ private EdmModelBuilder BuildEdmModelsForCosmos(DocumentNode graphQLSchemaRoot) foreach (ObjectTypeDefinitionNode typeDefinition in graphQLSchemaRoot.Definitions) { - EdmEntityType edmEntity = new(DEFAULT_NAMESPACE, typeDefinition.Name.Value); + string graphQLTypeName = typeDefinition.Name.Value; + EdmEntityType edmEntity = new(DEFAULT_NAMESPACE, graphQLTypeName); foreach (FieldDefinitionNode field in typeDefinition.Fields) { edmEntity.AddStructuralProperty( @@ -64,8 +66,19 @@ private EdmModelBuilder BuildEdmModelsForCosmos(DocumentNode graphQLSchemaRoot) } container.AddEntitySet( - name: typeDefinition.Name.Value, + name: graphQLTypeName, elementType: edmEntity); + + // Cosmos database policies are resolved by the configured entity name, which can + // differ from the GraphQL type name through @model(name: "..."). Make both paths + // available to OData so policies on aliased root models can be parsed. + string entityName = GraphQLNaming.ObjectTypeToEntityName(typeDefinition); + if (!string.Equals(entityName, graphQLTypeName, StringComparison.Ordinal)) + { + container.AddEntitySet( + name: entityName, + elementType: edmEntity); + } } _model.AddElement(container); diff --git a/src/Service.Tests/CosmosTests/QueryFilterTests.cs b/src/Service.Tests/CosmosTests/QueryFilterTests.cs index eb4f4c0fcc..cb06247db9 100644 --- a/src/Service.Tests/CosmosTests/QueryFilterTests.cs +++ b/src/Service.Tests/CosmosTests/QueryFilterTests.cs @@ -1026,6 +1026,8 @@ private async Task ExecuteStringClaimPolicyQueryAsync(string userId additionalClaims: [roleClaim, userIdClaim]), clientRoleHeader: clientRole); + Assert.AreEqual(JsonValueKind.Object, response.ValueKind, + $"The GraphQL policy request failed: {response}"); return response.GetProperty("items"); } diff --git a/src/Service.Tests/UnitTests/EdmModelBuilderTests.cs b/src/Service.Tests/UnitTests/EdmModelBuilderTests.cs index c27c754b75..1a5b016b4c 100644 --- a/src/Service.Tests/UnitTests/EdmModelBuilderTests.cs +++ b/src/Service.Tests/UnitTests/EdmModelBuilderTests.cs @@ -7,7 +7,9 @@ using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Parsers; using Azure.DataApiBuilder.Core.Services; +using HotChocolate.Language; using Microsoft.OData.Edm; +using Microsoft.OData.UriParser; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -94,6 +96,42 @@ public void BuildModel_LinkingEntity_IsSkipped() "Linking entities should not be added to the EDM model."); } + [TestMethod] + public void BuildModel_CosmosModelAlias_AddsConfiguredEntitySet() + { + DocumentNode graphQLSchema = Utf8GraphQLParser.Parse( + """ + type Planet @model(name: "PlanetAlias") { + id: ID + name: String + } + """); + + IEdmModel model = new EdmModelBuilder() + .BuildModel(graphQLSchema) + .GetModel(); + + Assert.IsNotNull(model.EntityContainer); + Assert.IsTrue(model.EntityContainer.EntitySets().Any(entitySet => entitySet.Name == "Planet")); + Assert.IsTrue(model.EntityContainer.EntitySets().Any(entitySet => entitySet.Name == "PlanetAlias")); + + Dictionary claimValueNodes = new() + { + ["@dabClaim0"] = new ConstantNode("Mars") + }; + ODataParser parser = new(); + parser.BuildModel(graphQLSchema); + + FilterClause filterClause = parser.GetFilterClause( + filterQueryString: "?$filter=name eq @dabClaim0", + resourcePath: "PlanetAlias", + customResolver: new ClaimsTypeDataUriResolver(claimValueNodes), + parameterAliasNodes: claimValueNodes); + + BinaryOperatorNode comparison = (BinaryOperatorNode)filterClause.Expression; + Assert.AreEqual("Mars", ((ConstantNode)comparison.Right).Value); + } + #region Helpers private static SourceDefinition BuildSourceDefinition() From 5acf5f1661af0cb436aee073d9a713ad2e289761 Mon Sep 17 00:00:00 2001 From: Aaron Burtle Date: Fri, 7 Aug 2026 17:54:39 -0700 Subject: [PATCH 8/8] Fix Cosmos root policy path generation --- .../Resolvers/AuthorizationPolicyHelpers.cs | 15 ++++++++- .../DatabasePolicyClaimBindingUnitTests.cs | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index f2efc35cf9..408b0f30d4 100644 --- a/src/Core/Resolvers/AuthorizationPolicyHelpers.cs +++ b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs @@ -114,7 +114,7 @@ public static void ProcessAuthorizationPolicies( else { predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( - $"{pathConfig.Path}.{pathConfig.ColumnName}", + GetCosmosPolicyPathPrefix(pathConfig), cosmosQueryStructure)); } @@ -137,6 +137,19 @@ public static void ProcessAuthorizationPolicies( } } + /// + /// 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. + /// + /// The configured entity path. + /// The path prefix used by the Cosmos OData visitor. + internal static string GetCosmosPolicyPathPrefix(EntityDbPolicyCosmosModel pathConfig) + { + return string.IsNullOrEmpty(pathConfig.ColumnName) + ? pathConfig.Path + : $"{pathConfig.Path}.{pathConfig.ColumnName}"; + } + /// /// Read the DB policy from the config file and process it to generate OData Filter Clause. /// Here, we are processing the DB policy for each elemental operation and then calling the postProcessCallback. diff --git a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs index 70d6622177..fb423de2c7 100644 --- a/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -72,6 +72,38 @@ public void StringClaim_RemainsBoundParameterAcrossSqlAndCosmos(string claimValu AssertParameterValues(cosmosStructure, claimValue); } + /// + /// Verifies a policy on the root Cosmos model uses the container alias directly instead + /// of producing an invalid path with an empty column segment, such as c..textCol. + /// + [TestMethod] + public void CosmosRootPolicyPath_DoesNotAppendEmptyColumnSegment() + { + const string claimValue = "Mars"; + (AuthorizationResolver resolver, DefaultHttpContext context) = CreateAuthorizationContext( + "@item.textCol eq @claims.value", + new Claim("value", claimValue, ClaimValueTypes.String)); + Mock metadataProvider = CreateMetadataProvider(); + FilterClause filterClause = ResolveFilterClause(resolver, context, metadataProvider.Object); + TestQueryStructure cosmosStructure = new(metadataProvider.Object, resolver); + EntityDbPolicyCosmosModel rootPath = new( + Path: CosmosQueryStructure.COSMOSDB_CONTAINER_DEFAULT_ALIAS, + EntityName: "PlanetAlias"); + EntityDbPolicyCosmosModel nestedPath = new( + Path: CosmosQueryStructure.COSMOSDB_CONTAINER_DEFAULT_ALIAS, + EntityName: "Character", + ColumnName: "character"); + + string rootPrefix = AuthorizationPolicyHelpers.GetCosmosPolicyPathPrefix(rootPath); + string predicate = filterClause.Expression.Accept( + new ODataASTCosmosVisitor(rootPrefix, cosmosStructure)); + + Assert.AreEqual("c", rootPrefix); + Assert.AreEqual("c.character", AuthorizationPolicyHelpers.GetCosmosPolicyPathPrefix(nestedPath)); + Assert.AreEqual("(c.textCol = @param0)", predicate); + AssertParameterValues(cosmosStructure, claimValue); + } + /// /// Verifies aliases in root and unary Boolean positions are replaced throughout the AST /// and generate executable, parameterized predicates for both SQL and Cosmos DB.