From 100ab612d885c900e84f33a1a6198151852acee8 Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Thu, 22 May 2025 00:06:42 +0900 Subject: [PATCH 1/6] Added metadata to support IAuthorizationRequirementData --- .../DefaultAuthorizationHandler.cs | 23 +++++++++++++++++-- .../src/Authorization/AuthorizeDirective.cs | 12 +++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/DefaultAuthorizationHandler.cs b/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/DefaultAuthorizationHandler.cs index bcc15389477..c1e7fc9bc05 100644 --- a/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/DefaultAuthorizationHandler.cs +++ b/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/DefaultAuthorizationHandler.cs @@ -142,7 +142,10 @@ private async ValueTask AuthorizeAsync( if (authorizationPolicy is null) { - authorizationPolicy = await BuildAuthorizationPolicy(directive.Policy, directive.Roles); + authorizationPolicy = await BuildAuthorizationPolicy( + directive.Policy, + directive.Roles, + directive.Metadata); if (_canCachePolicies) { @@ -164,7 +167,8 @@ private async ValueTask AuthorizeAsync( private async Task BuildAuthorizationPolicy( string? policyName, - IReadOnlyList? roles) + IReadOnlyList? roles, + IReadOnlyList? metadata) { var policyBuilder = new AuthorizationPolicyBuilder(); @@ -193,6 +197,21 @@ private async Task BuildAuthorizationPolicy( policyBuilder = policyBuilder.RequireRole(roles); } + var requirementData = metadata?.OfType()?.ToList() ?? []; + if (requirementData.Count > 0) + { + var reqPolicy = new AuthorizationPolicyBuilder(); + foreach (var rd in requirementData) + { + foreach (var r in rd.GetRequirements()) + { + reqPolicy.AddRequirements(r); + } + } + + policyBuilder = policyBuilder.Combine(reqPolicy.Build()); + } + return policyBuilder.Build(); } diff --git a/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs b/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs index b924db2d0ea..ab71c6293bc 100644 --- a/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs +++ b/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs @@ -44,14 +44,19 @@ public AuthorizeDirective(ApplyPolicy apply) /// /// Specifies when the authorization directive shall be applied. /// + /// + /// Metadata that can be used for authorization. + /// public AuthorizeDirective( string? policy = null, IReadOnlyList? roles = null, - ApplyPolicy apply = ApplyPolicy.BeforeResolver) + ApplyPolicy apply = ApplyPolicy.BeforeResolver, + IReadOnlyList? metadata = null) { Policy = policy; Roles = roles?.OrderBy(r => r).ToList(); Apply = apply; + Metadata = metadata; _cacheKey = BuildCacheKey(Policy, Roles); } @@ -79,6 +84,11 @@ public AuthorizeDirective( /// public ApplyPolicy Apply { get; } + /// + /// Gets the metadata that can be used for authorization. + /// + public IReadOnlyList? Metadata { get; } + /// /// Gets a cache key that uniquely identifies the combined authorization policy, /// of the specified and . From 73293bd1600899eae96eb2d73fb862e4aba2b78a Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Thu, 22 May 2025 10:29:04 +0900 Subject: [PATCH 2/6] Add tests --- .../AuthorizationRequirementDataTests.cs | 154 ++++++++++++++++++ ...ionRequirementDataTests.NotAuthorized.snap | 25 +++ 2 files changed, 179 insertions(+) create mode 100644 src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs create mode 100644 src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.NotAuthorized.snap diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs new file mode 100644 index 00000000000..f64426fc93f --- /dev/null +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs @@ -0,0 +1,154 @@ +using System.Net; +using System.Reflection; +using System.Security.Claims; +using HotChocolate.AspNetCore.Tests.Utilities; +using HotChocolate.Execution.Configuration; +using HotChocolate.Types; +using HotChocolate.Types.Descriptors; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; + +namespace HotChocolate.AspNetCore.Authorization; + +public class AuthorizationRequirementDataTests(TestServerFactory serverFactory) : ServerTestBase(serverFactory) +{ + [Fact] + public async Task Authorized() + { + // arrange + var server = CreateTestServer( + builder => + { + builder.Services.AddTransient(); + builder + .AddQueryType() + .AddAuthorization(); + }, + context => + { + var identity = new ClaimsIdentity("testauth"); + identity.AddClaim(new Claim( + "foo", + "bar")); + context.User = new ClaimsPrincipal(identity); + }); + + // act + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }", }); + + // assert + Assert.Equal(HttpStatusCode.OK, result.StatusCode); + Assert.Null(result.Errors); + } + + [Fact] + public async Task NotAuthorized() + { + // arrange + var server = CreateTestServer( + builder => + { + builder.Services.AddTransient(); + builder + .AddQueryType() + .AddAuthorization(); + }, + context => + { + var identity = new ClaimsIdentity("testauth"); + identity.AddClaim(new Claim( + "foo", + "foo")); + context.User = new ClaimsPrincipal(identity); + }); + + // act + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }", }); + + // assert + Assert.Equal(HttpStatusCode.OK, result.StatusCode); + result.MatchSnapshot(); + } + + public class Query + { + [CustomAuthorize("foo", "bar", Apply = HotChocolate.Authorization.ApplyPolicy.BeforeResolver)] + public string? GetFoo() => "foo"; + } + + private class CustomAuthorizeAttribute(string type, string value) + : HotChocolate.Authorization.AuthorizeAttribute, + IAuthorizationRequirement, + IAuthorizationRequirementData + { + public string Type => type; + + public string Value => value; + + public IEnumerable GetRequirements() + { + yield return this; + } + + protected internal override void TryConfigure( + IDescriptorContext context, + IDescriptor descriptor, + ICustomAttributeProvider element) + { + if (descriptor is IObjectTypeDescriptor type) + { + type.Directive(CreateDirective()); + } + else if (descriptor is IObjectFieldDescriptor field) + { + field.Directive(CreateDirective()); + } + } + + private HotChocolate.Authorization.AuthorizeDirective CreateDirective() + { + return new HotChocolate.Authorization.AuthorizeDirective(metadata: [.. GetRequirements()]); + } + } + + private class CustomAuthorizationHandler : AuthorizationHandler + { + protected override Task HandleRequirementAsync( + AuthorizationHandlerContext context, + CustomAuthorizeAttribute requirement) + { + if (context.User.HasClaim(requirement.Type, requirement.Value)) + { + context.Succeed(requirement); + } + return Task.CompletedTask; + } + } + + private TestServer CreateTestServer( + Action build, + Action configureUser) + { + return ServerFactory.Create( + services => + { + build(services + .AddRouting() + .AddGraphQLServer() + .AddHttpRequestInterceptor( + (context, requestExecutor, requestBuilder, cancellationToken) => + { + configureUser(context); + return default; + })); + }, + app => + { + app.UseRouting(); + app.UseEndpoints(b => b.MapGraphQL()); + }); + } +} diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.NotAuthorized.snap b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.NotAuthorized.snap new file mode 100644 index 00000000000..2e3e1db2f09 --- /dev/null +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.NotAuthorized.snap @@ -0,0 +1,25 @@ +{ + "ContentType": "application/graphql-response+json; charset=utf-8", + "StatusCode": "OK", + "Data": { + "foo": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "locations": [ + { + "line": 1, + "column": 3 + } + ], + "path": [ + "foo" + ], + "extensions": { + "code": "AUTH_NOT_AUTHORIZED" + } + } + ], + "Extensions": null +} From 17c96a83a7b8d5232853ede5780d2fcd7505d380 Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Thu, 22 May 2025 10:55:20 +0900 Subject: [PATCH 3/6] Add more tests --- .../AuthorizationRequirementDataTests.cs | 68 +++++++++++++++++++ ...ementDataTests.Multiple_NotAuthorized.snap | 25 +++++++ 2 files changed, 93 insertions(+) create mode 100644 src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.Multiple_NotAuthorized.snap diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs index f64426fc93f..77f8be003d5 100644 --- a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs @@ -73,10 +73,78 @@ public async Task NotAuthorized() result.MatchSnapshot(); } + [Fact] + public async Task Multiple_Authorized() + { + // arrange + var server = CreateTestServer( + builder => + { + builder.Services.AddTransient(); + builder + .AddQueryType() + .AddAuthorization(); + }, + context => + { + var identity = new ClaimsIdentity("testauth"); + identity.AddClaim(new Claim( + "foo", + "bar")); + identity.AddClaim(new Claim( + "bar", + "baz")); + context.User = new ClaimsPrincipal(identity); + }); + + // act + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }", }); + + // assert + Assert.Equal(HttpStatusCode.OK, result.StatusCode); + Assert.Null(result.Errors); + } + + [Fact] + public async Task Multiple_NotAuthorized() + { + // arrange + var server = CreateTestServer( + builder => + { + builder.Services.AddTransient(); + builder + .AddQueryType() + .AddAuthorization(); + }, + context => + { + var identity = new ClaimsIdentity("testauth"); + identity.AddClaim(new Claim( + "foo", + "bar")); + identity.AddClaim(new Claim( + "bar", + "bar")); + context.User = new ClaimsPrincipal(identity); + }); + + // act + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }", }); + + // assert + Assert.Equal(HttpStatusCode.OK, result.StatusCode); + result.MatchSnapshot(); + } + public class Query { [CustomAuthorize("foo", "bar", Apply = HotChocolate.Authorization.ApplyPolicy.BeforeResolver)] public string? GetFoo() => "foo"; + + [CustomAuthorize("foo", "bar", Apply = HotChocolate.Authorization.ApplyPolicy.BeforeResolver)] + [CustomAuthorize("bar", "baz", Apply = HotChocolate.Authorization.ApplyPolicy.BeforeResolver)] + public string? GetFooMultiple() => "foo"; } private class CustomAuthorizeAttribute(string type, string value) diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.Multiple_NotAuthorized.snap b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.Multiple_NotAuthorized.snap new file mode 100644 index 00000000000..59241c099f5 --- /dev/null +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/__snapshots__/AuthorizationRequirementDataTests.Multiple_NotAuthorized.snap @@ -0,0 +1,25 @@ +{ + "ContentType": "application/graphql-response+json; charset=utf-8", + "StatusCode": "OK", + "Data": { + "fooMultiple": null + }, + "Errors": [ + { + "message": "The current user is not authorized to access this resource.", + "locations": [ + { + "line": 1, + "column": 3 + } + ], + "path": [ + "fooMultiple" + ], + "extensions": { + "code": "AUTH_NOT_AUTHORIZED" + } + } + ], + "Extensions": null +} From ff4d05038c7aa5538e997a66b57fc760de347d68 Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Thu, 22 May 2025 10:59:31 +0900 Subject: [PATCH 4/6] Fixed policy cache to handle metadata --- .../AuthorizationPolicyCache.cs | 10 +- .../AuthorizeDirectiveTests.cs | 99 ------------------- .../src/Authorization/AuthorizeDirective.cs | 44 --------- 3 files changed, 3 insertions(+), 150 deletions(-) diff --git a/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs b/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs index 591c326aa1b..b8f3e2fa897 100644 --- a/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs +++ b/src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs @@ -6,19 +6,15 @@ namespace HotChocolate.AspNetCore.Authorization; internal sealed class AuthorizationPolicyCache { - private readonly ConcurrentDictionary _cache = new(); + private readonly ConcurrentDictionary _cache = new(); public AuthorizationPolicy? LookupPolicy(AuthorizeDirective directive) { - var cacheKey = directive.GetPolicyCacheKey(); - - return _cache.GetValueOrDefault(cacheKey); + return _cache.GetValueOrDefault(directive); } public void CachePolicy(AuthorizeDirective directive, AuthorizationPolicy policy) { - var cacheKey = directive.GetPolicyCacheKey(); - - _cache.TryAdd(cacheKey, policy); + _cache.TryAdd(directive, policy); } } diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizeDirectiveTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizeDirectiveTests.cs index 40bf76bfde4..ad8049d4ebf 100644 --- a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizeDirectiveTests.cs +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizeDirectiveTests.cs @@ -80,105 +80,6 @@ public void CreateInstance_Roles_RolesHasItems() t => Assert.Equal("b", t)); } - [Fact] - public void CacheKey_Policy_NoRoles() - { - // arrange - var authorizeDirective = new AuthorizeDirective( - policy: "policy"); - - // act - var cacheKey = authorizeDirective.GetPolicyCacheKey(); - - // assert - Assert.Equal("policy;", cacheKey); - } - - [Fact] - public void CacheKey_NoPolicy_Roles() - { - // arrange - var authorizeDirective = new AuthorizeDirective( - policy: null, - roles: ["a", "b"]); - - // act - var cacheKey = authorizeDirective.GetPolicyCacheKey(); - - // assert - Assert.Equal(";a,b", cacheKey); - } - - [Fact] - public void CacheKey_Policy_And_Roles() - { - // arrange - var authorizeDirective = new AuthorizeDirective( - policy: "policy", - roles: ["a", "b"]); - - // act - var cacheKey = authorizeDirective.GetPolicyCacheKey(); - - // assert - Assert.Equal("policy;a,b", cacheKey); - } - - [Fact] - public void CacheKey_NoPolicy_NoRoles() - { - // arrange - var authorizeDirective = new AuthorizeDirective( - policy: null, - roles: null); - - // act - var cacheKey = authorizeDirective.GetPolicyCacheKey(); - - // assert - Assert.Equal("", cacheKey); - } - - [Fact] - public void CacheKey_Policy_And_Role_Naming_Does_Not_Conflict() - { - // arrange - var authorizeDirective1 = new AuthorizeDirective( - policy: "policy", - roles: null); - - var authorizeDirective2 = new AuthorizeDirective( - policy: null, - roles: ["policy"]); - - // act - var cacheKey1 = authorizeDirective1.GetPolicyCacheKey(); - var cacheKey2 = authorizeDirective2.GetPolicyCacheKey(); - - // assert - Assert.NotEqual(cacheKey1, cacheKey2); - } - - [Fact] - public void CacheKey_Same_Roles_Albeit_Sorted_Differently_Have_Same_Cache_Key() - { - // arrange - var authorizeDirective1 = new AuthorizeDirective( - policy: null, - roles: ["a", "c", "b"]); - - var authorizeDirective2 = new AuthorizeDirective( - policy: null, - roles: ["c", "b", "a"]); - - // act - var cacheKey1 = authorizeDirective1.GetPolicyCacheKey(); - var cacheKey2 = authorizeDirective2.GetPolicyCacheKey(); - - // assert - Assert.Equal(cacheKey1, cacheKey2); - } - [Fact] public void TypeAuth_DefaultPolicy() { diff --git a/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs b/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs index ab71c6293bc..8864219ae76 100644 --- a/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs +++ b/src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs @@ -1,5 +1,3 @@ -using System.Text; - namespace HotChocolate.Authorization; /// @@ -7,8 +5,6 @@ namespace HotChocolate.Authorization; /// public sealed class AuthorizeDirective { - private readonly string _cacheKey; - /// /// Initializes a new instance of . /// @@ -57,8 +53,6 @@ public AuthorizeDirective( Roles = roles?.OrderBy(r => r).ToList(); Apply = apply; Metadata = metadata; - - _cacheKey = BuildCacheKey(Policy, Roles); } /// @@ -88,42 +82,4 @@ public AuthorizeDirective( /// Gets the metadata that can be used for authorization. /// public IReadOnlyList? Metadata { get; } - - /// - /// Gets a cache key that uniquely identifies the combined authorization policy, - /// of the specified and . - /// - internal string GetPolicyCacheKey() => _cacheKey; - - private static string BuildCacheKey(string? policy, IReadOnlyList? roles) - { - if (string.IsNullOrEmpty(policy) && roles is null) - { - return string.Empty; - } - - var sb = new StringBuilder(); - - if (!string.IsNullOrEmpty(policy)) - { - sb.Append(policy); - } - - sb.Append(";"); - - if (roles is not null) - { - for (var i = 0; i < roles.Count; i++) - { - sb.Append(roles[i]); - - if (i < roles.Count - 1) - { - sb.Append(","); - } - } - } - - return sb.ToString(); - } } From 7db173c16ff09878052846dfd9b27e911ecf17c3 Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Thu, 22 May 2025 12:50:20 +0900 Subject: [PATCH 5/6] add req-data instead of reqs --- .../AuthorizationRequirementDataTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs index 77f8be003d5..1265267391c 100644 --- a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs @@ -178,7 +178,7 @@ protected internal override void TryConfigure( private HotChocolate.Authorization.AuthorizeDirective CreateDirective() { - return new HotChocolate.Authorization.AuthorizeDirective(metadata: [.. GetRequirements()]); + return new HotChocolate.Authorization.AuthorizeDirective(metadata: [this]); } } From 1efcdb7a787cc8c652af4cbb326b01e98d1351ec Mon Sep 17 00:00:00 2001 From: Sunghwan Bang Date: Sun, 8 Jun 2025 16:42:24 +0900 Subject: [PATCH 6/6] Removed trailing commas --- .../AuthorizationRequirementDataTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs index 1265267391c..d939b662ea4 100644 --- a/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs +++ b/src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizationRequirementDataTests.cs @@ -37,7 +37,7 @@ public async Task Authorized() }); // act - var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }", }); + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }" }); // assert Assert.Equal(HttpStatusCode.OK, result.StatusCode); @@ -66,7 +66,7 @@ public async Task NotAuthorized() }); // act - var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }", }); + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ foo }" }); // assert Assert.Equal(HttpStatusCode.OK, result.StatusCode); @@ -98,7 +98,7 @@ public async Task Multiple_Authorized() }); // act - var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }", }); + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }" }); // assert Assert.Equal(HttpStatusCode.OK, result.StatusCode); @@ -130,7 +130,7 @@ public async Task Multiple_NotAuthorized() }); // act - var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }", }); + var result = await server.PostAsync(new ClientQueryRequest { Query = "{ fooMultiple }" }); // assert Assert.Equal(HttpStatusCode.OK, result.StatusCode);