Align authorization behavior to regular ASP.NET Core

src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs

@@ -0,0 +1,57 @@
+using System.Collections.Concurrent;
+using HotChocolate.Authorization;
+using Microsoft.AspNetCore.Authorization;
+
+namespace HotChocolate.AspNetCore.Authorization;
+
+internal sealed class AuthorizationPolicyCache(IAuthorizationPolicyProvider policyProvider)
+{
+    private readonly ConcurrentDictionary<string, Task<AuthorizationPolicy>> _cache = new();
+
+    public Task<AuthorizationPolicy> GetOrCreatePolicyAsync(AuthorizeDirective directive)
+    {
+        var cacheKey = directive.GetPolicyCacheKey();
+
+        return _cache.GetOrAdd(cacheKey, _ => BuildAuthorizationPolicy(directive.Policy, directive.Roles));
+    }
+
+    private async Task<AuthorizationPolicy> BuildAuthorizationPolicy(
+        string? policyName,
+        IReadOnlyList<string>? roles)
+    {
+        var policyBuilder = new AuthorizationPolicyBuilder();
+
+        if (!string.IsNullOrWhiteSpace(policyName))
+        {
+            var policy = await policyProvider.GetPolicyAsync(policyName).ConfigureAwait(false);
+
+            if (policy is not null)
+            {
+                policyBuilder = policyBuilder.Combine(policy);
+            }
+            else
+            {
+                throw new MissingAuthorizationPolicyException(policyName);
+            }
+        }
+        else
+        {
+            var defaultPolicy = await policyProvider.GetDefaultPolicyAsync().ConfigureAwait(false);
+
+            policyBuilder = policyBuilder.Combine(defaultPolicy);
+        }
+
+        if (roles is not null)
+        {
+            policyBuilder = policyBuilder.RequireRole(roles);
+        }
+
+        return policyBuilder.Build();
+    }
+}
+
+internal sealed class MissingAuthorizationPolicyException(string policyName)
+    : Exception($"The policy `{policyName}` does not exist.")
+{
+    public string PolicyName { get; } = policyName;
+}

src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/DefaultAuthorizationHandler.cs

@@ -12,29 +12,29 @@ namespace HotChocolate.AspNetCore.Authorization;
 internal sealed class DefaultAuthorizationHandler : IAuthorizationHandler
 {
     private readonly IAuthorizationService _authSvc;
-    private readonly IAuthorizationPolicyProvider _policyProvider;
+    private readonly AuthorizationPolicyCache _policyCache;
 
     /// <summary>
     /// Initializes a new instance <see cref="DefaultAuthorizationHandler"/>.
     /// </summary>
     /// <param name="authorizationService">
     /// The authorization service.
     /// </param>
-    /// <param name="authorizationPolicyProvider">
-    /// The authorization policy provider.
+    /// <param name="policyCache">
+    /// The authorization policy cache.
     /// </param>
     /// <exception cref="ArgumentNullException">
     /// <paramref name="authorizationService"/> is <c>null</c>.
-    /// <paramref name="authorizationPolicyProvider"/> is <c>null</c>.
+    /// <paramref name="policyCache"/> is <c>null</c>.
     /// </exception>
     public DefaultAuthorizationHandler(
         IAuthorizationService authorizationService,
-        IAuthorizationPolicyProvider authorizationPolicyProvider)
+        AuthorizationPolicyCache policyCache)
     {
         _authSvc = authorizationService ??
             throw new ArgumentNullException(nameof(authorizationService));
-        _policyProvider = authorizationPolicyProvider ??
-            throw new ArgumentNullException(nameof(authorizationPolicyProvider));
+        _policyCache = policyCache ??
+            throw new ArgumentNullException(nameof(policyCache));
     }
 
     /// <summary>
@@ -70,8 +70,7 @@ public async ValueTask<AuthorizeResult> AuthorizeAsync(
 
         return await AuthorizeAsync(
                 user,
-                directive.Policy,
-                directive.Roles,
+                directive,
                 authenticated,
                 context)
             .ConfigureAwait(false);
@@ -102,8 +101,7 @@ public async ValueTask<AuthorizeResult> AuthorizeAsync(
         {
             var result = await AuthorizeAsync(
                     user,
-                    directive.Policy,
-                    directive.Roles,
+                    directive,
                     authenticated,
                     context)
                 .ConfigureAwait(false);
@@ -119,62 +117,24 @@ public async ValueTask<AuthorizeResult> AuthorizeAsync(
 
     private async ValueTask<AuthorizeResult> AuthorizeAsync(
         ClaimsPrincipal user,
-        string? policyName,
-        IReadOnlyList<string>? roles,
+        AuthorizeDirective directive,
         bool authenticated,
         object context)
     {
-        var checkRoles = roles is { Count: > 0, };
-        var checkPolicy = !string.IsNullOrWhiteSpace(policyName);
-
-        // if the current directive has neither roles nor policies specified we will check if there
-        // is a default policy specified.
-        if (!checkRoles && !checkPolicy)
+        try
         {
-            var policy = await _policyProvider.GetDefaultPolicyAsync().ConfigureAwait(false);
+            var combinedPolicy = await _policyCache.GetOrCreatePolicyAsync(directive);
 
-            // if there is no default policy specified we will check if at least one of the
-            // identities are authenticated to authorize the user.
-            if (policy is null)
-            {
-                return authenticated
-                    ? AuthorizeResult.Allowed
-                    : AuthorizeResult.NoDefaultPolicy;
-            }
+            var result = await _authSvc.AuthorizeAsync(user, context, combinedPolicy).ConfigureAwait(false);
 
-            // if we find a default policy we will use this to authorize the access to a resource.
-            var result = await _authSvc.AuthorizeAsync(user, context, policy).ConfigureAwait(false);
             return result.Succeeded
                 ? AuthorizeResult.Allowed
                 : authenticated ? AuthorizeResult.NotAllowed : AuthorizeResult.NotAuthenticated;
         }
-
-        // We first check if the user fulfills any of the specified roles.
-        // If no role was specified the user fulfills them.
-        if (!checkRoles || FulfillsAnyRole(user, roles!))
+        catch (MissingAuthorizationPolicyException)
         {
-            if (!checkPolicy)
-            {
-                // The user fulfills one or all of the roles and no policy check was required.
-                return AuthorizeResult.Allowed;
-            }
-
-            // If a policy name was supplied we will try to resolve the policy
-            // and authorize with it.
-            var policy = await _policyProvider.GetPolicyAsync(policyName!).ConfigureAwait(false);
-
-            if (policy is null)
-            {
-                return AuthorizeResult.PolicyNotFound;
-            }
-
-            var result = await _authSvc.AuthorizeAsync(user, context, policy).ConfigureAwait(false);
-            return result.Succeeded
-                ? AuthorizeResult.Allowed
-                : authenticated ? AuthorizeResult.NotAllowed : AuthorizeResult.NotAuthenticated;
+            return AuthorizeResult.PolicyNotFound;
         }
-
-        return authenticated ? AuthorizeResult.NotAllowed : AuthorizeResult.NotAuthenticated;
     }
 
     private static UserState GetUserState(IDictionary<string, object?> contextData)
@@ -193,17 +153,4 @@ private static UserState GetUserState(IDictionary<string, object?> contextData)
 
     private static void SetUserState(IDictionary<string, object?> contextData, UserState state)
         => contextData[WellKnownContextData.UserState] = state;
-
-    private static bool FulfillsAnyRole(ClaimsPrincipal principal, IReadOnlyList<string> roles)
-    {
-        for (var i = 0; i < roles.Count; i++)
-        {
-            if (principal.IsInRole(roles[i]))
-            {
-                return true;
-            }
-        }
-
-        return false;
-    }
 }

src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/Extensions/HotChocolateAuthorizeRequestExecutorBuilder.cs

@@ -1,5 +1,6 @@
 using HotChocolate.AspNetCore.Authorization;
 using HotChocolate.Execution.Configuration;
+using Microsoft.Extensions.DependencyInjection.Extensions;
 
 namespace Microsoft.Extensions.DependencyInjection;
 
@@ -27,7 +28,7 @@ public static IRequestExecutorBuilder AddAuthorization(
         }
 
         builder.Services.AddAuthorization();
-        builder.AddAuthorizationHandler<DefaultAuthorizationHandler>();
+        builder.AddAuthorizationServices();
         return builder;
     }
 
@@ -60,7 +61,13 @@ public static IRequestExecutorBuilder AddAuthorization(
         }
 
         builder.Services.AddAuthorization(configure);
-        builder.AddAuthorizationHandler<DefaultAuthorizationHandler>();
+        builder.AddAuthorizationServices();
         return builder;
     }
+
+    private static void AddAuthorizationServices(this IRequestExecutorBuilder builder)
+    {
+        builder.Services.TryAddSingleton<AuthorizationPolicyCache>();
+        builder.AddAuthorizationHandler<DefaultAuthorizationHandler>();
+    }
 }

src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationAttributeTestData.cs

@@ -22,6 +22,10 @@ public class Query
         [GraphQLName("roles_ab")]
         public string? GetRolesAb() => "foo";
 
+        [Authorize(ApplyPolicy.BeforeResolver, Roles = ["a", "b"], Policy = "HasAgeDefined")]
+        [GraphQLName("rolesAndPolicy")]
+        public string? GetRolesAndPolicy() => "foo";
+
         [Authorize(ApplyPolicy.BeforeResolver, Policy = "a")]
         [Authorize(ApplyPolicy.BeforeResolver, Policy = "b")]
         public string? GetPiped() => "foo";

src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationTestData.cs

@@ -13,6 +13,7 @@ type Query {
             age: String @authorize(policy: ""HasAgeDefined"" apply: BEFORE_RESOLVER)
             roles: String @authorize(roles: [""a""] apply: BEFORE_RESOLVER)
             roles_ab: String @authorize(roles: [""a"" ""b""] apply: BEFORE_RESOLVER)
+            rolesAndPolicy: String @authorize(roles: [""a"" ""b""] policy: ""HasAgeDefined"" apply: BEFORE_RESOLVER)
             piped: String
                 @authorize(policy: ""a"" apply: BEFORE_RESOLVER)
                 @authorize(policy: ""b"" apply: BEFORE_RESOLVER)