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": [