From adc61b221a8ea3cae5853c246b82d0d5ac2735a9 Mon Sep 17 00:00:00 2001 From: aaronburtle <93220300+aaronburtle@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:24:00 +0000 Subject: [PATCH] Bind database policy claims as typed OData constants (#3756) ## Why make this change? Closes https://github.com/Azure/data-api-builder/issues/3755 ## What is this change? - Replaces claim references in database policies with inert OData parameter aliases. - Keeps claim values separate from policy text in a `ResolvedDatabasePolicy`. - Injects claim values into the parsed OData AST as typed `ConstantNode` values before operand type promotion. - Converts supported primitive claim types to CLR values using invariant parsing and fails closed when a claim does not match its declared type. - Continues using database parameters for SQL predicates and now parameterizes Cosmos DB policy constants instead of inlining them. - Preserves legitimate claim values containing apostrophes, percent characters, or encoded text without decoding or rewriting them. Database policy resolution now returns a `ResolvedDatabasePolicy` rather than a string. This keeps trusted OData policy syntax separate from untrusted claim values: the policy contains inert aliases, while typed values are carried independently and bound during OData AST processing. This prevents URI decoding or escaping behavior from turning claim data into policy syntax. Relevant specification: - [OData 4.01 URL Conventions: Parameter Aliases](https://docs.oasis-open.org/odata/odata/v4.01/cs02/part2-url-conventions/odata-v4.01-cs02-part2-url-conventions.html#sec_ParameterAliases) ```mermaid flowchart TD A["Trusted configured policy
@item.ownerId eq @claims.userId"] B["Untrusted authenticated claim
alice%27 or 1 eq 1 or %27"] A --> C["AuthorizationResolver"] B --> C C --> D["ResolvedDatabasePolicy"] D --> E["Policy:
ownerId eq @dabClaim0"] D --> F["ClaimValues:
@dabClaim0 maps to raw CLR string"] E --> G["ODataParser"] F --> H["ConstantNode map"] H --> G G --> I["ClaimsTypeDataUriResolver
Resolves aliases before type promotion"] I --> J["ParameterAliasRewriter
Resolves remaining aliases and Boolean contexts"] J --> K["Typed FilterClause AST"] K --> L["ODataASTVisitor"] K --> M["ODataASTCosmosVisitor"] L --> N["SQL predicate and provider parameters"] M --> O["Cosmos SQL predicate and provider parameters"] ``` ## How was this tested? - [x] Integration Tests - [x] Unit Tests Focused unit-test coverage includes: - Literal apostrophes in string claims. - Percent-encoded text. - Double-encoded and mixed-encoded text. - Legitimate percent characters. - Typed boolean, integer, floating-point, and null claims. - Malformed primitive claims failing closed. - Cosmos DB policy constants being emitted as query parameters. Integration tests use an authenticated REST test in FindApiTestBase, so it will run against MSSQL, PGSQL, MySQL, and DWSQL, along with a cosmos specific authenticated GQL integration test. ## Sample Request(s) No client-facing request contract changes are introduced. Example database policy: @item.ownerId eq @claims.userId Example request: GET /api/Note Authorization: Bearer X-MS-API-ROLE: authenticated A legitimate claim such as `O'Brien` or `50% complete` is preserved exactly and bound as a database parameter. It is never inserted into or reinterpreted as OData policy syntax. --- 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/IAuthorizationResolver.cs | 9 +- src/Auth/ResolvedDatabasePolicy.cs | 46 ++ .../Authorization/AuthorizationResolver.cs | 147 ++++-- src/Core/Parsers/ClaimsTypeDataUriResolver.cs | 40 +- src/Core/Parsers/EdmModelBuilder.cs | 17 +- src/Core/Parsers/FilterParser.cs | 20 +- src/Core/Parsers/ODataASTCosmosVisitor.cs | 8 +- src/Core/Parsers/ParameterAliasRewriter.cs | 136 +++++ .../Resolvers/AuthorizationPolicyHelpers.cs | 48 +- src/Core/Resolvers/CosmosQueryStructure.cs | 8 - .../AuthorizationResolverUnitTests.cs | 130 +++-- .../REST/RestAuthorizationHandlerUnitTests.cs | 7 +- .../DabCacheServiceIntegrationTests.cs | 14 + .../CosmosTests/QueryFilterTests.cs | 54 ++ ...ReadingRuntimeConfigForCosmos.verified.txt | 11 + ...tReadingRuntimeConfigForMsSql.verified.txt | 16 + ...tReadingRuntimeConfigForMySql.verified.txt | 16 + ...ingRuntimeConfigForPostgreSql.verified.txt | 16 + .../RestApiTests/Find/FindApiTestBase.cs | 59 +++ .../DatabasePolicyClaimBindingUnitTests.cs | 482 ++++++++++++++++++ .../UnitTests/DwSqlQueryBuilderUpsertTests.cs | 11 +- .../UnitTests/EdmModelBuilderTests.cs | 38 ++ .../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 + 32 files changed, 1285 insertions(+), 132 deletions(-) create mode 100644 src/Auth/ResolvedDatabasePolicy.cs create mode 100644 src/Core/Parsers/ParameterAliasRewriter.cs create mode 100644 src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs 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 bbf3251fd8..c6ed745768 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/IAuthorizationResolver.cs b/src/Auth/IAuthorizationResolver.cs index 3a961ece4d..c990611f9a 100644 --- a/src/Auth/IAuthorizationResolver.cs +++ b/src/Auth/IAuthorizationResolver.cs @@ -72,16 +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 successfully processed, or an exception otherwise. - 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/ResolvedDatabasePolicy.cs b/src/Auth/ResolvedDatabasePolicy.cs new file mode 100644 index 0000000000..08ad51edd9 --- /dev/null +++ b/src/Auth/ResolvedDatabasePolicy.cs @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.ObjectModel; + +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. +/// +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; } + + /// + /// 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. + /// + /// 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)); + } +} diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 205dc3d646..fd0da59393 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 ResolveDBPolicy(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,83 @@ 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 ParseFiniteDoubleClaimValue(claim.Value); + 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); + } + } + + /// + /// 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) + { + 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: message, + 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..f8f213dfa2 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,19 +34,29 @@ 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) { - if (leftNode.TypeReference.PrimitiveKind() != rightNode.TypeReference.PrimitiveKind()) + ResolveClaimAlias(ref leftNode); + ResolveClaimAlias(ref rightNode); + + 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 ); } @@ -48,6 +65,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/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/Core/Parsers/FilterParser.cs b/src/Core/Parsers/FilterParser.cs index c9cfc1eb53..e79e559ee5 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,7 +65,18 @@ public FilterClause GetFilterClause(string filterQueryString, string resourcePat parser.Resolver = customResolver; } - return parser.ParseFilter(); + if (parameterAliasNodes is { Count: > 0 }) + { + foreach ((string alias, SingleValueNode valueNode) in parameterAliasNodes) + { + parser.ParameterAliasNodes.Add(alias, valueNode); + } + } + + FilterClause filterClause = parser.ParseFilter(); + return parameterAliasNodes is not { Count: > 0 } + ? filterClause + : new ParameterAliasRewriter(parameterAliasNodes).Rewrite(filterClause); } catch (ODataException e) { diff --git a/src/Core/Parsers/ODataASTCosmosVisitor.cs b/src/Core/Parsers/ODataASTCosmosVisitor.cs index 1fca34d624..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,14 +13,17 @@ namespace Azure.DataApiBuilder.Core.Parsers internal class ODataASTCosmosVisitor : QueryNodeVisitor { private string _prefix; + 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 /// /// - public ODataASTCosmosVisitor(string prefix) + /// Stores bound Cosmos DB query parameters. + public ODataASTCosmosVisitor(string prefix, BaseQueryStructure queryStructure) { this._prefix = prefix; + _queryStructure = queryStructure; } /// @@ -123,7 +127,7 @@ public override string Visit(ConstantNode nodeIn) return "NULL"; } - return $"'{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..3aef5624ab --- /dev/null +++ b/src/Core/Parsers/ParameterAliasRewriter.cs @@ -0,0 +1,136 @@ +// 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; + + /// + /// 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; + } + + /// + /// 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); + } + + /// + /// 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 + { + 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.") + }; + } + + /// + /// 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; + return new BinaryOperatorNode( + node.OperatorKind, + RewriteNode(node.Left, operandsArePredicates), + 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)) + { + throw new ODataException($"No value was supplied for database policy parameter alias '{aliasNode.Alias}'."); + } + + 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 || + node.TypeReference?.PrimitiveKind() is not EdmPrimitiveTypeKind.Boolean || + IsPredicateExpression(node)) + { + return node; + } + + return new BinaryOperatorNode( + BinaryOperatorKind.Equal, + 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/Core/Resolvers/AuthorizationPolicyHelpers.cs b/src/Core/Resolvers/AuthorizationPolicyHelpers.cs index 58f4d1461d..408b0f30d4 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, + cosmosQueryStructure)); existQuery = CosmosQueryBuilder.BuildExistsQueryForCosmos(fromClause, predicates); } else { - predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor($"{pathConfig.Path}.{pathConfig.ColumnName}")); + predicates = filterClause?.Expression.Accept(new ODataASTCosmosVisitor( + GetCosmosPolicyPathPrefix(pathConfig), + cosmosQueryStructure)); } if (pathConfig.EntityName == entity.Key) @@ -133,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. @@ -160,11 +177,11 @@ private static List ProcessFilter( List filterClauses = new(); foreach (EntityActionOperation elementalOperation in elementalOperations) { - string dbQueryPolicy = authorizationResolver.ProcessDBPolicy( - entityName, - clientRoleHeader, - elementalOperation, - context); + ResolvedDatabasePolicy dbQueryPolicy = authorizationResolver.ResolveDBPolicy( + entityName, + clientRoleHeader, + elementalOperation, + context); FilterClause? filterClause = GetDBPolicyClauseForQueryStructure( dbQueryPolicy, @@ -179,31 +196,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/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 a40839f8e8..1ab41a7c1a 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.ResolveDBPolicy(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.ResolveDBPolicy(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.ResolveDBPolicy(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,42 @@ 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. + /// + [DataTestMethod] + [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( + 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", 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.ResolveDBPolicy(TEST_ENTITY, TEST_ROLE, TEST_OPERATION, context.Object)); + + 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); + } + /// /// Test to validate that we are correctly throwing an appropriate exception when the user request /// lacks a claim required by the policy. @@ -1479,7 +1517,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) { @@ -1531,16 +1569,18 @@ 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.ResolveDBPolicy(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: // 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, @@ -1583,15 +1623,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); - 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 d6e7d55bfe..8db19814d0 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -99,12 +99,13 @@ public void TestWildcardPolicyResolvesToEmpty(string httpMethod) AuthorizationResolver authorizationResolver = SetupAuthResolverWithWildcardOperation(); HttpContext httpContext = CreateHttpContext(httpMethod: httpMethod, clientRole: "admin"); - 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) - ); + 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/CosmosTests/QueryFilterTests.cs b/src/Service.Tests/CosmosTests/QueryFilterTests.cs index 636988331c..cb06247db9 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,59 @@ 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); + + Assert.AreEqual(JsonValueKind.Object, response.ValueKind, + $"The GraphQL policy request failed: {response}"); + 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/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 c216b4349c..8a4f2ced15 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -3,12 +3,17 @@ using System.Linq; using System.Net; +using System.Net.Http; +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 +117,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/UnitTests/DatabasePolicyClaimBindingUnitTests.cs b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs new file mode 100644 index 0000000000..fb423de2c7 --- /dev/null +++ b/src/Service.Tests/UnitTests/DatabasePolicyClaimBindingUnitTests.cs @@ -0,0 +1,482 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Data; +using System.Linq; +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 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 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 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")] + [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 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 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. + /// + [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 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 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() + { + User = new ClaimsPrincipal(identity) + }; + context.Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER] = ROLE_NAME; + + 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() + { + [ENTITY_NAME] = databaseObject + }; + + 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) + { + } + } + + private sealed class TestQueryStructure : BaseQueryStructure + { + 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 3d70162832..ac5de6109a 100644 --- a/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs +++ b/src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs @@ -139,12 +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 the resolver only needs - // to return an empty policy (no throw) during construction. + // The update policy is injected directly onto the structure after construction. Mock authorizationResolver = new(); authorizationResolver - .Setup(x => x.ProcessDBPolicy(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(string.Empty); + .Setup(x => x.ResolveDBPolicy( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(ResolvedDatabasePolicy.Empty); RuntimeConfigProvider runtimeConfigProvider = TestHelper.GetRuntimeConfigProvider(TestHelper.GetRuntimeConfigLoader()); Mock metadataProviderFactory = new(); 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() 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 fd4a34084a..579cecaf29 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": [