From 8d9c30377282c3cb40a9482f991c9f53aad83817 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 13:54:37 +0300 Subject: [PATCH 01/22] Auth engine rework --- src/BasicSample/Program.cs | 33 +++-- .../GraphQL.Authorization.approved.txt | 111 ++++++++++++---- .../AuthenticatedUserRequirementTests.cs | 41 ------ ...rTests.cs => AuthorizationServiceTests.cs} | 122 ++++++++---------- .../AuthorizationSettingsTests.cs | 2 +- .../AuthorizationValidationRuleTests.cs | 35 +---- .../ClaimAuthorizationRequirementTests.cs | 104 --------------- .../AuthenticatedUserRequirementTests.cs | 53 ++++++++ .../ClaimAuthorizationRequirementTests.cs | 98 ++++++++++++++ .../ValidationTestBase.cs | 6 +- .../AuthorizationContext.cs | 45 ------- .../AuthorizationError.cs | 115 +++++++++++++++++ .../AuthorizationEvaluator.cs | 66 ---------- .../AuthorizationFailure.cs | 36 ++++++ .../AuthorizationPolicy.cs | 11 +- .../AuthorizationPolicyBuilder.cs | 16 +-- .../AuthorizationResult.cs | 22 ++-- .../AuthorizationSettings.cs | 2 +- .../AuthorizationValidationRule.cs | 102 ++++++++++----- .../DefaultAuthorizationContext.cs | 59 +++++++++ .../DefaultAuthorizationPolicyProvider.cs | 23 ++++ .../DefaultAuthorizationService.cs | 23 ++++ .../DefaultClaimsPrincipalAccessor.cs | 14 ++ .../IAuthorizationContext.cs | 59 +++++++++ .../IAuthorizationEvaluator.cs | 26 ---- .../IAuthorizationPolicyProvider.cs | 15 +++ .../IAuthorizationService.cs | 16 +++ .../IClaimsPrincipalAccessor.cs | 18 +++ .../AuthenticatedUserRequirement.cs | 10 +- .../ClaimAuthorizationRequirement.cs | 97 -------------- .../ClaimsAuthorizationRequirement.cs | 99 ++++++++++++++ .../Requirements/IAuthorizationRequirement.cs | 6 +- .../Requirements/PolicyExistsRequirement.cs | 30 +++++ src/Harness/GraphQLAuthExtensions.cs | 24 +--- 34 files changed, 941 insertions(+), 598 deletions(-) delete mode 100644 src/GraphQL.Authorization.Tests/AuthenticatedUserRequirementTests.cs rename src/GraphQL.Authorization.Tests/{AuthorizationEvaluatorTests.cs => AuthorizationServiceTests.cs} (50%) delete mode 100644 src/GraphQL.Authorization.Tests/ClaimAuthorizationRequirementTests.cs create mode 100644 src/GraphQL.Authorization.Tests/Requirements/AuthenticatedUserRequirementTests.cs create mode 100644 src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs delete mode 100644 src/GraphQL.Authorization/AuthorizationContext.cs create mode 100644 src/GraphQL.Authorization/AuthorizationError.cs delete mode 100644 src/GraphQL.Authorization/AuthorizationEvaluator.cs create mode 100644 src/GraphQL.Authorization/AuthorizationFailure.cs create mode 100644 src/GraphQL.Authorization/DefaultAuthorizationContext.cs create mode 100644 src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs create mode 100644 src/GraphQL.Authorization/DefaultAuthorizationService.cs create mode 100644 src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs create mode 100644 src/GraphQL.Authorization/IAuthorizationContext.cs delete mode 100644 src/GraphQL.Authorization/IAuthorizationEvaluator.cs create mode 100644 src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs create mode 100644 src/GraphQL.Authorization/IAuthorizationService.cs create mode 100644 src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs delete mode 100644 src/GraphQL.Authorization/Requirements/ClaimAuthorizationRequirement.cs create mode 100644 src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs create mode 100644 src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs diff --git a/src/BasicSample/Program.cs b/src/BasicSample/Program.cs index da796ed..1bdea9a 100644 --- a/src/BasicSample/Program.cs +++ b/src/BasicSample/Program.cs @@ -18,13 +18,14 @@ internal class Program private static async Task Main() { using var serviceProvider = new ServiceCollection() - .AddSingleton() - .AddTransient() - .AddTransient(s => + .AddSingleton() + .AddSingleton() + .AddSingleton() + .AddSingleton(provider => { var authSettings = new AuthorizationSettings(); authSettings.AddPolicy("AdminPolicy", p => p.RequireClaim("role", "Admin")); - return authSettings; + return new DefaultAuthorizationPolicyProvider(authSettings); }) .BuildServiceProvider(); @@ -41,20 +42,24 @@ type Query { "; var schema = Schema.For(definitions, builder => builder.Types.Include()); - // remove claims to see the failure var authorizedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("role", "Admin") })); + var nonauthorizedUser = new ClaimsPrincipal(new ClaimsIdentity()); - string json = await schema.ExecuteAsync(_ => + foreach (var principal in new[] { authorizedUser, nonauthorizedUser }) { - _.Query = "{ viewer { id name } }"; - _.ValidationRules = serviceProvider - .GetServices() - .Concat(DocumentValidator.CoreRules); - _.RequestServices = serviceProvider; - _.UserContext = new GraphQLUserContext { User = authorizedUser }; - }); + string json = await schema.ExecuteAsync(options => + { + options.Query = "{ viewer { id name } }"; + options.ValidationRules = serviceProvider + .GetServices() + .Concat(DocumentValidator.CoreRules); + options.RequestServices = serviceProvider; + options.UserContext = new GraphQLUserContext { User = principal }; + }); - Console.WriteLine(json); + Console.WriteLine(json); + Console.WriteLine(); + } } } diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index 6c0e83d..d86f2cf 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -3,25 +3,27 @@ namespace GraphQL.Authorization public class AuthenticatedUserRequirement : GraphQL.Authorization.IAuthorizationRequirement { public AuthenticatedUserRequirement() { } - public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context) { } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } } - public class AuthorizationContext + public class AuthorizationError : GraphQL.Validation.ValidationError { - public AuthorizationContext() { } - public System.Collections.Generic.IEnumerable Errors { get; } - public bool HasErrors { get; } - public System.Collections.Generic.IReadOnlyDictionary Inputs { get; set; } - public System.Security.Claims.ClaimsPrincipal User { get; set; } - public System.Collections.Generic.IDictionary UserContext { get; set; } - public void ReportError(string error) { } + public AuthorizationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } + public AuthorizationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, string message, GraphQL.Authorization.AuthorizationResult result) { } + public virtual GraphQL.Authorization.AuthorizationResult AuthorizationResult { get; } + public GraphQL.Language.AST.OperationType? OperationType { get; } + public static void AppendFailureHeader(System.Text.StringBuilder error, GraphQL.Language.AST.OperationType? operationType) { } + public static void AppendFailureLine(System.Text.StringBuilder error, GraphQL.Authorization.IAuthorizationRequirement authorizationRequirement) { } } - public class AuthorizationEvaluator : GraphQL.Authorization.IAuthorizationEvaluator + public class AuthorizationFailure { - public AuthorizationEvaluator(GraphQL.Authorization.AuthorizationSettings settings) { } - public System.Threading.Tasks.Task Evaluate(System.Security.Claims.ClaimsPrincipal principal, System.Collections.Generic.IDictionary userContext, System.Collections.Generic.IReadOnlyDictionary inputs, System.Collections.Generic.IEnumerable requiredPolicies) { } + public bool FailCalled { get; } + public System.Collections.Generic.IEnumerable FailedRequirements { get; } + public static GraphQL.Authorization.AuthorizationFailure ExplicitFail() { } + public static GraphQL.Authorization.AuthorizationFailure Failed(System.Collections.Generic.IEnumerable failed) { } } public class AuthorizationPolicy : GraphQL.Authorization.IAuthorizationPolicy { + public AuthorizationPolicy(params GraphQL.Authorization.IAuthorizationRequirement[] requirements) { } public AuthorizationPolicy(System.Collections.Generic.IEnumerable requirements) { } public System.Collections.Generic.IEnumerable Requirements { get; } } @@ -37,10 +39,10 @@ namespace GraphQL.Authorization } public class AuthorizationResult { - public AuthorizationResult() { } - public System.Collections.Generic.IEnumerable Errors { get; } + public GraphQL.Authorization.AuthorizationFailure Failure { get; } public bool Succeeded { get; } - public static GraphQL.Authorization.AuthorizationResult Fail(System.Collections.Generic.IEnumerable errors) { } + public static GraphQL.Authorization.AuthorizationResult Failed() { } + public static GraphQL.Authorization.AuthorizationResult Failed(GraphQL.Authorization.AuthorizationFailure failure) { } public static GraphQL.Authorization.AuthorizationResult Success() { } } public class AuthorizationSettings @@ -54,16 +56,49 @@ namespace GraphQL.Authorization } public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule { - public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationEvaluator evaluator) { } + public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationService authorizationService, GraphQL.Authorization.IClaimsPrincipalAccessor claimsPrincipalAccessor, GraphQL.Authorization.IAuthorizationPolicyProvider policyProvider) { } + protected virtual void AddValidationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } + protected virtual GraphQL.Authorization.IAuthorizationContext CreateAuthorizationContext(GraphQL.Validation.ValidationContext context, string policyName) { } public System.Threading.Tasks.Task ValidateAsync(GraphQL.Validation.ValidationContext context) { } } - public class ClaimAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement + public class ClaimsAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement + { + public ClaimsAuthorizationRequirement(string claimType) { } + public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues) { } + public ClaimsAuthorizationRequirement(string claimType, params string[] allowedValues) { } + public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues, System.Collections.Generic.IEnumerable displayValues) { } + public System.Collections.Generic.IEnumerable AllowedValues { get; } + public string ClaimType { get; } + public System.Collections.Generic.IEnumerable DisplayValues { get; } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } + } + public class DefaultAuthorizationContext : GraphQL.Authorization.IAuthorizationContext { - public ClaimAuthorizationRequirement(string claimType) { } - public ClaimAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues) { } - public ClaimAuthorizationRequirement(string claimType, params string[] allowedValues) { } - public ClaimAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues, System.Collections.Generic.IEnumerable displayValues) { } - public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context) { } + public DefaultAuthorizationContext(GraphQL.Authorization.IAuthorizationPolicy policy, System.Security.Claims.ClaimsPrincipal user) { } + public virtual bool HasFailed { get; } + public virtual bool HasSucceeded { get; } + public System.Collections.Generic.IReadOnlyDictionary Inputs { get; set; } + public virtual System.Collections.Generic.IEnumerable PendingRequirements { get; } + public GraphQL.Authorization.IAuthorizationPolicy Policy { get; } + public System.Security.Claims.ClaimsPrincipal User { get; } + public System.Collections.Generic.IDictionary UserContext { get; set; } + public virtual void Fail() { } + public virtual void Succeed(GraphQL.Authorization.IAuthorizationRequirement requirement) { } + } + public class DefaultAuthorizationPolicyProvider : GraphQL.Authorization.IAuthorizationPolicyProvider + { + public DefaultAuthorizationPolicyProvider(GraphQL.Authorization.AuthorizationSettings settings) { } + public GraphQL.Authorization.IAuthorizationPolicy GetPolicy(string policyName) { } + } + public class DefaultAuthorizationService : GraphQL.Authorization.IAuthorizationService + { + public DefaultAuthorizationService() { } + public System.Threading.Tasks.Task AuthorizeAsync(GraphQL.Authorization.IAuthorizationContext context) { } + } + public class DefaultClaimsPrincipalAccessor : GraphQL.Authorization.IClaimsPrincipalAccessor + { + public DefaultClaimsPrincipalAccessor() { } + public System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } } public class GraphQLAuthorizeAttribute : GraphQL.GraphQLAttribute { @@ -72,20 +107,46 @@ namespace GraphQL.Authorization public override void Modify(GraphQL.Utilities.FieldConfig field) { } public override void Modify(GraphQL.Utilities.TypeConfig type) { } } - public interface IAuthorizationEvaluator + public interface IAuthorizationContext { - System.Threading.Tasks.Task Evaluate(System.Security.Claims.ClaimsPrincipal principal, System.Collections.Generic.IDictionary userContext, System.Collections.Generic.IReadOnlyDictionary inputs, System.Collections.Generic.IEnumerable requiredPolicies); + bool HasFailed { get; } + bool HasSucceeded { get; } + System.Collections.Generic.IReadOnlyDictionary Inputs { get; } + System.Collections.Generic.IEnumerable PendingRequirements { get; } + GraphQL.Authorization.IAuthorizationPolicy Policy { get; } + System.Security.Claims.ClaimsPrincipal User { get; } + System.Collections.Generic.IDictionary UserContext { get; } + void Fail(); + void Succeed(GraphQL.Authorization.IAuthorizationRequirement requirement); } public interface IAuthorizationPolicy { System.Collections.Generic.IEnumerable Requirements { get; } } + public interface IAuthorizationPolicyProvider + { + GraphQL.Authorization.IAuthorizationPolicy GetPolicy(string policyName); + } public interface IAuthorizationRequirement { - System.Threading.Tasks.Task Authorize(GraphQL.Authorization.AuthorizationContext context); + System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context); + } + public interface IAuthorizationService + { + System.Threading.Tasks.Task AuthorizeAsync(GraphQL.Authorization.IAuthorizationContext context); + } + public interface IClaimsPrincipalAccessor + { + System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context); } public interface IProvideClaimsPrincipal { System.Security.Claims.ClaimsPrincipal User { get; } } + public class PolicyDefinedRequirement : GraphQL.Authorization.IAuthorizationRequirement + { + public PolicyDefinedRequirement(string policyName) { } + public string PolicyName { get; } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } + } } \ No newline at end of file diff --git a/src/GraphQL.Authorization.Tests/AuthenticatedUserRequirementTests.cs b/src/GraphQL.Authorization.Tests/AuthenticatedUserRequirementTests.cs deleted file mode 100644 index 61fdd46..0000000 --- a/src/GraphQL.Authorization.Tests/AuthenticatedUserRequirementTests.cs +++ /dev/null @@ -1,41 +0,0 @@ -using System.Linq; -using System.Threading.Tasks; -using Shouldly; -using Xunit; - -namespace GraphQL.Authorization.Tests -{ - public class AuthenticatedUserRequirementTests - { - [Fact] - public async Task produces_error_when_not_authenticated() - { - var req = new AuthenticatedUserRequirement(); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal() - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeTrue(); - context.Errors.Single().ShouldBe("An authenticated user is required."); - } - - [Fact] - public async Task no_errors_when_authenticated() - { - var req = new AuthenticatedUserRequirement(); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal("jwt") - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeFalse(); - } - } -} diff --git a/src/GraphQL.Authorization.Tests/AuthorizationEvaluatorTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs similarity index 50% rename from src/GraphQL.Authorization.Tests/AuthorizationEvaluatorTests.cs rename to src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs index 1230ebb..557ea4c 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationEvaluatorTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs @@ -1,20 +1,34 @@ using System; using System.Collections.Generic; +using System.Security.Claims; using System.Threading.Tasks; using Shouldly; using Xunit; namespace GraphQL.Authorization.Tests { - public class AuthorizationEvaluatorTests + public class AuthorizationServiceTests { - private readonly AuthorizationEvaluator _evaluator; + private readonly DefaultAuthorizationService _authorizationService; private readonly AuthorizationSettings _settings; - public AuthorizationEvaluatorTests() + public AuthorizationServiceTests() { _settings = new AuthorizationSettings(); - _evaluator = new AuthorizationEvaluator(_settings); + _authorizationService = new DefaultAuthorizationService(); + } + + private IAuthorizationContext CreateAuthorizationContext( + ClaimsPrincipal principal, + IDictionary userContext, + IReadOnlyDictionary inputs, + string requiredPolicy) + { + return new DefaultAuthorizationContext(new DefaultAuthorizationPolicyProvider(_settings).GetPolicy(requiredPolicy), principal) + { + UserContext = userContext, + Inputs = inputs, + }; } [Fact] @@ -22,12 +36,12 @@ public async Task fails_with_null_principal() { _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); - var result = await _evaluator.Evaluate( + var result = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext( null, null, null, - new[] { "MyPolicy" } - ); + "MyPolicy" + )); result.Succeeded.ShouldBeFalse(); } @@ -37,117 +51,85 @@ public async Task fails_when_missing_claim() { _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); - var result = await _evaluator.Evaluate( + var result = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext( ValidationTestBase.CreatePrincipal(), null, null, - new[] { "MyPolicy" } - ); + "MyPolicy" + )); result.Succeeded.ShouldBeFalse(); } [Fact] - public async Task fails_when_missing_policy() + public void throws_when_missing_policy() { _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); - var result = await _evaluator.Evaluate( - ValidationTestBase.CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }), + Should.Throw(() => CreateAuthorizationContext( + ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }), null, null, - new[] { "PolicyDoesNotExist" } - ); - - result.Succeeded.ShouldBeFalse(); + "PolicyDoesNotExist" + )).ParamName.ShouldBe("policy"); } [Fact] - public async Task succeeds_when_policy_applied() + public void throws_when_null_policy() { _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); - var result = await _evaluator.Evaluate( - ValidationTestBase.CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }), + Should.Throw(() => CreateAuthorizationContext( + ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }), null, null, - new[] { "MyPolicy" } - ); - - result.Succeeded.ShouldBeTrue(); - } - - [Fact] - public async Task succeeds_with_claim_value() - { - _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin", "true")); - - var result = await _evaluator.Evaluate( - ValidationTestBase.CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }), - null, - null, - new[] { "MyPolicy" } - ); - - result.Succeeded.ShouldBeTrue(); + null + )).ParamName.ShouldBe("policy"); } [Fact] - public async Task succeeds_when_null_policies() + public async Task succeeds_when_policy_applied() { _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); - var result = await _evaluator.Evaluate( - ValidationTestBase.CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }), + var result = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext( + ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }), null, null, - null - ); + "MyPolicy" + )); result.Succeeded.ShouldBeTrue(); } [Fact] - public async Task succeeds_when_empty_policies() + public async Task succeeds_with_claim_value() { - _settings.AddPolicy("MyPolicy", _ => { }); + _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin", "true")); - var result = await _evaluator.Evaluate( - ValidationTestBase.CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }), + var result = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext( + ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }), null, null, - Array.Empty() - ); + "MyPolicy" + )); result.Succeeded.ShouldBeTrue(); } [Fact] - public async Task succeeds_when_null_principal() + public async Task fails_when_null_principal() { - var result = await _evaluator.Evaluate( + _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin", "true")); + + var result = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext( null, null, null, - null - ); + "MyPolicy" + )); - result.Succeeded.ShouldBeTrue(); + result.Succeeded.ShouldBeFalse(); } } } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index 158f50b..c8d8746 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -21,7 +21,7 @@ public void can_add_a_claim_policy() _settings.Policies.Count().ShouldBe(1); var policy = _settings.Policies.Single(); - policy.Requirements.Single().ShouldBeOfType(); + policy.Requirements.Single().ShouldBeOfType(); } } } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index 61cb32d..024df26 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -17,10 +17,7 @@ public void class_policy_success() { _.Query = @"query { post }"; _.Schema = BasicSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -46,10 +43,7 @@ public void field_policy_success() { _.Query = @"query { post }"; _.Schema = BasicSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -74,10 +68,7 @@ public void nested_type_policy_success() { _.Query = @"query { post }"; _.Schema = NestedSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -126,10 +117,7 @@ public void passes_with_claim_on_input_type() { _.Query = @"query { author(input: { name: ""Quinn"" }) }"; _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -156,10 +144,7 @@ public void passes_with_multiple_policies_on_field_and_single_on_input_type() { _.Query = @"query { author(input: { name: ""Quinn"" }) project(input: { name: ""TEST"" }) }"; _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -170,10 +155,7 @@ public void Issue61() { _.Query = @"query { unknown(obj: {id: 7}) }"; _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } @@ -186,10 +168,7 @@ public void passes_with_policy_on_connection_type() { _.Query = @"query { posts { items { id } } }"; _.Schema = TypedSchema(); - _.User = CreatePrincipal(claims: new Dictionary - { - { "Admin", "true" } - }); + _.User = CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }); }); } diff --git a/src/GraphQL.Authorization.Tests/ClaimAuthorizationRequirementTests.cs b/src/GraphQL.Authorization.Tests/ClaimAuthorizationRequirementTests.cs deleted file mode 100644 index c760af2..0000000 --- a/src/GraphQL.Authorization.Tests/ClaimAuthorizationRequirementTests.cs +++ /dev/null @@ -1,104 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; -using Shouldly; -using Xunit; - -namespace GraphQL.Authorization.Tests -{ - public class ClaimAuthorizationRequirementTests - { - [Fact] - public async Task produces_error_when_missing_claim_ignoring_value() - { - var req = new ClaimAuthorizationRequirement("Admin"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal() - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeTrue(); - context.Errors.Single().ShouldBe("Required claim 'Admin' is not present."); - } - - [Fact] - public async Task produces_error_when_missing_claim_with_single_value() - { - var req = new ClaimAuthorizationRequirement("Admin", "true"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal() - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeTrue(); - context.Errors.Single().ShouldBe("Required claim 'Admin' with any value of 'true' is not present."); - } - - [Fact] - public async Task produces_error_when_missing_claim_with_multiple_values() - { - var req = new ClaimAuthorizationRequirement("Admin", "true", "maybe"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal() - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeTrue(); - context.Errors.Single().ShouldBe("Required claim 'Admin' with any value of 'true, maybe' is not present."); - } - - [Fact] - public async Task succeeds_when_claim_with_ignoring_value() - { - var req = new ClaimAuthorizationRequirement("Admin"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }) - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeFalse(); - } - - [Fact] - public async Task succeeds_when_claim_with_single_value() - { - var req = new ClaimAuthorizationRequirement("Admin", "true"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } }) - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeFalse(); - } - - [Fact] - public async Task succeeds_when_claim_with_multiple_values() - { - var req = new ClaimAuthorizationRequirement("Admin", "true", "maybe"); - - var context = new AuthorizationContext - { - User = ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "maybe" } }) - }; - - await req.Authorize(context); - - context.HasErrors.ShouldBeFalse(); - } - } -} diff --git a/src/GraphQL.Authorization.Tests/Requirements/AuthenticatedUserRequirementTests.cs b/src/GraphQL.Authorization.Tests/Requirements/AuthenticatedUserRequirementTests.cs new file mode 100644 index 0000000..2b6aec4 --- /dev/null +++ b/src/GraphQL.Authorization.Tests/Requirements/AuthenticatedUserRequirementTests.cs @@ -0,0 +1,53 @@ +using System.Linq; +using System.Threading.Tasks; +using Shouldly; +using Xunit; + +namespace GraphQL.Authorization.Tests +{ + public class AuthenticatedUserRequirementTests + { + [Fact] + public async Task produces_error_when_user_is_null() + { + var req = new AuthenticatedUserRequirement(); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, null); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("An authenticated user is required."); + } + + [Fact] + public async Task produces_error_when_not_authenticated() + { + var req = new AuthenticatedUserRequirement(); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal()); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("An authenticated user is required."); + } + + [Fact] + public async Task no_errors_when_authenticated() + { + var req = new AuthenticatedUserRequirement(); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal("jwt")); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeTrue(); + context.HasFailed.ShouldBeFalse(); + } + } +} diff --git a/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs b/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs new file mode 100644 index 0000000..913e2c5 --- /dev/null +++ b/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs @@ -0,0 +1,98 @@ +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Shouldly; +using Xunit; + +namespace GraphQL.Authorization.Tests +{ + public class ClaimAuthorizationRequirementTests + { + [Fact] + public async Task produces_error_when_missing_claim_ignoring_value() + { + var req = new ClaimsAuthorizationRequirement("Admin"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal()); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("Required claim 'Admin' is not present."); + } + + [Fact] + public async Task produces_error_when_missing_claim_with_single_value() + { + var req = new ClaimsAuthorizationRequirement("Admin", "true"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal()); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("Required claim 'Admin' with any value of 'true' is not present."); + } + + [Fact] + public async Task produces_error_when_missing_claim_with_multiple_values() + { + var req = new ClaimsAuthorizationRequirement("Admin", "true", "maybe"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal()); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("Required claim 'Admin' with any value of 'true, maybe' is not present."); + } + + [Fact] + public async Task succeeds_when_claim_with_ignoring_value() + { + var req = new ClaimsAuthorizationRequirement("Admin"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } })); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeTrue(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Count().ShouldBe(0); + } + + [Fact] + public async Task succeeds_when_claim_with_single_value() + { + var req = new ClaimsAuthorizationRequirement("Admin", "true"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "true" } })); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeTrue(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Count().ShouldBe(0); + } + + [Fact] + public async Task succeeds_when_claim_with_multiple_values() + { + var req = new ClaimsAuthorizationRequirement("Admin", "true", "maybe"); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal(claims: new Dictionary { { "Admin", "maybe" } })); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeTrue(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Count().ShouldBe(0); + } + } +} diff --git a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs index 10efddf..8c3ef1b 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs @@ -13,13 +13,13 @@ public class ValidationTestBase public ValidationTestBase() { Settings = new AuthorizationSettings(); - Rule = new AuthorizationValidationRule(new AuthorizationEvaluator(Settings)); + Rule = new AuthorizationValidationRule(new DefaultAuthorizationService(), new DefaultClaimsPrincipalAccessor(), new DefaultAuthorizationPolicyProvider(Settings)); } - protected AuthorizationValidationRule Rule { get; } - protected AuthorizationSettings Settings { get; } + protected AuthorizationValidationRule Rule { get; } + protected void ShouldPassRule(Action configure) { var config = new ValidationTestConfig(); diff --git a/src/GraphQL.Authorization/AuthorizationContext.cs b/src/GraphQL.Authorization/AuthorizationContext.cs deleted file mode 100644 index cdeaed9..0000000 --- a/src/GraphQL.Authorization/AuthorizationContext.cs +++ /dev/null @@ -1,45 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using System.Security.Claims; - -namespace GraphQL.Authorization -{ - /// - /// Provides context information for . - /// - public class AuthorizationContext - { - private List _errors; - - /// - /// Current user. - /// - public ClaimsPrincipal User { get; set; } - - /// - /// Arbitrary user defined context represented as dictionary. - /// - public IDictionary UserContext { get; set; } - - /// - /// Represents a readonly dictionary of variable inputs to an executed document. - /// - public IReadOnlyDictionary Inputs { get; set; } - - /// - /// Returns a set of authorization errors. - /// - public IEnumerable Errors => _errors ?? Enumerable.Empty(); - - /// - /// Returns whether there are any errors. - /// - public bool HasErrors => _errors?.Count > 0; - - /// - /// Reports an error during evaluation of policy requirement. - /// - /// Error message. - public void ReportError(string error) => (_errors ??= new List()).Add(error); - } -} diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs new file mode 100644 index 0000000..50f28f2 --- /dev/null +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -0,0 +1,115 @@ +using System; +using System.Linq; +using System.Text; +using GraphQL.Language.AST; +using GraphQL.Validation; + +namespace GraphQL.Authorization +{ + /// + /// An error that represents an authorization failure while parsing the document. + /// + public class AuthorizationError : ValidationError + { + /// + /// Initializes a new instance of the class for a specified authorization result. + /// + public AuthorizationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + : this(node, context, GenerateMessage(operationType, result), result) + { + OperationType = operationType; + } + + /// + /// Initializes a new instance of the class for a specified authorization result with a specific error message. + /// + public AuthorizationError(INode node, ValidationContext context, string message, AuthorizationResult result) + : base(context.Document.OriginalQuery, "6.1.1", message, node == null ? Array.Empty() : new INode[] { node }) + { + Code = "authorization"; + AuthorizationResult = result; + } + + /// + /// Returns the result of authorization request. + /// + public virtual AuthorizationResult AuthorizationResult { get; } + + /// + /// The GraphQL operation type. + /// + public OperationType? OperationType { get; } + + private static string GenerateMessage(OperationType? operationType, AuthorizationResult result) + { + var error = new StringBuilder(); + AppendFailureHeader(error, operationType); + + foreach (var failure in result.Failure.FailedRequirements) + { + AppendFailureLine(error, failure); + } + + return error.ToString(); + } + + private static string GetOperationType(OperationType? operationType) + { + return operationType switch + { + Language.AST.OperationType.Query => "query", + Language.AST.OperationType.Mutation => "mutation", + Language.AST.OperationType.Subscription => "subscription", + _ => "operation", + }; + } + + /// + /// Appends the error message header for this to the provided . + /// + /// The error message . + /// The GraphQL operation type. + public static void AppendFailureHeader(StringBuilder error, OperationType? operationType) + { + error.Append("You are not authorized to run this ") + .Append(GetOperationType(operationType)) + .Append("."); + } + + /// + /// Appends a description of the failed to the supplied . + /// + /// The which is used to compose the error message. + /// The failed . + public static void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) + { + error.AppendLine(); + + switch (authorizationRequirement) + { + case PolicyDefinedRequirement policyExistsRequirement: + error.Append($"Required policy '{policyExistsRequirement.PolicyName}' is not present."); + break; + + case AuthenticatedUserRequirement _: + error.Append("An authenticated user is required."); + break; + + case ClaimsAuthorizationRequirement claimsAuthorizationRequirement: + error.Append("Required claim '"); + error.Append(claimsAuthorizationRequirement.ClaimType); + if (claimsAuthorizationRequirement.AllowedValues == null || !claimsAuthorizationRequirement.AllowedValues.Any()) + { + error.Append("' is not present."); + } + else + { + error.Append("' with any value of '"); + error.Append(string.Join(", ", claimsAuthorizationRequirement.AllowedValues ?? claimsAuthorizationRequirement.DisplayValues)); + error.Append("' is not present."); + } + break; + } + } + } +} diff --git a/src/GraphQL.Authorization/AuthorizationEvaluator.cs b/src/GraphQL.Authorization/AuthorizationEvaluator.cs deleted file mode 100644 index 52f7564..0000000 --- a/src/GraphQL.Authorization/AuthorizationEvaluator.cs +++ /dev/null @@ -1,66 +0,0 @@ -using System.Collections.Generic; -using System.Security.Claims; -using System.Threading.Tasks; - -namespace GraphQL.Authorization -{ - /// - /// Default implementation of . - /// - public class AuthorizationEvaluator : IAuthorizationEvaluator - { - private readonly AuthorizationSettings _settings; - - /// - /// Creates an instance of with the - /// specified authorization settings. - /// - public AuthorizationEvaluator(AuthorizationSettings settings) - { - _settings = settings; - } - - /// - public async Task Evaluate( - ClaimsPrincipal principal, - IDictionary userContext, - IReadOnlyDictionary inputs, - IEnumerable requiredPolicies) - { - if (requiredPolicies == null) - return AuthorizationResult.Success(); - - var context = new AuthorizationContext - { - User = principal ?? new ClaimsPrincipal(new ClaimsIdentity()), - UserContext = userContext, - Inputs = inputs - }; - - var tasks = new List(); - - foreach (string requiredPolicy in requiredPolicies) - { - var authorizationPolicy = _settings.GetPolicy(requiredPolicy); - if (authorizationPolicy == null) - { - context.ReportError($"Required policy '{requiredPolicy}' is not present."); - } - else - { - foreach (var r in authorizationPolicy.Requirements) - { - var task = r.Authorize(context); - tasks.Add(task); - } - } - } - - await Task.WhenAll(tasks).ConfigureAwait(false); - - return context.HasErrors - ? AuthorizationResult.Fail(context.Errors) - : AuthorizationResult.Success(); - } - } -} diff --git a/src/GraphQL.Authorization/AuthorizationFailure.cs b/src/GraphQL.Authorization/AuthorizationFailure.cs new file mode 100644 index 0000000..ab45e52 --- /dev/null +++ b/src/GraphQL.Authorization/AuthorizationFailure.cs @@ -0,0 +1,36 @@ +using System; +using System.Collections.Generic; + +namespace GraphQL.Authorization +{ + /// + /// Encapsulates a failure result of . + /// + public class AuthorizationFailure + { + private AuthorizationFailure() { } + + /// + /// Failure was due to being called. + /// + public bool FailCalled { get; private set; } + + /// + /// Failure was due to these requirements not being met via . + /// + public IEnumerable FailedRequirements { get; private set; } = Array.Empty(); + + /// + /// Return a failure due to being called. + /// + /// The failure. + public static AuthorizationFailure ExplicitFail() => new AuthorizationFailure { FailCalled = true }; + + /// + /// Return a failure due to some requirements not being met via . + /// + /// The requirements that were not met. + /// The failure. + public static AuthorizationFailure Failed(IEnumerable failed) => new AuthorizationFailure { FailedRequirements = failed }; + } +} diff --git a/src/GraphQL.Authorization/AuthorizationPolicy.cs b/src/GraphQL.Authorization/AuthorizationPolicy.cs index 114ae23..0ae2f0e 100644 --- a/src/GraphQL.Authorization/AuthorizationPolicy.cs +++ b/src/GraphQL.Authorization/AuthorizationPolicy.cs @@ -22,11 +22,20 @@ public AuthorizationPolicy(IEnumerable requirements) _requirements.ForEach(req => { if (req == null) - throw new ArgumentNullException(nameof(requirements), $"One of the ({_requirements.Count}) requirements is null"); + throw new ArgumentNullException(nameof(requirements), $"One of the {_requirements.Count} requirements is null"); }); } } + /// + /// Creates a policy with a set of specified requirements. + /// + /// Specified requirements. + public AuthorizationPolicy(params IAuthorizationRequirement[] requirements) + : this((IEnumerable)requirements) + { + } + /// public IEnumerable Requirements => _requirements; } diff --git a/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs b/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs index c126b1c..b283076 100644 --- a/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs +++ b/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs @@ -18,41 +18,41 @@ public class AuthorizationPolicyBuilder public AuthorizationPolicy Build() => new AuthorizationPolicy(_requirements); /// - /// Adds with the specified claim type. + /// Adds with the specified claim type. /// /// Type of the claim. /// Reference to the same builder. public AuthorizationPolicyBuilder RequireClaim(string claimType) { - _requirements.Add(new ClaimAuthorizationRequirement(claimType)); + _requirements.Add(new ClaimsAuthorizationRequirement(claimType)); return this; } /// - /// Adds with the specified claim type and allowed values. + /// Adds with the specified claim type and allowed values. /// /// Type of the claim. /// Allowed values for this claim. /// Reference to the same builder. public AuthorizationPolicyBuilder RequireClaim(string claimType, params string[] allowedValues) { - _requirements.Add(new ClaimAuthorizationRequirement(claimType, allowedValues)); + _requirements.Add(new ClaimsAuthorizationRequirement(claimType, allowedValues)); return this; } /// - /// Adds with the specified claim type, allowed values and display values. + /// Adds with the specified claim type, allowed values and display values. /// /// Type of the claim. /// Allowed values for this claim. /// - /// Display values for this claim. If no allowed claims are found, display values will be used to generate - /// an error message for . + /// Display values for this claim. If no allowed claims are found, display values should be used to generate + /// an error message if the requirement is not met. /// /// Reference to the same builder. public AuthorizationPolicyBuilder RequireClaim(string claimType, IEnumerable allowedValues, IEnumerable displayValues) { - _requirements.Add(new ClaimAuthorizationRequirement(claimType, allowedValues, displayValues)); + _requirements.Add(new ClaimsAuthorizationRequirement(claimType, allowedValues, displayValues)); return this; } diff --git a/src/GraphQL.Authorization/AuthorizationResult.cs b/src/GraphQL.Authorization/AuthorizationResult.cs index 3a70e40..c5238a7 100644 --- a/src/GraphQL.Authorization/AuthorizationResult.cs +++ b/src/GraphQL.Authorization/AuthorizationResult.cs @@ -1,5 +1,3 @@ -using System.Collections.Generic; - namespace GraphQL.Authorization { /// @@ -10,27 +8,33 @@ public class AuthorizationResult // allocation optimization for green path private static readonly AuthorizationResult _success = new AuthorizationResult { Succeeded = true }; + private AuthorizationResult() { } + /// /// Is the authorization result successful? /// public bool Succeeded { get; private set; } /// - /// Returns a set of authorization errors if the authorization result is unsuccessful. + /// Contains information about why authorization failed. /// - public IEnumerable Errors { get; private set; } + public AuthorizationFailure Failure { get; private set; } /// /// Creates successful authorization result. /// - /// Instance of . public static AuthorizationResult Success() => _success; /// - /// Creates unsuccessful authorization result + /// Creates a failed authorization result. + /// + /// Contains information about why authorization failed. + public static AuthorizationResult Failed(AuthorizationFailure failure) => new AuthorizationResult { Failure = failure }; + + /// + /// Creates a failed authorization result. /// - /// A set of authorization errors. - /// Instance of . - public static AuthorizationResult Fail(IEnumerable errors) => new AuthorizationResult { Errors = errors }; + /// The . + public static AuthorizationResult Failed() => new AuthorizationResult { Failure = AuthorizationFailure.ExplicitFail() }; } } diff --git a/src/GraphQL.Authorization/AuthorizationSettings.cs b/src/GraphQL.Authorization/AuthorizationSettings.cs index 0fc0362..89a2b44 100644 --- a/src/GraphQL.Authorization/AuthorizationSettings.cs +++ b/src/GraphQL.Authorization/AuthorizationSettings.cs @@ -44,7 +44,7 @@ public IEnumerable GetPolicies(IEnumerable policie /// /// Name of the required policy. /// Required policy if exists, otherwise . - public IAuthorizationPolicy GetPolicy(string name) => _policies.TryGetValue(name, out var policy) ? policy : null; + public IAuthorizationPolicy GetPolicy(string name) => name == null ? null : _policies.TryGetValue(name, out var policy) ? policy : null; /// /// Adds a policy with the specified name. If a policy with that name already exists then it will be replaced. diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index 892c542..fc257f0 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -1,3 +1,6 @@ +using System; +using System.Collections.Generic; +using System.Security.Claims; using System.Threading.Tasks; using GraphQL.Language.AST; using GraphQL.Types; @@ -11,21 +14,25 @@ namespace GraphQL.Authorization /// public class AuthorizationValidationRule : IValidationRule { - private readonly IAuthorizationEvaluator _evaluator; + private readonly IAuthorizationService _authorizationService; + private readonly IClaimsPrincipalAccessor _claimsPrincipalAccessor; + private readonly IAuthorizationPolicyProvider _policyProvider; /// /// Creates an instance of with - /// the specified authorization evaluator. + /// the specified values. /// - public AuthorizationValidationRule(IAuthorizationEvaluator evaluator) + public AuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) { - _evaluator = evaluator; + _authorizationService = authorizationService; + _claimsPrincipalAccessor = claimsPrincipalAccessor; + _policyProvider = policyProvider; } /// - public Task ValidateAsync(ValidationContext context) + public async Task ValidateAsync(ValidationContext context) { - var userContext = context.UserContext as IProvideClaimsPrincipal; + await AuthorizeAsync(null, context.Schema, context, null); var operationType = OperationType.Query; // this could leak info about hidden fields or types in error messages @@ -34,13 +41,13 @@ public Task ValidateAsync(ValidationContext context) // - filtering the Schema is not currently supported // TODO: apply ISchemaFilter - context.Schema.Filter.AllowXXX - return Task.FromResult((INodeVisitor)new NodeVisitors( + return new NodeVisitors( new MatchingNodeVisitor((astType, context) => { operationType = astType.OperationType; var type = context.TypeInfo.GetLastType(); - CheckAuth(astType, type, userContext, context, operationType); + AuthorizeAsync(astType, type, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; }), new MatchingNodeVisitor((objectFieldAst, context) => @@ -48,7 +55,7 @@ public Task ValidateAsync(ValidationContext context) if (context.TypeInfo.GetArgument()?.ResolvedType.GetNamedType() is IComplexGraphType argumentType) { var fieldType = argumentType.GetField(objectFieldAst.Name); - CheckAuth(objectFieldAst, fieldType, userContext, context, operationType); + AuthorizeAsync(objectFieldAst, fieldType, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; } }), @@ -60,39 +67,68 @@ public Task ValidateAsync(ValidationContext context) return; // check target field - CheckAuth(fieldAst, fieldDef, userContext, context, operationType); + AuthorizeAsync(fieldAst, fieldDef, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; // check returned graph type - CheckAuth(fieldAst, fieldDef.ResolvedType.GetNamedType(), userContext, context, operationType); + AuthorizeAsync(fieldAst, fieldDef.ResolvedType.GetNamedType(), context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; }) - )); + ); } - private void CheckAuth( - INode node, - IProvideMetadata provider, - IProvideClaimsPrincipal userContext, - ValidationContext context, - OperationType? operationType) + /// + /// Creates authorization context to pass to . + /// + /// GraphQL validation context. + /// Name of checked policy for the current authorization processing. + protected virtual IAuthorizationContext CreateAuthorizationContext(ValidationContext context, string policyName) + { + if (policyName == null) + throw new ArgumentNullException(nameof(policyName)); + + return new DefaultAuthorizationContext( + _policyProvider.GetPolicy(policyName) ?? new AuthorizationPolicy(new PolicyDefinedRequirement(policyName)), + _claimsPrincipalAccessor.GetClaimsPrincipal(context) ?? new ClaimsPrincipal(new ClaimsIdentity())) + { + UserContext = context.UserContext, + Inputs = context.Inputs, + }; + } + + private async Task AuthorizeAsync(INode node, IProvideMetadata provider, ValidationContext context, OperationType? operationType) { - if (provider == null || !provider.RequiresAuthorization()) - return; + var policyNames = provider?.GetPolicies(); - // TODO: async -> sync transition - var result = _evaluator - .Evaluate(userContext?.User, context.UserContext, context.Inputs, provider.GetPolicies()) - .GetAwaiter() - .GetResult(); + if (policyNames?.Count == 1) + { + // small optimization for the single policy - no 'new List<>()', no 'await Task.WhenAll()' + var authorizationResult = await _authorizationService.AuthorizeAsync(CreateAuthorizationContext(context, policyNames[0])); + if (!authorizationResult.Succeeded) + AddValidationError(node, context, operationType, authorizationResult); + } + else if (policyNames?.Count > 1) + { + var tasks = new List>(policyNames.Count); + foreach (string policyName in policyNames) + { + var task = _authorizationService.AuthorizeAsync(CreateAuthorizationContext(context, policyName)); + tasks.Add(task); + } - if (result.Succeeded) - return; + var authorizationResults = await Task.WhenAll(tasks); - string errors = string.Join("\n", result.Errors); + foreach (var result in authorizationResults) + { + if (!result.Succeeded) + AddValidationError(node, context, operationType, result); + } + } + } - context.ReportError(new ValidationError( - context.Document.OriginalQuery, - "authorization", - $"You are not authorized to run this {operationType.ToString().ToLower()}.\n{errors}", - node)); + /// + /// Adds an authorization failure error to the document response. + /// + protected virtual void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + { + context.ReportError(new AuthorizationError(node, context, operationType, result)); } } } diff --git a/src/GraphQL.Authorization/DefaultAuthorizationContext.cs b/src/GraphQL.Authorization/DefaultAuthorizationContext.cs new file mode 100644 index 0000000..1068f19 --- /dev/null +++ b/src/GraphQL.Authorization/DefaultAuthorizationContext.cs @@ -0,0 +1,59 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; + +namespace GraphQL.Authorization +{ + /// + /// Default implementation of . + /// + public class DefaultAuthorizationContext : IAuthorizationContext + { + private bool _failCalled; + private bool _succeedCalled; + + /// + /// Creates a new instance of . + /// + /// A checked policy for the current authorization processing. + /// A representing the current user. + public DefaultAuthorizationContext(IAuthorizationPolicy policy, ClaimsPrincipal user) + { + Policy = policy ?? throw new ArgumentNullException(nameof(policy)); + User = user; + PendingRequirements = new HashSet(policy.Requirements); + } + + /// + public IAuthorizationPolicy Policy { get; } + + /// + public ClaimsPrincipal User { get; } + + /// + public IDictionary UserContext { get; set; } + + /// + public IReadOnlyDictionary Inputs { get; set; } + + /// + public virtual IEnumerable PendingRequirements { get; } + + /// + public virtual bool HasFailed => _failCalled; + + /// + public virtual bool HasSucceeded => !_failCalled && _succeedCalled && !PendingRequirements.Any(); + + /// + public virtual void Fail() => _failCalled = true; + + /// + public virtual void Succeed(IAuthorizationRequirement requirement) + { + _succeedCalled = true; + ((HashSet)PendingRequirements).Remove(requirement); + } + } +} diff --git a/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs b/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs new file mode 100644 index 0000000..1e008e2 --- /dev/null +++ b/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs @@ -0,0 +1,23 @@ +namespace GraphQL.Authorization +{ + /// + /// Default implementation of that gets + /// policies from the configured . + /// + public class DefaultAuthorizationPolicyProvider : IAuthorizationPolicyProvider + { + private readonly AuthorizationSettings _settings; + + /// + /// Creates an instance of with the + /// specified authorization settings. + /// + public DefaultAuthorizationPolicyProvider(AuthorizationSettings settings) + { + _settings = settings; + } + + /// + public IAuthorizationPolicy GetPolicy(string policyName) => _settings.GetPolicy(policyName); + } +} diff --git a/src/GraphQL.Authorization/DefaultAuthorizationService.cs b/src/GraphQL.Authorization/DefaultAuthorizationService.cs new file mode 100644 index 0000000..7d8dcc8 --- /dev/null +++ b/src/GraphQL.Authorization/DefaultAuthorizationService.cs @@ -0,0 +1,23 @@ +using System.Threading.Tasks; + +namespace GraphQL.Authorization +{ + /// + /// Default implementation of . + /// + public class DefaultAuthorizationService : IAuthorizationService + { + /// + public async Task AuthorizeAsync(IAuthorizationContext context) + { + foreach (var requirement in context.Policy.Requirements) + await requirement.Authorize(context); + + return context.HasSucceeded + ? AuthorizationResult.Success() + : AuthorizationResult.Failed(context.HasFailed + ? AuthorizationFailure.ExplicitFail() + : AuthorizationFailure.Failed(context.PendingRequirements)); + } + } +} diff --git a/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs new file mode 100644 index 0000000..bffe7e8 --- /dev/null +++ b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs @@ -0,0 +1,14 @@ +using System.Security.Claims; +using GraphQL.Validation; + +namespace GraphQL.Authorization +{ + /// + /// The default claims principal accessor. + /// + public class DefaultClaimsPrincipalAccessor : IClaimsPrincipalAccessor + { + /// + public ClaimsPrincipal GetClaimsPrincipal(ValidationContext context) => (context.UserContext as IProvideClaimsPrincipal)?.User; + } +} diff --git a/src/GraphQL.Authorization/IAuthorizationContext.cs b/src/GraphQL.Authorization/IAuthorizationContext.cs new file mode 100644 index 0000000..dc0c97c --- /dev/null +++ b/src/GraphQL.Authorization/IAuthorizationContext.cs @@ -0,0 +1,59 @@ +using System.Collections.Generic; +using System.Security.Claims; + +namespace GraphQL.Authorization +{ + /// + /// Provides context information for the current authorization processing. + /// + public interface IAuthorizationContext + { + /// + /// A checked policy for the current authorization processing. + /// + IAuthorizationPolicy Policy { get; } + + /// + /// Current user. + /// + ClaimsPrincipal User { get; } + + /// + /// Arbitrary user defined context represented as a dictionary. + /// + IDictionary UserContext { get; } + + /// + /// Represents a readonly dictionary of variable inputs to an executed document. + /// + IReadOnlyDictionary Inputs { get; } + + /// + /// Gets the requirements that have not yet been marked as succeeded. + /// + IEnumerable PendingRequirements { get; } + + /// + /// Flag indicating whether the current authorization processing has failed. + /// + bool HasFailed { get; } + + /// + /// Flag indicating whether the current authorization processing has succeeded. + /// + bool HasSucceeded { get; } + + /// + /// Called to indicate will + /// never return , even if all requirements are met. + /// + void Fail(); + + /// + /// Called to mark the specified as being + /// successfully evaluated. + /// + /// The requirement whose evaluation has succeeded. + void Succeed(IAuthorizationRequirement requirement); + } +} diff --git a/src/GraphQL.Authorization/IAuthorizationEvaluator.cs b/src/GraphQL.Authorization/IAuthorizationEvaluator.cs deleted file mode 100644 index 84be38a..0000000 --- a/src/GraphQL.Authorization/IAuthorizationEvaluator.cs +++ /dev/null @@ -1,26 +0,0 @@ -using System.Collections.Generic; -using System.Security.Claims; -using System.Threading.Tasks; - -namespace GraphQL.Authorization -{ - /// - /// Interface to evaluate the authorization result. - /// - public interface IAuthorizationEvaluator - { - /// - /// Evaluates authorization result. - /// - /// Represents the current user. - /// Arbitrary user defined context represented as dictionary. - /// Represents a readonly dictionary of variable inputs to an executed document. - /// A set of policies names to authorize. - /// - Task Evaluate( - ClaimsPrincipal principal, - IDictionary userContext, - IReadOnlyDictionary inputs, - IEnumerable requiredPolicies); - } -} diff --git a/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs b/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs new file mode 100644 index 0000000..5355fcd --- /dev/null +++ b/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs @@ -0,0 +1,15 @@ +namespace GraphQL.Authorization +{ + /// + /// A type which can provide a . + /// + public interface IAuthorizationPolicyProvider + { + /// + /// Gets a from the given . + /// + /// The policy name to retrieve. + /// The named . + IAuthorizationPolicy GetPolicy(string policyName); + } +} diff --git a/src/GraphQL.Authorization/IAuthorizationService.cs b/src/GraphQL.Authorization/IAuthorizationService.cs new file mode 100644 index 0000000..fa95034 --- /dev/null +++ b/src/GraphQL.Authorization/IAuthorizationService.cs @@ -0,0 +1,16 @@ +using System.Threading.Tasks; + +namespace GraphQL.Authorization +{ + /// + /// Interface to evaluate the authorization result. + /// + public interface IAuthorizationService + { + /// + /// Evaluates authorization result. + /// + /// Provides context information to evaluate the authorization result. + Task AuthorizeAsync(IAuthorizationContext context); + } +} diff --git a/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs new file mode 100644 index 0000000..701d39c --- /dev/null +++ b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs @@ -0,0 +1,18 @@ +using System.Security.Claims; +using GraphQL.Validation; + +namespace GraphQL.Authorization +{ + /// + /// Provides access to the used for GraphQL operation authorization. + /// + public interface IClaimsPrincipalAccessor + { + /// + /// Provides the for the current + /// + /// The of the current operation + /// + ClaimsPrincipal GetClaimsPrincipal(ValidationContext context); + } +} diff --git a/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs b/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs index 75dffe0..92f1b0d 100644 --- a/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs @@ -5,19 +5,17 @@ namespace GraphQL.Authorization { /// /// Implements an which requires that - /// current user from must be authenticated. + /// current user from must be authenticated. /// public class AuthenticatedUserRequirement : IAuthorizationRequirement { internal static readonly AuthenticatedUserRequirement Instance = new AuthenticatedUserRequirement(); /// - public Task Authorize(AuthorizationContext context) + public Task Authorize(IAuthorizationContext context) { - if (context.User == null || !context.User.Identities.Any(x => x.IsAuthenticated)) - { - context.ReportError("An authenticated user is required."); - } + if (context.User != null && context.User.Identities.Any(x => x.IsAuthenticated)) + context.Succeed(this); return Task.CompletedTask; } diff --git a/src/GraphQL.Authorization/Requirements/ClaimAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/ClaimAuthorizationRequirement.cs deleted file mode 100644 index 395d1e1..0000000 --- a/src/GraphQL.Authorization/Requirements/ClaimAuthorizationRequirement.cs +++ /dev/null @@ -1,97 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; - -namespace GraphQL.Authorization -{ - /// - /// Implements an which requires an instance of the specified - /// claim type, and, if allowed values are specified, the claim value must be any of the allowed values. - /// - public class ClaimAuthorizationRequirement : IAuthorizationRequirement - { - private readonly string _claimType; - private readonly IEnumerable _displayValues; - private readonly IEnumerable _allowedValues; - - /// - /// Creates a new instance of with - /// the specified claim type. - /// - public ClaimAuthorizationRequirement(string claimType) - : this(claimType, (IEnumerable)null, null) - { - } - - /// - /// Creates a new instance of with - /// the specified claim type and optional list of claim values, which, if present, - /// the claim must match. - /// - public ClaimAuthorizationRequirement(string claimType, IEnumerable allowedValues) - : this(claimType, allowedValues, null) - { - } - - /// - /// Creates a new instance of with - /// the specified claim type and optional list of claim values, which, if present, - /// the claim must match. - /// - public ClaimAuthorizationRequirement(string claimType, params string[] allowedValues) - : this(claimType, allowedValues, null) - { - } - - /// - /// Creates a new instance of with - /// the specified claim type and optional list of claim values, which, if present, - /// the claim must match. Additional argument - /// specifies the set of displayed claim values that will be used to generate an - /// error message if the requirement is not met. - /// - public ClaimAuthorizationRequirement(string claimType, IEnumerable allowedValues, IEnumerable displayValues) - { - _claimType = claimType ?? throw new ArgumentNullException(nameof(claimType)); - _allowedValues = allowedValues ?? Enumerable.Empty(); - _displayValues = displayValues; - } - - /// - public Task Authorize(AuthorizationContext context) - { - bool found = false; - - if (context.User != null) - { - if (_allowedValues == null || !_allowedValues.Any()) - { - found = context.User.Claims.Any( - claim => string.Equals(claim.Type, _claimType, StringComparison.OrdinalIgnoreCase)); - } - else - { - found = context.User.Claims.Any( - claim => string.Equals(claim.Type, _claimType, StringComparison.OrdinalIgnoreCase) - && _allowedValues.Contains(claim.Value, StringComparer.Ordinal)); - } - } - - if (!found) - { - if (_allowedValues != null && _allowedValues.Any()) - { - string values = string.Join(", ", _displayValues ?? _allowedValues); - context.ReportError($"Required claim '{_claimType}' with any value of '{values}' is not present."); - } - else - { - context.ReportError($"Required claim '{_claimType}' is not present."); - } - } - - return Task.CompletedTask; - } - } -} diff --git a/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs new file mode 100644 index 0000000..6267942 --- /dev/null +++ b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs @@ -0,0 +1,99 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +namespace GraphQL.Authorization +{ + /// + /// Implements an which requires an instance of the specified + /// claim type, and, if allowed values are specified, the claim value must be any of the allowed values. + /// + public class ClaimsAuthorizationRequirement : IAuthorizationRequirement + { + /// + /// Creates a new instance of with + /// the specified claim type. + /// + public ClaimsAuthorizationRequirement(string claimType) + : this(claimType, (IEnumerable)null, null) + { + } + + /// + /// Creates a new instance of with + /// the specified claim type and optional list of claim values, which, if present, + /// the claim must match. + /// + public ClaimsAuthorizationRequirement(string claimType, IEnumerable allowedValues) + : this(claimType, allowedValues, null) + { + } + + /// + /// Creates a new instance of with + /// the specified claim type and optional list of claim values, which, if present, + /// the claim must match. + /// + public ClaimsAuthorizationRequirement(string claimType, params string[] allowedValues) + : this(claimType, allowedValues, null) + { + } + + /// + /// Creates a new instance of with + /// the specified claim type and optional list of claim values, which, if present, + /// the claim must match. Additional argument + /// specifies the set of displayed claim values that should be used to generate an + /// error message if the requirement is not met. + /// + public ClaimsAuthorizationRequirement(string claimType, IEnumerable allowedValues, IEnumerable displayValues) + { + ClaimType = claimType ?? throw new ArgumentNullException(nameof(claimType)); + AllowedValues = allowedValues ?? Enumerable.Empty(); + DisplayValues = displayValues; + } + + /// + /// Gets the claim type that must be present. + /// + public string ClaimType { get; } + + /// + /// Gets the optional list of claim values, which, if present, the claim must match. + /// + public IEnumerable AllowedValues { get; } + + /// + /// Specifies the set of displayed claim values that should be used to generate an + /// error message if the requirement is not met. + /// + public IEnumerable DisplayValues { get; } + + /// + public Task Authorize(IAuthorizationContext context) + { + bool found = false; + + if (context.User != null) + { + if (AllowedValues == null || !AllowedValues.Any()) + { + found = context.User.Claims.Any( + claim => string.Equals(claim.Type, ClaimType, StringComparison.OrdinalIgnoreCase)); + } + else + { + found = context.User.Claims.Any( + claim => string.Equals(claim.Type, ClaimType, StringComparison.OrdinalIgnoreCase) + && AllowedValues.Contains(claim.Value, StringComparer.Ordinal)); + } + } + + if (found) + context.Succeed(this); + + return Task.CompletedTask; + } + } +} diff --git a/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs index 443057d..2835524 100644 --- a/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs @@ -9,9 +9,9 @@ namespace GraphQL.Authorization public interface IAuthorizationRequirement { /// - /// Execute requirement. If the requirement is not met then this method - /// should call . + /// Execute requirement. If the requirement is met then this method + /// should call . /// - Task Authorize(AuthorizationContext context); + Task Authorize(IAuthorizationContext context); } } diff --git a/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs new file mode 100644 index 0000000..7ad612e --- /dev/null +++ b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs @@ -0,0 +1,30 @@ +using System.Threading.Tasks; + +namespace GraphQL.Authorization +{ + /// + /// Implements an which requires that + /// the specified policy must be defined. + /// + public class PolicyDefinedRequirement : IAuthorizationRequirement + { + /// + /// Creates a new instance of with + /// the specified (undefined) policy name. + /// + public PolicyDefinedRequirement(string policyName) + { + PolicyName = policyName; + } + + /// + /// Gets name of the undefined policy. + /// + public string PolicyName { get; } + + /// + /// Execute requirement. This requirement always isn't met by design. + /// + public Task Authorize(IAuthorizationContext context) => Task.CompletedTask; + } +} diff --git a/src/Harness/GraphQLAuthExtensions.cs b/src/Harness/GraphQLAuthExtensions.cs index 1e353b7..53f5149 100644 --- a/src/Harness/GraphQLAuthExtensions.cs +++ b/src/Harness/GraphQLAuthExtensions.cs @@ -17,14 +17,14 @@ public static class GraphQLAuthExtensions /// public static void AddGraphQLAuth(this IServiceCollection services, Action configure) { - services.TryAddSingleton(); - services.AddTransient(); - - services.TryAddTransient(s => + services.TryAddSingleton(); + services.TryAddSingleton(); + services.TryAddSingleton(); + services.TryAddSingleton(provider => { var authSettings = new AuthorizationSettings(); - configure(authSettings, s); - return authSettings; + configure(authSettings, provider); + return new DefaultAuthorizationPolicyProvider(authSettings); }); } @@ -33,16 +33,6 @@ public static void AddGraphQLAuth(this IServiceCollection services, Action public static void AddGraphQLAuth(this IServiceCollection services, Action configure) - { - services.TryAddSingleton(); - services.AddTransient(); - - services.TryAddTransient(s => - { - var authSettings = new AuthorizationSettings(); - configure(authSettings); - return authSettings; - }); - } + => services.AddGraphQLAuth((settings, _) => configure(settings)); } } From 77791f2d649973e6bab9bc0a0a88096c635647c2 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 13:59:46 +0300 Subject: [PATCH 02/22] apply format --- src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs index bffe7e8..4039227 100644 --- a/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs +++ b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs @@ -1,4 +1,4 @@ -using System.Security.Claims; +using System.Security.Claims; using GraphQL.Validation; namespace GraphQL.Authorization From d0224d80717a2b5aaa681f8b36acbd7b4841403c Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 14:02:11 +0300 Subject: [PATCH 03/22] 1 --- src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs index 701d39c..37b5b53 100644 --- a/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs +++ b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs @@ -4,14 +4,14 @@ namespace GraphQL.Authorization { /// - /// Provides access to the used for GraphQL operation authorization. + /// Provides access to the used when authorizing a GraphQL operation. /// public interface IClaimsPrincipalAccessor { /// - /// Provides the for the current + /// Provides the for the current . /// - /// The of the current operation + /// The of the current operation. /// ClaimsPrincipal GetClaimsPrincipal(ValidationContext context); } From cc5c5c7665f94b6ec6216765be185f85cc4ca080 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 14:04:26 +0300 Subject: [PATCH 04/22] rename --- .../GraphQL.Authorization.approved.txt | 12 ++++++------ src/GraphQL.Authorization/AuthorizationError.cs | 4 ++-- .../AuthorizationValidationRule.cs | 2 +- .../Requirements/PolicyExistsRequirement.cs | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index d86f2cf..af480a6 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -100,6 +100,12 @@ namespace GraphQL.Authorization public DefaultClaimsPrincipalAccessor() { } public System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } } + public class DefinedPolicyRequirement : GraphQL.Authorization.IAuthorizationRequirement + { + public DefinedPolicyRequirement(string policyName) { } + public string PolicyName { get; } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } + } public class GraphQLAuthorizeAttribute : GraphQL.GraphQLAttribute { public GraphQLAuthorizeAttribute() { } @@ -143,10 +149,4 @@ namespace GraphQL.Authorization { System.Security.Claims.ClaimsPrincipal User { get; } } - public class PolicyDefinedRequirement : GraphQL.Authorization.IAuthorizationRequirement - { - public PolicyDefinedRequirement(string policyName) { } - public string PolicyName { get; } - public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } - } } \ No newline at end of file diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs index 50f28f2..5a4b2c7 100644 --- a/src/GraphQL.Authorization/AuthorizationError.cs +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -87,8 +87,8 @@ public static void AppendFailureLine(StringBuilder error, IAuthorizationRequirem switch (authorizationRequirement) { - case PolicyDefinedRequirement policyExistsRequirement: - error.Append($"Required policy '{policyExistsRequirement.PolicyName}' is not present."); + case DefinedPolicyRequirement definedPolicyRequirement: + error.Append($"Required policy '{definedPolicyRequirement.PolicyName}' is not present."); break; case AuthenticatedUserRequirement _: diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index fc257f0..53e1e84 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -85,7 +85,7 @@ protected virtual IAuthorizationContext CreateAuthorizationContext(ValidationCon throw new ArgumentNullException(nameof(policyName)); return new DefaultAuthorizationContext( - _policyProvider.GetPolicy(policyName) ?? new AuthorizationPolicy(new PolicyDefinedRequirement(policyName)), + _policyProvider.GetPolicy(policyName) ?? new AuthorizationPolicy(new DefinedPolicyRequirement(policyName)), _claimsPrincipalAccessor.GetClaimsPrincipal(context) ?? new ClaimsPrincipal(new ClaimsIdentity())) { UserContext = context.UserContext, diff --git a/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs index 7ad612e..635cace 100644 --- a/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs @@ -6,13 +6,13 @@ namespace GraphQL.Authorization /// Implements an which requires that /// the specified policy must be defined. /// - public class PolicyDefinedRequirement : IAuthorizationRequirement + public class DefinedPolicyRequirement : IAuthorizationRequirement { /// - /// Creates a new instance of with + /// Creates a new instance of with /// the specified (undefined) policy name. /// - public PolicyDefinedRequirement(string policyName) + public DefinedPolicyRequirement(string policyName) { PolicyName = policyName; } From 73010f1cc3e3034e5bbb039a08c45f09182c6bc1 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 17:58:10 +0300 Subject: [PATCH 05/22] update readme --- src/BasicSample/Program.cs | 2 +- .../AuthorizationError.cs | 2 +- .../AuthorizationPolicyBuilder.cs | 50 --------------- .../AuthorizationPolicyBuilderExtensions.cs | 63 +++++++++++++++++++ src/Harness/Startup.cs | 2 +- 5 files changed, 66 insertions(+), 53 deletions(-) create mode 100644 src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs diff --git a/src/BasicSample/Program.cs b/src/BasicSample/Program.cs index 1bdea9a..41811e7 100644 --- a/src/BasicSample/Program.cs +++ b/src/BasicSample/Program.cs @@ -24,7 +24,7 @@ private static async Task Main() .AddSingleton(provider => { var authSettings = new AuthorizationSettings(); - authSettings.AddPolicy("AdminPolicy", p => p.RequireClaim("role", "Admin")); + authSettings.AddPolicy("AdminPolicy", b => b.RequireClaim("role", "Admin")); return new DefaultAuthorizationPolicyProvider(authSettings); }) .BuildServiceProvider(); diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs index 5a4b2c7..44ff277 100644 --- a/src/GraphQL.Authorization/AuthorizationError.cs +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -73,7 +73,7 @@ public static void AppendFailureHeader(StringBuilder error, OperationType? opera { error.Append("You are not authorized to run this ") .Append(GetOperationType(operationType)) - .Append("."); + .Append('.'); } /// diff --git a/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs b/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs index b283076..4f985c3 100644 --- a/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs +++ b/src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs @@ -5,7 +5,6 @@ namespace GraphQL.Authorization { /// /// Configures and then builds authorization policy from various authorization requirements. - /// Provides fluent API. /// public class AuthorizationPolicyBuilder { @@ -17,55 +16,6 @@ public class AuthorizationPolicyBuilder /// Created policy. public AuthorizationPolicy Build() => new AuthorizationPolicy(_requirements); - /// - /// Adds with the specified claim type. - /// - /// Type of the claim. - /// Reference to the same builder. - public AuthorizationPolicyBuilder RequireClaim(string claimType) - { - _requirements.Add(new ClaimsAuthorizationRequirement(claimType)); - return this; - } - - /// - /// Adds with the specified claim type and allowed values. - /// - /// Type of the claim. - /// Allowed values for this claim. - /// Reference to the same builder. - public AuthorizationPolicyBuilder RequireClaim(string claimType, params string[] allowedValues) - { - _requirements.Add(new ClaimsAuthorizationRequirement(claimType, allowedValues)); - return this; - } - - /// - /// Adds with the specified claim type, allowed values and display values. - /// - /// Type of the claim. - /// Allowed values for this claim. - /// - /// Display values for this claim. If no allowed claims are found, display values should be used to generate - /// an error message if the requirement is not met. - /// - /// Reference to the same builder. - public AuthorizationPolicyBuilder RequireClaim(string claimType, IEnumerable allowedValues, IEnumerable displayValues) - { - _requirements.Add(new ClaimsAuthorizationRequirement(claimType, allowedValues, displayValues)); - return this; - } - - /// - /// Adds . - /// - /// Reference to the same builder. - public AuthorizationPolicyBuilder RequireAuthenticatedUser() - { - _requirements.Add(AuthenticatedUserRequirement.Instance); - return this; - } - /// /// Adds specified authorization requirement. /// diff --git a/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs new file mode 100644 index 0000000..08c69ea --- /dev/null +++ b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; + +namespace GraphQL.Authorization +{ + /// + /// Extension methods for . + /// + public static class AuthorizationPolicyBuilderExtensions + { + /// + /// Adds with the specified claim type. + /// + /// Authorization policy builder to add requirement to. + /// Type of the claim. + /// Reference to the same builder. + public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType) + { + builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType)); + return builder; + } + + /// + /// Adds with the specified claim type and allowed values. + /// + /// Authorization policy builder to add requirement to. + /// Type of the claim. + /// Allowed values for this claim. + /// Reference to the same builder. + public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType, params string[] allowedValues) + { + builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues)); + return builder; + } + + /// + /// Adds with the specified claim type, allowed values and display values. + /// + /// Authorization policy builder to add requirement to. + /// Type of the claim. + /// Allowed values for this claim. + /// + /// Display values for this claim. If no allowed claims are found, display values should be used to generate + /// an error message if the requirement is not met. + /// + /// Reference to the same builder. + public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType, IEnumerable allowedValues, IEnumerable displayValues) + { + builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues, displayValues)); + return builder; + } + + /// + /// Adds . + /// + /// Authorization policy builder to add requirement to. + /// Reference to the same builder. + public static AuthorizationPolicyBuilder RequireAuthenticatedUser(this AuthorizationPolicyBuilder builder) + { + builder.AddRequirement(AuthenticatedUserRequirement.Instance); + return builder; + } + } +} diff --git a/src/Harness/Startup.cs b/src/Harness/Startup.cs index e03465e..77fb99f 100644 --- a/src/Harness/Startup.cs +++ b/src/Harness/Startup.cs @@ -40,7 +40,7 @@ type Query { }); // extension method defined in this project - services.AddGraphQLAuth((settings, provider) => settings.AddPolicy("AdminPolicy", p => p.RequireClaim("role", "Admin"))); + services.AddGraphQLAuth((settings, provider) => settings.AddPolicy("AdminPolicy", b => b.RequireClaim("role", "Admin"))); // claims principal must look something like this to allow access // var user = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("role", "Admin") })); From e2a1b4471454b0e00a1948d6fcc5993d7ece0eb6 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 18:05:17 +0300 Subject: [PATCH 06/22] readme --- README.md | 157 ++++++++++++++++-- .../AuthorizationPolicyBuilderExtensions.cs | 2 +- 2 files changed, 145 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index d4b4bb0..23f1cad 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,9 @@ ![Size](https://img.shields.io/github/repo-size/graphql-dotnet/authorization) A toolset for authorizing access to graph types for [GraphQL.NET](https://github.com/graphql-dotnet/graphql-dotnet). +It provides a [validation rule](src/GraphQL.Authorization/AuthorizationValidationRule.cs) that checks all of the +Graph Types in the GraphQL operation (query/mutation/subscription) to see if they have authorization policies applied +to them and evaluates these policies if any. Provides the following packages: @@ -28,24 +31,32 @@ Provides the following packages: You can get all preview versions from [GitHub Packages](https://github.com/orgs/graphql-dotnet/packages?repo_name=authorization). Note that GitHub requires authentication to consume the feed. See [here](https://docs.github.com/en/free-pro-team@latest/packages/publishing-and-managing-packages/about-github-packages#authenticating-to-github-packages). -# Usage +## Usage -* Register the authorization classes in your DI container - `IAuthorizationEvaluator`, `AuthorizationSettings`, and the `AuthorizationValidationRule`. -* Provide a custom `UserContext` class that implements `IProvideClaimsPrincipal`. -* Add policies to the `AuthorizationSettings`. -* Apply a policy to a GraphType or Field (both implement `IProvideMetadata`): - - using `AuthorizeWith(string policy)` extension method - - or with `GraphQLAuthorize` attribute if using Schema + Handler syntax. -* The `AuthorizationValidationRule` will run and verify the policies based on the registered policies. -* You can write your own `IAuthorizationRequirement`. +1. Register the necessary authorization classes in your DI container: + - `IValidationRule/AuthorizationValidationRule` + - `IAuthorizationService/DefaultAuthorizationService` + - `IClaimsPrincipalAccessor/DefaultClaimsPrincipalAccessor` + - `IAuthorizationPolicyProvider/DefaultAuthorizationPolicyProvider` +2. If you use `DefaultClaimsPrincipalAccessor` then provide a custom `UserContext` class that implements `IProvideClaimsPrincipal`. +3. Add policies to the `AuthorizationSettings`. +4. Apply a policy to a `GraphType` or `FieldType` (both implement `IProvideMetadata`): + - using `AuthorizeWith(string policy)` extension method + - or with `GraphQLAuthorize` attribute if using Schema + Handler syntax. +5. The `AuthorizationValidationRule` will run and verify the policies based on the registered policies. +6. You can write your own `IAuthorizationRequirement` and an extension method to add this requirement to `AuthorizationPolicyBuilder`. -# Examples +## Examples + +#### Examples in this repository 1. Fully functional basic [Console sample](src/BasicSample/Program.cs). 2. Fully functional [ASP.NET Core sample](src/Harness/Program.cs). -3. GraphType first syntax - use `AuthorizeWith` extension method on `IGraphType` or `IFieldType`. +#### Add authorization policy [GraphType first syntax] + +Use `AuthorizeWith` extension method on `IGraphType` or `IFieldType`. ```csharp public class MyType : ObjectGraphType @@ -58,7 +69,9 @@ public class MyType : ObjectGraphType } ``` -4. Schema first syntax - use `GraphQLAuthorize` attribute on type, method or property. +#### Add authorization policy [Schema first syntax] + +Use `GraphQLAuthorize` attribute on type, method or property. ```csharp [GraphQLAuthorize(Policy = "MyPolicy")] @@ -75,6 +88,124 @@ public class MutationType } ``` -# Known Issues +#### Custom authorization requirement + +You can add your own requirements to the authorization framework to extend it. +Create your own `IAuthorizationRequirement` class and add that requirement to your policy. + +```csharp +public class OnlyMondayRequirement : IAuthorizationRequirement +{ + public Task Authorize(IAuthorizationContext context) + { + if (DateTime.Now.DayOfWeek == DayOfWeek.Monday) + context.Succeed(this); + } +} + +public static class MyAuthorizationPolicyBuilderExtensions +{ + public static AuthorizationPolicyBuilder RequireMonday(this AuthorizationPolicyBuilder builder) + { + builder.AddRequirement(new OnlyMondayRequirement()); + return builder; + } +} + +public static void ConfigureAuthorizationServices(ServiceCollection services) +{ + services + .AddSingleton() + .AddSingleton() + .AddSingleton() + .AddSingleton(provider => + { + var authSettings = new AuthorizationSettings(); + authSettings.AddPolicy("MyPolicy", b => b.RequireMonday()); + return new DefaultAuthorizationPolicyProvider(authSettings); + }) +} +``` + +#### How to change error messages + +All authorization requirements (built-in or custom ones) only check the compliance of +the current execution state to their criteria. If the requirement is satisfied, then +it is marked as 'passed' and the next requirement is checked. If all requirements are +satisfied, then the validation rule returns a successful result. Otherwise for each +unsatisfied requirement, the validation rule will add an authorization error in the +`ValidationContext`. The text of this error may not suit you, especially if you write +your own authorization requirements. In this case, you can override the default behavior. + +**Option 1.** Create a descendant from `AuthorizationValidationRule` and override `AddValidationError` method. + +```csharp +public class CustomAuthorizationValidationRule : AuthorizationValidationRule +{ + public CustomAuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) + : base(authorizationService, claimsPrincipalAccessor, policyProvider) + { + } + + protected override void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + { + if (result.Failure.FailedRequirements.Any(r => r is MySpecialRequirement)) + context.ReportError(new AuthorizationError(node, context, "My special error message", result)); + else + base.AddValidationError(node, context, operationType, result); + } +} +``` + +Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValidationRule` in your DI container. + +**Option 2.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from the main GraphQL.NET +repository. For convenience you may use `ErrorInfoProvider` base class. + +```csharp +public class CustomErrorInfoProvider : ErrorInfoProvider +{ + public override ErrorInfo GetInfo(ExecutionError executionError) + { + var info = base.GetInfo(executionError); + info.Message = executionError switch + { + AuthorizationError authorizationError => GetAuthorizationErrorMessage(authorizationError), + _ => info.Message, + }; + return info; + } + + private string GetAuthorizationErrorMessage(AuthorizationError error) + { + var errorMessage = new StringBuilder(); + AuthorizationError.AppendFailureHeader(errorMessage, error.OperationType); + + foreach (var failedRequirement in error.AuthorizationResult.Failure.FailedRequirements) + { + switch (failedRequirement) + { + case OnlyMondayRequirement onlyMondayRequirement: + errorMessage.AppendLine(); + errorMessage.Append("Access is allowed only on Mondays."); + break; + default: + AuthorizationError.AppendFailureLine(errorMessage, failedRequirement); + break; + } + } + + return errorMessage.ToString(); + } +} +``` + +Then register `CustomErrorInfoProvider` in your DI container. + +```csharp +services.AddSingleton(); +``` + +## Known Issues * It is currently not possible to add a policy to Input objects using Schema first approach. diff --git a/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs index 08c69ea..932442d 100644 --- a/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs +++ b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace GraphQL.Authorization { From aa5c080442f036edcbc0ad8b969a4c8235923e11 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 20:26:55 +0300 Subject: [PATCH 07/22] 1 --- README.md | 4 ++-- src/BasicSample/Program.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 23f1cad..ac99b4c 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,8 @@ A toolset for authorizing access to graph types for [GraphQL.NET](https://github.com/graphql-dotnet/graphql-dotnet). It provides a [validation rule](src/GraphQL.Authorization/AuthorizationValidationRule.cs) that checks all of the -Graph Types in the GraphQL operation (query/mutation/subscription) to see if they have authorization policies applied -to them and evaluates these policies if any. +Graph Types in the given GraphQL operation (query/mutation/subscription) to see if they have authorization policies +applied to them and evaluates these policies if any. Provides the following packages: diff --git a/src/BasicSample/Program.cs b/src/BasicSample/Program.cs index 41811e7..bbf05a9 100644 --- a/src/BasicSample/Program.cs +++ b/src/BasicSample/Program.cs @@ -43,9 +43,9 @@ type Query { var schema = Schema.For(definitions, builder => builder.Types.Include()); var authorizedUser = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("role", "Admin") })); - var nonauthorizedUser = new ClaimsPrincipal(new ClaimsIdentity()); + var nonAuthorizedUser = new ClaimsPrincipal(new ClaimsIdentity()); - foreach (var principal in new[] { authorizedUser, nonauthorizedUser }) + foreach (var principal in new[] { authorizedUser, nonAuthorizedUser }) { string json = await schema.ExecuteAsync(options => { From 7ffa47e1b18265508bb0c57ee564d14e362559a3 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 20:52:44 +0300 Subject: [PATCH 08/22] NRT --- src/BasicSample/Program.cs | 6 +-- src/Directory.Build.props | 1 + .../GraphQL.Authorization.approved.txt | 51 ++++++++++--------- .../AuthorizationSchemaBuilderTests.cs | 2 +- .../AuthorizationServiceTests.cs | 10 ++-- .../AuthorizationValidationRuleTests.cs | 10 ++-- .../GraphQLUserContext.cs | 2 +- .../ValidationTestBase.cs | 2 +- .../ValidationTestConfig.cs | 8 +-- .../AuthorizationError.cs | 11 ++-- .../AuthorizationResult.cs | 2 +- .../AuthorizationSettings.cs | 4 +- .../AuthorizationValidationRule.cs | 10 ++-- .../DefaultAuthorizationContext.cs | 8 +-- .../DefaultAuthorizationPolicyProvider.cs | 2 +- .../DefaultClaimsPrincipalAccessor.cs | 2 +- .../GraphQLAuthorizeAttribute.cs | 2 +- .../IAuthorizationContext.cs | 6 +-- .../IAuthorizationPolicyProvider.cs | 2 +- .../IClaimsPrincipalAccessor.cs | 2 +- .../IProvideClaimsPrincipal.cs | 2 +- .../ClaimsAuthorizationRequirement.cs | 8 +-- src/Harness/GraphQL.cs | 6 +-- src/Harness/Startup.cs | 1 + 24 files changed, 84 insertions(+), 76 deletions(-) diff --git a/src/BasicSample/Program.cs b/src/BasicSample/Program.cs index bbf05a9..1bd996e 100644 --- a/src/BasicSample/Program.cs +++ b/src/BasicSample/Program.cs @@ -69,7 +69,7 @@ type Query { public class GraphQLUserContext : Dictionary, IProvideClaimsPrincipal { /// - public ClaimsPrincipal User { get; set; } + public ClaimsPrincipal? User { get; set; } } /// @@ -97,11 +97,11 @@ public class User /// /// Resolver for 'User.id' field. Just a simple property. /// - public string Id { get; set; } + public string? Id { get; set; } /// /// Resolver for 'User.name' field. Just a simple property. /// - public string Name { get; set; } + public string? Name { get; set; } } } diff --git a/src/Directory.Build.props b/src/Directory.Build.props index c0499c2..8da477c 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -16,6 +16,7 @@ embedded true true + enable diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index af480a6..39a774a 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -7,8 +7,8 @@ namespace GraphQL.Authorization } public class AuthorizationError : GraphQL.Validation.ValidationError { - public AuthorizationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } - public AuthorizationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, string message, GraphQL.Authorization.AuthorizationResult result) { } + public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } + public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, string message, GraphQL.Authorization.AuthorizationResult result) { } public virtual GraphQL.Authorization.AuthorizationResult AuthorizationResult { get; } public GraphQL.Language.AST.OperationType? OperationType { get; } public static void AppendFailureHeader(System.Text.StringBuilder error, GraphQL.Language.AST.OperationType? operationType) { } @@ -32,14 +32,17 @@ namespace GraphQL.Authorization public AuthorizationPolicyBuilder() { } public GraphQL.Authorization.AuthorizationPolicyBuilder AddRequirement(GraphQL.Authorization.IAuthorizationRequirement requirement) { } public GraphQL.Authorization.AuthorizationPolicy Build() { } - public GraphQL.Authorization.AuthorizationPolicyBuilder RequireAuthenticatedUser() { } - public GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(string claimType) { } - public GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(string claimType, params string[] allowedValues) { } - public GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(string claimType, System.Collections.Generic.IEnumerable allowedValues, System.Collections.Generic.IEnumerable displayValues) { } + } + public static class AuthorizationPolicyBuilderExtensions + { + public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireAuthenticatedUser(this GraphQL.Authorization.AuthorizationPolicyBuilder builder) { } + public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, string claimType) { } + public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, string claimType, params string[] allowedValues) { } + public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, string claimType, System.Collections.Generic.IEnumerable allowedValues, System.Collections.Generic.IEnumerable displayValues) { } } public class AuthorizationResult { - public GraphQL.Authorization.AuthorizationFailure Failure { get; } + public GraphQL.Authorization.AuthorizationFailure? Failure { get; } public bool Succeeded { get; } public static GraphQL.Authorization.AuthorizationResult Failed() { } public static GraphQL.Authorization.AuthorizationResult Failed(GraphQL.Authorization.AuthorizationFailure failure) { } @@ -52,12 +55,12 @@ namespace GraphQL.Authorization public void AddPolicy(string name, GraphQL.Authorization.IAuthorizationPolicy policy) { } public void AddPolicy(string name, System.Action configure) { } public System.Collections.Generic.IEnumerable GetPolicies(System.Collections.Generic.IEnumerable policies) { } - public GraphQL.Authorization.IAuthorizationPolicy GetPolicy(string name) { } + public GraphQL.Authorization.IAuthorizationPolicy? GetPolicy(string name) { } } public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule { public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationService authorizationService, GraphQL.Authorization.IClaimsPrincipalAccessor claimsPrincipalAccessor, GraphQL.Authorization.IAuthorizationPolicyProvider policyProvider) { } - protected virtual void AddValidationError(GraphQL.Language.AST.INode node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } + protected virtual void AddValidationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } protected virtual GraphQL.Authorization.IAuthorizationContext CreateAuthorizationContext(GraphQL.Validation.ValidationContext context, string policyName) { } public System.Threading.Tasks.Task ValidateAsync(GraphQL.Validation.ValidationContext context) { } } @@ -66,29 +69,29 @@ namespace GraphQL.Authorization public ClaimsAuthorizationRequirement(string claimType) { } public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues) { } public ClaimsAuthorizationRequirement(string claimType, params string[] allowedValues) { } - public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues, System.Collections.Generic.IEnumerable displayValues) { } - public System.Collections.Generic.IEnumerable AllowedValues { get; } + public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable? allowedValues, System.Collections.Generic.IEnumerable? displayValues) { } + public System.Collections.Generic.IEnumerable? AllowedValues { get; } public string ClaimType { get; } - public System.Collections.Generic.IEnumerable DisplayValues { get; } + public System.Collections.Generic.IEnumerable? DisplayValues { get; } public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } } public class DefaultAuthorizationContext : GraphQL.Authorization.IAuthorizationContext { - public DefaultAuthorizationContext(GraphQL.Authorization.IAuthorizationPolicy policy, System.Security.Claims.ClaimsPrincipal user) { } + public DefaultAuthorizationContext(GraphQL.Authorization.IAuthorizationPolicy policy, System.Security.Claims.ClaimsPrincipal? user) { } public virtual bool HasFailed { get; } public virtual bool HasSucceeded { get; } - public System.Collections.Generic.IReadOnlyDictionary Inputs { get; set; } + public System.Collections.Generic.IReadOnlyDictionary? Inputs { get; set; } public virtual System.Collections.Generic.IEnumerable PendingRequirements { get; } public GraphQL.Authorization.IAuthorizationPolicy Policy { get; } - public System.Security.Claims.ClaimsPrincipal User { get; } - public System.Collections.Generic.IDictionary UserContext { get; set; } + public System.Security.Claims.ClaimsPrincipal? User { get; } + public System.Collections.Generic.IDictionary? UserContext { get; set; } public virtual void Fail() { } public virtual void Succeed(GraphQL.Authorization.IAuthorizationRequirement requirement) { } } public class DefaultAuthorizationPolicyProvider : GraphQL.Authorization.IAuthorizationPolicyProvider { public DefaultAuthorizationPolicyProvider(GraphQL.Authorization.AuthorizationSettings settings) { } - public GraphQL.Authorization.IAuthorizationPolicy GetPolicy(string policyName) { } + public GraphQL.Authorization.IAuthorizationPolicy? GetPolicy(string policyName) { } } public class DefaultAuthorizationService : GraphQL.Authorization.IAuthorizationService { @@ -98,7 +101,7 @@ namespace GraphQL.Authorization public class DefaultClaimsPrincipalAccessor : GraphQL.Authorization.IClaimsPrincipalAccessor { public DefaultClaimsPrincipalAccessor() { } - public System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } + public System.Security.Claims.ClaimsPrincipal? GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } } public class DefinedPolicyRequirement : GraphQL.Authorization.IAuthorizationRequirement { @@ -117,11 +120,11 @@ namespace GraphQL.Authorization { bool HasFailed { get; } bool HasSucceeded { get; } - System.Collections.Generic.IReadOnlyDictionary Inputs { get; } + System.Collections.Generic.IReadOnlyDictionary? Inputs { get; } System.Collections.Generic.IEnumerable PendingRequirements { get; } GraphQL.Authorization.IAuthorizationPolicy Policy { get; } - System.Security.Claims.ClaimsPrincipal User { get; } - System.Collections.Generic.IDictionary UserContext { get; } + System.Security.Claims.ClaimsPrincipal? User { get; } + System.Collections.Generic.IDictionary? UserContext { get; } void Fail(); void Succeed(GraphQL.Authorization.IAuthorizationRequirement requirement); } @@ -131,7 +134,7 @@ namespace GraphQL.Authorization } public interface IAuthorizationPolicyProvider { - GraphQL.Authorization.IAuthorizationPolicy GetPolicy(string policyName); + GraphQL.Authorization.IAuthorizationPolicy? GetPolicy(string policyName); } public interface IAuthorizationRequirement { @@ -143,10 +146,10 @@ namespace GraphQL.Authorization } public interface IClaimsPrincipalAccessor { - System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context); + System.Security.Claims.ClaimsPrincipal? GetClaimsPrincipal(GraphQL.Validation.ValidationContext context); } public interface IProvideClaimsPrincipal { - System.Security.Claims.ClaimsPrincipal User { get; } + System.Security.Claims.ClaimsPrincipal? User { get; } } } \ No newline at end of file diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs index 368c212..2adb4b2 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs @@ -20,7 +20,7 @@ type Query { schema.Initialize(); - var query = schema.AllTypes["Query"] as IObjectGraphType; + var query = (IObjectGraphType)schema.AllTypes["Query"]; query.RequiresAuthorization().ShouldBeTrue(); query.GetPolicies().Single().ShouldBe("ClassPolicy"); diff --git a/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs index 557ea4c..cc79a15 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationServiceTests.cs @@ -19,12 +19,12 @@ public AuthorizationServiceTests() } private IAuthorizationContext CreateAuthorizationContext( - ClaimsPrincipal principal, - IDictionary userContext, - IReadOnlyDictionary inputs, - string requiredPolicy) + ClaimsPrincipal? principal, + IDictionary? userContext, + IReadOnlyDictionary? inputs, + string? requiredPolicy) { - return new DefaultAuthorizationContext(new DefaultAuthorizationPolicyProvider(_settings).GetPolicy(requiredPolicy), principal) + return new DefaultAuthorizationContext(new DefaultAuthorizationPolicyProvider(_settings).GetPolicy(requiredPolicy!)!, principal) { UserContext = userContext, Inputs = inputs, diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index 024df26..a1809f8 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -230,17 +230,17 @@ type Post { public class NestedQueryWithAttributes { [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "test")] - public Post Post(string id) => null; + public Post? Post(string id) => null; - public IEnumerable Posts() => null; + public IEnumerable? Posts() => null; - public IEnumerable PostsNonNull() => null; + public IEnumerable? PostsNonNull() => null; } [GraphQLAuthorize(Policy = "PostPolicy")] public class Post { - public string Id { get; set; } + public string? Id { get; set; } } public class PostGraphType : ObjectGraphType @@ -253,7 +253,7 @@ public PostGraphType() public class Author { - public string Name { get; set; } + public string? Name { get; set; } } private static ISchema TypedSchema() diff --git a/src/GraphQL.Authorization.Tests/GraphQLUserContext.cs b/src/GraphQL.Authorization.Tests/GraphQLUserContext.cs index a852fd1..2c41e61 100644 --- a/src/GraphQL.Authorization.Tests/GraphQLUserContext.cs +++ b/src/GraphQL.Authorization.Tests/GraphQLUserContext.cs @@ -5,6 +5,6 @@ namespace GraphQL.Authorization.Tests { internal class GraphQLUserContext : Dictionary, IProvideClaimsPrincipal { - public ClaimsPrincipal User { get; set; } + public ClaimsPrincipal? User { get; set; } } } diff --git a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs index 8c3ef1b..f8d9a00 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestBase.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestBase.cs @@ -66,7 +66,7 @@ private static IValidationResult Validate(ValidationTestConfig config) return validator.ValidateAsync(config.Schema, document, document.Operations.First().Variables, config.Rules, userContext, config.Inputs).GetAwaiter().GetResult().validationResult; } - internal static ClaimsPrincipal CreatePrincipal(string authenticationType = null, IDictionary claims = null) + internal static ClaimsPrincipal CreatePrincipal(string? authenticationType = null, IDictionary? claims = null) { var claimsList = new List(); diff --git a/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs b/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs index 9dec7b4..cc7cb5f 100644 --- a/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs +++ b/src/GraphQL.Authorization.Tests/ValidationTestConfig.cs @@ -8,15 +8,15 @@ namespace GraphQL.Authorization.Tests { public class ValidationTestConfig { - public string Query { get; set; } + public string? Query { get; set; } - public ISchema Schema { get; set; } + public ISchema Schema { get; set; } = null!; public List Rules { get; set; } = new List(); - public ClaimsPrincipal User { get; set; } + public ClaimsPrincipal? User { get; set; } - public Inputs Inputs { get; set; } + public Inputs? Inputs { get; set; } public Action ValidateResult = _ => { }; } diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs index 44ff277..8bd1d57 100644 --- a/src/GraphQL.Authorization/AuthorizationError.cs +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -14,7 +14,7 @@ public class AuthorizationError : ValidationError /// /// Initializes a new instance of the class for a specified authorization result. /// - public AuthorizationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + public AuthorizationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) : this(node, context, GenerateMessage(operationType, result), result) { OperationType = operationType; @@ -23,7 +23,7 @@ public AuthorizationError(INode node, ValidationContext context, OperationType? /// /// Initializes a new instance of the class for a specified authorization result with a specific error message. /// - public AuthorizationError(INode node, ValidationContext context, string message, AuthorizationResult result) + public AuthorizationError(INode? node, ValidationContext context, string message, AuthorizationResult result) : base(context.Document.OriginalQuery, "6.1.1", message, node == null ? Array.Empty() : new INode[] { node }) { Code = "authorization"; @@ -45,9 +45,12 @@ private static string GenerateMessage(OperationType? operationType, Authorizatio var error = new StringBuilder(); AppendFailureHeader(error, operationType); - foreach (var failure in result.Failure.FailedRequirements) + if (result.Failure != null) { - AppendFailureLine(error, failure); + foreach (var failure in result.Failure.FailedRequirements) + { + AppendFailureLine(error, failure); + } } return error.ToString(); diff --git a/src/GraphQL.Authorization/AuthorizationResult.cs b/src/GraphQL.Authorization/AuthorizationResult.cs index c5238a7..b477dab 100644 --- a/src/GraphQL.Authorization/AuthorizationResult.cs +++ b/src/GraphQL.Authorization/AuthorizationResult.cs @@ -18,7 +18,7 @@ private AuthorizationResult() { } /// /// Contains information about why authorization failed. /// - public AuthorizationFailure Failure { get; private set; } + public AuthorizationFailure? Failure { get; private set; } /// /// Creates successful authorization result. diff --git a/src/GraphQL.Authorization/AuthorizationSettings.cs b/src/GraphQL.Authorization/AuthorizationSettings.cs index 89a2b44..2a5ddfe 100644 --- a/src/GraphQL.Authorization/AuthorizationSettings.cs +++ b/src/GraphQL.Authorization/AuthorizationSettings.cs @@ -24,7 +24,7 @@ public class AuthorizationSettings /// Policies with matched names. public IEnumerable GetPolicies(IEnumerable policies) { - List found = null; + List? found = null; if (policies != null) { @@ -44,7 +44,7 @@ public IEnumerable GetPolicies(IEnumerable policie /// /// Name of the required policy. /// Required policy if exists, otherwise . - public IAuthorizationPolicy GetPolicy(string name) => name == null ? null : _policies.TryGetValue(name, out var policy) ? policy : null; + public IAuthorizationPolicy? GetPolicy(string name) => name == null ? null : _policies.TryGetValue(name, out var policy) ? policy : null; /// /// Adds a policy with the specified name. If a policy with that name already exists then it will be replaced. diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index 53e1e84..a822764 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -24,9 +24,9 @@ public class AuthorizationValidationRule : IValidationRule /// public AuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) { - _authorizationService = authorizationService; - _claimsPrincipalAccessor = claimsPrincipalAccessor; - _policyProvider = policyProvider; + _authorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); + _claimsPrincipalAccessor = claimsPrincipalAccessor ?? throw new ArgumentNullException(nameof(claimsPrincipalAccessor)); + _policyProvider = policyProvider ?? throw new ArgumentNullException(nameof(policyProvider)); } /// @@ -93,7 +93,7 @@ protected virtual IAuthorizationContext CreateAuthorizationContext(ValidationCon }; } - private async Task AuthorizeAsync(INode node, IProvideMetadata provider, ValidationContext context, OperationType? operationType) + private async Task AuthorizeAsync(INode? node, IProvideMetadata provider, ValidationContext context, OperationType? operationType) { var policyNames = provider?.GetPolicies(); @@ -126,7 +126,7 @@ private async Task AuthorizeAsync(INode node, IProvideMetadata provider, Validat /// /// Adds an authorization failure error to the document response. /// - protected virtual void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + protected virtual void AddValidationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) { context.ReportError(new AuthorizationError(node, context, operationType, result)); } diff --git a/src/GraphQL.Authorization/DefaultAuthorizationContext.cs b/src/GraphQL.Authorization/DefaultAuthorizationContext.cs index 1068f19..635c11c 100644 --- a/src/GraphQL.Authorization/DefaultAuthorizationContext.cs +++ b/src/GraphQL.Authorization/DefaultAuthorizationContext.cs @@ -18,7 +18,7 @@ public class DefaultAuthorizationContext : IAuthorizationContext /// /// A checked policy for the current authorization processing. /// A representing the current user. - public DefaultAuthorizationContext(IAuthorizationPolicy policy, ClaimsPrincipal user) + public DefaultAuthorizationContext(IAuthorizationPolicy policy, ClaimsPrincipal? user) { Policy = policy ?? throw new ArgumentNullException(nameof(policy)); User = user; @@ -29,13 +29,13 @@ public DefaultAuthorizationContext(IAuthorizationPolicy policy, ClaimsPrincipal public IAuthorizationPolicy Policy { get; } /// - public ClaimsPrincipal User { get; } + public ClaimsPrincipal? User { get; } /// - public IDictionary UserContext { get; set; } + public IDictionary? UserContext { get; set; } /// - public IReadOnlyDictionary Inputs { get; set; } + public IReadOnlyDictionary? Inputs { get; set; } /// public virtual IEnumerable PendingRequirements { get; } diff --git a/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs b/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs index 1e008e2..a042d82 100644 --- a/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs +++ b/src/GraphQL.Authorization/DefaultAuthorizationPolicyProvider.cs @@ -18,6 +18,6 @@ public DefaultAuthorizationPolicyProvider(AuthorizationSettings settings) } /// - public IAuthorizationPolicy GetPolicy(string policyName) => _settings.GetPolicy(policyName); + public IAuthorizationPolicy? GetPolicy(string policyName) => _settings.GetPolicy(policyName); } } diff --git a/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs index 4039227..4a2dcc2 100644 --- a/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs +++ b/src/GraphQL.Authorization/DefaultClaimsPrincipalAccessor.cs @@ -9,6 +9,6 @@ namespace GraphQL.Authorization public class DefaultClaimsPrincipalAccessor : IClaimsPrincipalAccessor { /// - public ClaimsPrincipal GetClaimsPrincipal(ValidationContext context) => (context.UserContext as IProvideClaimsPrincipal)?.User; + public ClaimsPrincipal? GetClaimsPrincipal(ValidationContext context) => (context.UserContext as IProvideClaimsPrincipal)?.User; } } diff --git a/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs b/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs index c61aa66..57460b8 100644 --- a/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs +++ b/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs @@ -10,7 +10,7 @@ public class GraphQLAuthorizeAttribute : GraphQLAttribute /// /// The name of policy to apply. /// - public string Policy { get; set; } + public string Policy { get; set; } = null!; /// public override void Modify(TypeConfig type) => type.AuthorizeWith(Policy); diff --git a/src/GraphQL.Authorization/IAuthorizationContext.cs b/src/GraphQL.Authorization/IAuthorizationContext.cs index dc0c97c..646eb9f 100644 --- a/src/GraphQL.Authorization/IAuthorizationContext.cs +++ b/src/GraphQL.Authorization/IAuthorizationContext.cs @@ -16,17 +16,17 @@ public interface IAuthorizationContext /// /// Current user. /// - ClaimsPrincipal User { get; } + ClaimsPrincipal? User { get; } /// /// Arbitrary user defined context represented as a dictionary. /// - IDictionary UserContext { get; } + IDictionary? UserContext { get; } /// /// Represents a readonly dictionary of variable inputs to an executed document. /// - IReadOnlyDictionary Inputs { get; } + IReadOnlyDictionary? Inputs { get; } /// /// Gets the requirements that have not yet been marked as succeeded. diff --git a/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs b/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs index 5355fcd..7be483e 100644 --- a/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs +++ b/src/GraphQL.Authorization/IAuthorizationPolicyProvider.cs @@ -10,6 +10,6 @@ public interface IAuthorizationPolicyProvider /// /// The policy name to retrieve. /// The named . - IAuthorizationPolicy GetPolicy(string policyName); + IAuthorizationPolicy? GetPolicy(string policyName); } } diff --git a/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs index 37b5b53..f57f217 100644 --- a/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs +++ b/src/GraphQL.Authorization/IClaimsPrincipalAccessor.cs @@ -13,6 +13,6 @@ public interface IClaimsPrincipalAccessor /// /// The of the current operation. /// - ClaimsPrincipal GetClaimsPrincipal(ValidationContext context); + ClaimsPrincipal? GetClaimsPrincipal(ValidationContext context); } } diff --git a/src/GraphQL.Authorization/IProvideClaimsPrincipal.cs b/src/GraphQL.Authorization/IProvideClaimsPrincipal.cs index 06ab11f..a7bf3ee 100644 --- a/src/GraphQL.Authorization/IProvideClaimsPrincipal.cs +++ b/src/GraphQL.Authorization/IProvideClaimsPrincipal.cs @@ -11,6 +11,6 @@ public interface IProvideClaimsPrincipal /// /// Gets the current user. /// - ClaimsPrincipal User { get; } + ClaimsPrincipal? User { get; } } } diff --git a/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs index 6267942..04fa44d 100644 --- a/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs @@ -16,7 +16,7 @@ public class ClaimsAuthorizationRequirement : IAuthorizationRequirement /// the specified claim type. /// public ClaimsAuthorizationRequirement(string claimType) - : this(claimType, (IEnumerable)null, null) + : this(claimType, (IEnumerable?)null, null) { } @@ -47,7 +47,7 @@ public ClaimsAuthorizationRequirement(string claimType, params string[] allowedV /// specifies the set of displayed claim values that should be used to generate an /// error message if the requirement is not met. /// - public ClaimsAuthorizationRequirement(string claimType, IEnumerable allowedValues, IEnumerable displayValues) + public ClaimsAuthorizationRequirement(string claimType, IEnumerable? allowedValues, IEnumerable? displayValues) { ClaimType = claimType ?? throw new ArgumentNullException(nameof(claimType)); AllowedValues = allowedValues ?? Enumerable.Empty(); @@ -62,13 +62,13 @@ public ClaimsAuthorizationRequirement(string claimType, IEnumerable allo /// /// Gets the optional list of claim values, which, if present, the claim must match. /// - public IEnumerable AllowedValues { get; } + public IEnumerable? AllowedValues { get; } /// /// Specifies the set of displayed claim values that should be used to generate an /// error message if the requirement is not met. /// - public IEnumerable DisplayValues { get; } + public IEnumerable? DisplayValues { get; } /// public Task Authorize(IAuthorizationContext context) diff --git a/src/Harness/GraphQL.cs b/src/Harness/GraphQL.cs index 3d62bce..97992da 100644 --- a/src/Harness/GraphQL.cs +++ b/src/Harness/GraphQL.cs @@ -11,7 +11,7 @@ namespace Harness public class GraphQLUserContext : Dictionary, IProvideClaimsPrincipal { /// - public ClaimsPrincipal User { get; set; } + public ClaimsPrincipal? User { get; set; } } /// @@ -39,11 +39,11 @@ public class User /// /// Resolver for 'User.id' field. Just a simple property. /// - public string Id { get; set; } + public string? Id { get; set; } /// /// Resolver for 'User.name' field. Just a simple property. /// - public string Name { get; set; } + public string? Name { get; set; } } } diff --git a/src/Harness/Startup.cs b/src/Harness/Startup.cs index 77fb99f..9b5d042 100644 --- a/src/Harness/Startup.cs +++ b/src/Harness/Startup.cs @@ -1,4 +1,5 @@ using GraphQL; +using GraphQL.Authorization; using GraphQL.Server; using GraphQL.Types; using Microsoft.AspNetCore.Builder; From 65754ebdca46a86d7a7173cfb116e4c56a247c4f Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 23:25:34 +0300 Subject: [PATCH 09/22] note for ASP.NET Core users --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index ac99b4c..ce13a23 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,15 @@ Provides the following packages: You can get all preview versions from [GitHub Packages](https://github.com/orgs/graphql-dotnet/packages?repo_name=authorization). Note that GitHub requires authentication to consume the feed. See [here](https://docs.github.com/en/free-pro-team@latest/packages/publishing-and-managing-packages/about-github-packages#authenticating-to-github-packages). +## Note for ASP.NET Core users + +If you came here in search for GraphQL authorization for the ASP.NET Core applications, +then it makes sense to look into the [server](https://github.com/graphql-dotnet/server) project +and its [GraphQL.Server.Authorization.AspNetCore](https://www.nuget.org/packages/GraphQL.Server.Authorization.AspNetCore) +package. Although you will be able to integrate GraphQL authorization with the help of classes +from the current repository, the _GraphQL.Server.Authorization.AspNetCore_ package is much better +adapted to work within the ASP.NET Core applications. + ## Usage 1. Register the necessary authorization classes in your DI container: From 50deed376aae10c21b5d1ac88c278046b45d2ed0 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Tue, 23 Mar 2021 23:28:05 +0300 Subject: [PATCH 10/22] reformat --- README.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index ce13a23..c8c8667 100644 --- a/README.md +++ b/README.md @@ -47,13 +47,16 @@ adapted to work within the ASP.NET Core applications. - `IAuthorizationService/DefaultAuthorizationService` - `IClaimsPrincipalAccessor/DefaultClaimsPrincipalAccessor` - `IAuthorizationPolicyProvider/DefaultAuthorizationPolicyProvider` -2. If you use `DefaultClaimsPrincipalAccessor` then provide a custom `UserContext` class that implements `IProvideClaimsPrincipal`. +2. If you use `DefaultClaimsPrincipalAccessor` then provide a custom `UserContext` +class that implements `IProvideClaimsPrincipal`. 3. Add policies to the `AuthorizationSettings`. 4. Apply a policy to a `GraphType` or `FieldType` (both implement `IProvideMetadata`): - using `AuthorizeWith(string policy)` extension method - or with `GraphQLAuthorize` attribute if using Schema + Handler syntax. -5. The `AuthorizationValidationRule` will run and verify the policies based on the registered policies. -6. You can write your own `IAuthorizationRequirement` and an extension method to add this requirement to `AuthorizationPolicyBuilder`. +5. The `AuthorizationValidationRule` will run and verify the policies based on the +registered policies. +6. You can write your own `IAuthorizationRequirement` and an extension method to add +this requirement to `AuthorizationPolicyBuilder`. ## Examples @@ -146,7 +149,8 @@ unsatisfied requirement, the validation rule will add an authorization error in `ValidationContext`. The text of this error may not suit you, especially if you write your own authorization requirements. In this case, you can override the default behavior. -**Option 1.** Create a descendant from `AuthorizationValidationRule` and override `AddValidationError` method. +**Option 1.** Create a descendant from `AuthorizationValidationRule` and override +`AddValidationError` method. ```csharp public class CustomAuthorizationValidationRule : AuthorizationValidationRule @@ -166,10 +170,11 @@ public class CustomAuthorizationValidationRule : AuthorizationValidationRule } ``` -Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValidationRule` in your DI container. +Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValidationRule` +in your DI container. -**Option 2.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from the main GraphQL.NET -repository. For convenience you may use `ErrorInfoProvider` base class. +**Option 2.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from +the main GraphQL.NET repository. For convenience you may use `ErrorInfoProvider` base class. ```csharp public class CustomErrorInfoProvider : ErrorInfoProvider From bc6949316976b3601f636ad128133269dbf48214 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 00:47:10 +0300 Subject: [PATCH 11/22] add tests and new methods --- .../GraphQL.Authorization.approved.txt | 7 +++ .../AuthorizationSettingsTests.cs | 28 +++++++++++- .../AuthorizationValidationRuleTests.cs | 22 ++++++++++ .../AuthorizationPolicyBuilderExtensions.cs | 44 ++++++++++++------- .../Requirements/DelegatedRequirement.cs | 25 +++++++++++ 5 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 src/GraphQL.Authorization/Requirements/DelegatedRequirement.cs diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index 39a774a..68f9dc3 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -35,6 +35,8 @@ namespace GraphQL.Authorization } public static class AuthorizationPolicyBuilderExtensions { + public static GraphQL.Authorization.AuthorizationPolicyBuilder Require(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, System.Action action) { } + public static GraphQL.Authorization.AuthorizationPolicyBuilder Require(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, System.Func action) { } public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireAuthenticatedUser(this GraphQL.Authorization.AuthorizationPolicyBuilder builder) { } public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, string claimType) { } public static GraphQL.Authorization.AuthorizationPolicyBuilder RequireClaim(this GraphQL.Authorization.AuthorizationPolicyBuilder builder, string claimType, params string[] allowedValues) { } @@ -109,6 +111,11 @@ namespace GraphQL.Authorization public string PolicyName { get; } public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } } + public class DelegatedRequirement : GraphQL.Authorization.IAuthorizationRequirement + { + public DelegatedRequirement(System.Func action) { } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } + } public class GraphQLAuthorizeAttribute : GraphQL.GraphQLAttribute { public GraphQLAuthorizeAttribute() { } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index c8d8746..1a339d6 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -1,3 +1,4 @@ +using System; using System.Linq; using Shouldly; using Xunit; @@ -13,15 +14,38 @@ public AuthorizationSettingsTests() _settings = new AuthorizationSettings(); } + [Fact] + public void throw_if_add_null_requirement() + { + Should.Throw(() => _settings.AddPolicy("MyPolicy", builder => builder.AddRequirement(null!))); + } + [Fact] public void can_add_a_claim_policy() { - _settings.AddPolicy("MyPolicy", builder => builder.RequireClaim("Admin")); + _settings.AddPolicy("MyPolicy", builder => builder + .RequireClaim("Admin") + .RequireClaim("SuperAdmin", "Super1", "Super2") + .RequireClaim("SuperDuperAdmin", new[] { "Super1", "Super2" }, new[] { "Display1", "Display2" }) + ); + + _settings.Policies.Count().ShouldBe(1); + + var policy = _settings.Policies.Single(); + policy.Requirements.Count().ShouldBe(3); + policy.Requirements.ToList().ForEach(r => r.ShouldBeOfType()); + } + + [Fact] + public void can_add_authenticated_user_policy() + { + _settings.AddPolicy("MyPolicy", builder => builder.RequireAuthenticatedUser()); _settings.Policies.Count().ShouldBe(1); var policy = _settings.Policies.Single(); - policy.Requirements.Single().ShouldBeOfType(); + policy.Requirements.Count().ShouldBe(1); + policy.Requirements.Single().ShouldBeOfType(); } } } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index a1809f8..bd11173 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -1,6 +1,9 @@ +using System; using System.Collections.Generic; +using System.Linq; using GraphQL.Types; using GraphQL.Types.Relay.DataObjects; +using Shouldly; using Xunit; namespace GraphQL.Authorization.Tests @@ -185,6 +188,20 @@ public void fails_on_missing_claim_on_connection_type() }); } + [Fact] + public void fails_on_explicitly_called_fail() + { + Settings.AddPolicy("FailedPolicy", _ => _.Require(c => c.Fail())); + + ShouldFailRule(_ => + { + _.Query = @"query { failed }"; + _.Schema = TypedSchema(); + _.User = CreatePrincipal(); + _.ValidateResult = r => r.Errors.Single().Message.ShouldBe("You are not authorized to run this query."); + }); + } + private static ISchema BasicSchema() { string defs = @" @@ -276,6 +293,11 @@ private static ISchema TypedSchema() resolve: context => "testing" ).AuthorizeWith("AdminPolicy").AuthorizeWith("ConfidentialPolicy"); + query.Field( + "failed", + resolve: context => throw new NotSupportedException("Should never called") + ).AuthorizeWith("FailedPolicy"); + return new Schema { Query = query }; } diff --git a/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs index 932442d..39d8c6f 100644 --- a/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs +++ b/src/GraphQL.Authorization/AuthorizationPolicyBuilderExtensions.cs @@ -1,4 +1,6 @@ +using System; using System.Collections.Generic; +using System.Threading.Tasks; namespace GraphQL.Authorization { @@ -14,10 +16,7 @@ public static class AuthorizationPolicyBuilderExtensions /// Type of the claim. /// Reference to the same builder. public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType) - { - builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType)); - return builder; - } + => builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType)); /// /// Adds with the specified claim type and allowed values. @@ -27,10 +26,7 @@ public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBu /// Allowed values for this claim. /// Reference to the same builder. public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType, params string[] allowedValues) - { - builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues)); - return builder; - } + => builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues)); /// /// Adds with the specified claim type, allowed values and display values. @@ -44,10 +40,7 @@ public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBu /// /// Reference to the same builder. public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBuilder builder, string claimType, IEnumerable allowedValues, IEnumerable displayValues) - { - builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues, displayValues)); - return builder; - } + => builder.AddRequirement(new ClaimsAuthorizationRequirement(claimType, allowedValues, displayValues)); /// /// Adds . @@ -55,9 +48,28 @@ public static AuthorizationPolicyBuilder RequireClaim(this AuthorizationPolicyBu /// Authorization policy builder to add requirement to. /// Reference to the same builder. public static AuthorizationPolicyBuilder RequireAuthenticatedUser(this AuthorizationPolicyBuilder builder) - { - builder.AddRequirement(AuthenticatedUserRequirement.Instance); - return builder; - } + => builder.AddRequirement(AuthenticatedUserRequirement.Instance); + + /// + /// Adds with the specified delegate. + /// + /// Authorization policy builder to add requirement to. + /// Delegate to execute. + /// Reference to the same builder. + public static AuthorizationPolicyBuilder Require(this AuthorizationPolicyBuilder builder, Func action) + => builder.AddRequirement(new DelegatedRequirement(action)); + + /// + /// Adds with the specified delegate. + /// + /// Authorization policy builder to add requirement to. + /// Delegate to execute. + /// Reference to the same builder. + public static AuthorizationPolicyBuilder Require(this AuthorizationPolicyBuilder builder, Action action) + => builder.AddRequirement(new DelegatedRequirement(context => + { + action(context); + return Task.CompletedTask; + })); } } diff --git a/src/GraphQL.Authorization/Requirements/DelegatedRequirement.cs b/src/GraphQL.Authorization/Requirements/DelegatedRequirement.cs new file mode 100644 index 0000000..44605a2 --- /dev/null +++ b/src/GraphQL.Authorization/Requirements/DelegatedRequirement.cs @@ -0,0 +1,25 @@ +using System; +using System.Threading.Tasks; + +namespace GraphQL.Authorization +{ + /// + /// Implements an that calls the specified delegate. + /// + public class DelegatedRequirement : IAuthorizationRequirement + { + private readonly Func _action; + + /// + /// Creates a new instance of with + /// the specified delegate. + /// + public DelegatedRequirement(Func action) + { + _action = action; + } + + /// + public Task Authorize(IAuthorizationContext context) => _action(context); + } +} From cf219d72a2eed44556d5e720a38816a5db65857d Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 00:58:37 +0300 Subject: [PATCH 12/22] tests --- .../AuthorizationSettingsTests.cs | 34 +++++++++++++++++++ .../AuthorizationSettings.cs | 7 ++++ 2 files changed, 41 insertions(+) diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index 1a339d6..6855ec5 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Threading.Tasks; using Shouldly; using Xunit; @@ -14,6 +15,12 @@ public AuthorizationSettingsTests() _settings = new AuthorizationSettings(); } + [Fact] + public void throw_if_add_null_delegate() + { + Should.Throw(() => _settings.AddPolicy("MyPolicy", (Action)null!)); + } + [Fact] public void throw_if_add_null_requirement() { @@ -47,5 +54,32 @@ public void can_add_authenticated_user_policy() policy.Requirements.Count().ShouldBe(1); policy.Requirements.Single().ShouldBeOfType(); } + + [Fact] + public void can_add_policy_instance() + { + _settings.AddPolicy("MyPolicy", new AuthorizationPolicy(new DelegatedRequirement(c => Task.CompletedTask))); + + _settings.Policies.Count().ShouldBe(1); + + var policy = _settings.Policies.Single(); + policy.Requirements.Count().ShouldBe(1); + policy.Requirements.Single().ShouldBeOfType(); + } + + [Fact] + public void get_policies() + { + _settings.AddPolicy("MyPolicy", new AuthorizationPolicy(new DelegatedRequirement(c => Task.CompletedTask))); + + _settings.GetPolicies("a").ShouldBeEmpty(); + _settings.GetPolicies("a", "b").ShouldBeEmpty(); + _settings.GetPolicies(Enumerable.Empty()).ShouldBeEmpty(); + + _settings.GetPolicies("MyPolicy").Count().ShouldBe(1); + _settings.GetPolicies("a", "MyPolicy", "b").Count().ShouldBe(1); + + _settings.Policies.Count().ShouldBe(1); + } } } diff --git a/src/GraphQL.Authorization/AuthorizationSettings.cs b/src/GraphQL.Authorization/AuthorizationSettings.cs index 2a5ddfe..545f877 100644 --- a/src/GraphQL.Authorization/AuthorizationSettings.cs +++ b/src/GraphQL.Authorization/AuthorizationSettings.cs @@ -39,6 +39,13 @@ public IEnumerable GetPolicies(IEnumerable policie return found ?? Enumerable.Empty(); } + /// + /// Returns policies with the specified names. + /// + /// A set of policies names. + /// Policies with matched names. + public IEnumerable GetPolicies(params string[] policies) => GetPolicies((IEnumerable)policies); + /// /// Returns one policy with the specified name. /// From 3529db4ef532f7cbf2c0f280fbe42b30a4c63167 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 01:02:36 +0300 Subject: [PATCH 13/22] 1 --- .../GraphQL.Authorization.approved.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index 68f9dc3..8cec7e5 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -57,6 +57,7 @@ namespace GraphQL.Authorization public void AddPolicy(string name, GraphQL.Authorization.IAuthorizationPolicy policy) { } public void AddPolicy(string name, System.Action configure) { } public System.Collections.Generic.IEnumerable GetPolicies(System.Collections.Generic.IEnumerable policies) { } + public System.Collections.Generic.IEnumerable GetPolicies(params string[] policies) { } public GraphQL.Authorization.IAuthorizationPolicy? GetPolicy(string name) { } } public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule From 4409573033e581ac338fd0fa4e3e366f27019404 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 01:15:46 +0300 Subject: [PATCH 14/22] more tests --- .../AuthorizationSettingsTests.cs | 2 +- .../DefaultClaimsPrincipalAccessorTests.cs | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/GraphQL.Authorization.Tests/DefaultClaimsPrincipalAccessorTests.cs diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index 6855ec5..78963ef 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -64,7 +64,7 @@ public void can_add_policy_instance() var policy = _settings.Policies.Single(); policy.Requirements.Count().ShouldBe(1); - policy.Requirements.Single().ShouldBeOfType(); + policy.Requirements.Single().ShouldBeOfType(); } [Fact] diff --git a/src/GraphQL.Authorization.Tests/DefaultClaimsPrincipalAccessorTests.cs b/src/GraphQL.Authorization.Tests/DefaultClaimsPrincipalAccessorTests.cs new file mode 100644 index 0000000..ba71122 --- /dev/null +++ b/src/GraphQL.Authorization.Tests/DefaultClaimsPrincipalAccessorTests.cs @@ -0,0 +1,53 @@ +using System.Collections.Generic; +using System.Security.Claims; +using GraphQL.Validation; +using Shouldly; +using Xunit; + +namespace GraphQL.Authorization.Tests +{ + public class DefaultClaimsPrincipalAccessorTests + { + [Fact] + public void returns_null_from_null_user_context() + { + var accessor = new DefaultClaimsPrincipalAccessor(); + var context = new ValidationContext(); + accessor.GetClaimsPrincipal(context).ShouldBeNull(); + } + + [Fact] + public void returns_null_from_empty_user_context() + { + var accessor = new DefaultClaimsPrincipalAccessor(); + var context = new ValidationContext { UserContext = new Dictionary() }; + accessor.GetClaimsPrincipal(context).ShouldBeNull(); + } + + [Fact] + public void returns_null_from_typed_user_context() + { + var accessor = new DefaultClaimsPrincipalAccessor(); + var context = new ValidationContext { UserContext = new TestContext1() }; + accessor.GetClaimsPrincipal(context).ShouldBeNull(); + } + + [Fact] + public void returns_principal_from_typed_user_context() + { + var accessor = new DefaultClaimsPrincipalAccessor(); + var context = new ValidationContext { UserContext = new TestContext2() }; + accessor.GetClaimsPrincipal(context).ShouldNotBeNull(); + } + + private class TestContext1 : Dictionary, IProvideClaimsPrincipal + { + public ClaimsPrincipal? User => null; + } + + private class TestContext2 : Dictionary, IProvideClaimsPrincipal + { + public ClaimsPrincipal? User => new ClaimsPrincipal(); + } + } +} From 5ab3fd5fe91e6626b16744e32e57fe7a62d26e03 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 01:36:27 +0300 Subject: [PATCH 15/22] test --- .../AuthorizationSettingsTests.cs | 23 ++++++++++++++---- .../ClaimAuthorizationRequirementTests.cs | 24 ++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index 78963ef..80355d1 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -34,12 +34,13 @@ public void can_add_a_claim_policy() .RequireClaim("Admin") .RequireClaim("SuperAdmin", "Super1", "Super2") .RequireClaim("SuperDuperAdmin", new[] { "Super1", "Super2" }, new[] { "Display1", "Display2" }) + .AddRequirement(new ClaimsAuthorizationRequirement("SuperPlus", Enumerable.Empty())) ); _settings.Policies.Count().ShouldBe(1); var policy = _settings.Policies.Single(); - policy.Requirements.Count().ShouldBe(3); + policy.Requirements.Count().ShouldBe(4); policy.Requirements.ToList().ForEach(r => r.ShouldBeOfType()); } @@ -70,16 +71,30 @@ public void can_add_policy_instance() [Fact] public void get_policies() { - _settings.AddPolicy("MyPolicy", new AuthorizationPolicy(new DelegatedRequirement(c => Task.CompletedTask))); + _settings.AddPolicy("MyPolicy1", new AuthorizationPolicy(new DelegatedRequirement(c => Task.CompletedTask))); + _settings.AddPolicy("MyPolicy2", b => b.Require(c => Task.CompletedTask)); _settings.GetPolicies("a").ShouldBeEmpty(); _settings.GetPolicies("a", "b").ShouldBeEmpty(); _settings.GetPolicies(Enumerable.Empty()).ShouldBeEmpty(); - _settings.GetPolicies("MyPolicy").Count().ShouldBe(1); - _settings.GetPolicies("a", "MyPolicy", "b").Count().ShouldBe(1); + _settings.GetPolicies("MyPolicy1").Count().ShouldBe(1); + _settings.GetPolicies("a", "MyPolicy1", "b").Count().ShouldBe(1); + _settings.GetPolicies("a", "MyPolicy2", "MyPolicy1", "b").Count().ShouldBe(2); + + _settings.Policies.Count().ShouldBe(2); + } + + [Fact] + public void replace_policy() + { + _settings.AddPolicy("MyPolicy1", b => b.RequireAuthenticatedUser()); + _settings.AddPolicy("MyPolicy1", b => b.RequireClaim("claim_777")); _settings.Policies.Count().ShouldBe(1); + var req = _settings.Policies.Single().Requirements.Single().ShouldBeOfType(); + req.ClaimType.ShouldBe("claim_777"); + req.DisplayValues.ShouldBeNull(); } } } diff --git a/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs b/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs index 913e2c5..a2e4d8b 100644 --- a/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs +++ b/src/GraphQL.Authorization.Tests/Requirements/ClaimAuthorizationRequirementTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -9,7 +10,13 @@ namespace GraphQL.Authorization.Tests public class ClaimAuthorizationRequirementTests { [Fact] - public async Task produces_error_when_missing_claim_ignoring_value() + public void throw_on_null_claim_type() + { + Should.Throw(() => new ClaimsAuthorizationRequirement(null!)).ParamName.ShouldBe("claimType"); + } + + [Fact] + public async Task produces_error_when_missing_claim_ignoring_value1() { var req = new ClaimsAuthorizationRequirement("Admin"); var policy = new AuthorizationPolicy(req); @@ -23,6 +30,21 @@ public async Task produces_error_when_missing_claim_ignoring_value() //context.Errors.Single().ShouldBe("Required claim 'Admin' is not present."); } + [Fact] + public async Task produces_error_when_missing_claim_ignoring_value2() + { + var req = new ClaimsAuthorizationRequirement("Admin", Enumerable.Empty()); + var policy = new AuthorizationPolicy(req); + var context = new DefaultAuthorizationContext(policy, ValidationTestBase.CreatePrincipal()); + + await req.Authorize(context); + + context.HasSucceeded.ShouldBeFalse(); + context.HasFailed.ShouldBeFalse(); + context.PendingRequirements.Single().ShouldBe(req); + //context.Errors.Single().ShouldBe("Required claim 'Admin' is not present."); + } + [Fact] public async Task produces_error_when_missing_claim_with_single_value() { From 57857dcaf37a8b0ae4d78673dd3374ce7cc726cc Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 01:49:38 +0300 Subject: [PATCH 16/22] tests --- .../AuthorizationSettingsTests.cs | 8 ++++++++ src/GraphQL.Authorization/DefaultAuthorizationService.cs | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs index 80355d1..6808ad1 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs @@ -68,6 +68,14 @@ public void can_add_policy_instance() policy.Requirements.Single().ShouldBeOfType(); } + [Fact] + public void throw_if_add_null_requirement_in_policy() + { + var ex = Should.Throw(() => _settings.AddPolicy("MyPolicy", new AuthorizationPolicy(null!, new DelegatedRequirement(c => Task.CompletedTask)))); + ex.ParamName.ShouldBe("requirements"); + ex.Message.ShouldStartWith("One of the 2 requirements is null"); + } + [Fact] public void get_policies() { diff --git a/src/GraphQL.Authorization/DefaultAuthorizationService.cs b/src/GraphQL.Authorization/DefaultAuthorizationService.cs index 7d8dcc8..d2290de 100644 --- a/src/GraphQL.Authorization/DefaultAuthorizationService.cs +++ b/src/GraphQL.Authorization/DefaultAuthorizationService.cs @@ -15,9 +15,9 @@ public async Task AuthorizeAsync(IAuthorizationContext cont return context.HasSucceeded ? AuthorizationResult.Success() - : AuthorizationResult.Failed(context.HasFailed - ? AuthorizationFailure.ExplicitFail() - : AuthorizationFailure.Failed(context.PendingRequirements)); + : context.HasFailed + ? AuthorizationResult.Failed() + : AuthorizationResult.Failed(AuthorizationFailure.Failed(context.PendingRequirements)); } } } From 2ec62114f581b9638d4e8a832ba26aa3c0e57208 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 02:06:01 +0300 Subject: [PATCH 17/22] null check --- .../AuthorizationValidationRuleTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index bd11173..d817fc0 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -10,6 +10,14 @@ namespace GraphQL.Authorization.Tests { public class AuthorizationValidationRuleTests : ValidationTestBase { + [Fact] + public void throw_on_null_arguments() + { + Should.Throw(() => new AuthorizationValidationRule(null!, new DefaultClaimsPrincipalAccessor(), new DefaultAuthorizationPolicyProvider(new AuthorizationSettings()))).ParamName.ShouldBe("authorizationService"); + Should.Throw(() => new AuthorizationValidationRule(new DefaultAuthorizationService(), null!, new DefaultAuthorizationPolicyProvider(new AuthorizationSettings()))).ParamName.ShouldBe("claimsPrincipalAccessor"); + Should.Throw(() => new AuthorizationValidationRule(new DefaultAuthorizationService(), new DefaultClaimsPrincipalAccessor(), null!)).ParamName.ShouldBe("policyProvider"); + } + [Fact] public void class_policy_success() { From a160bcf91297a8c859c1b90a2119a5063a545bb3 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 09:46:16 +0300 Subject: [PATCH 18/22] Update README.md Co-authored-by: Shane Krueger --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c8c8667..386f826 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValid in your DI container. **Option 2.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from -the main GraphQL.NET repository. For convenience you may use `ErrorInfoProvider` base class. +the main GraphQL.NET repository. For convenience you may use the `ErrorInfoProvider` base class. ```csharp public class CustomErrorInfoProvider : ErrorInfoProvider From 2c503e8e26b079e3970893a5c1d71a379ea62ce1 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 12:20:20 +0300 Subject: [PATCH 19/22] introduce AuthorizationErrorMessageBuilder --- README.md | 59 +++++++++++- .../GraphQL.Authorization.approved.txt | 9 +- .../AuthorizationError.cs | 81 +---------------- .../AuthorizationErrorMessageBuilder.cs | 90 +++++++++++++++++++ 4 files changed, 155 insertions(+), 84 deletions(-) create mode 100644 src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs diff --git a/README.md b/README.md index c8c8667..b931d00 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ unsatisfied requirement, the validation rule will add an authorization error in your own authorization requirements. In this case, you can override the default behavior. **Option 1.** Create a descendant from `AuthorizationValidationRule` and override -`AddValidationError` method. +`AddValidationError` method passing a message built manually. ```csharp public class CustomAuthorizationValidationRule : AuthorizationValidationRule @@ -173,12 +173,63 @@ public class CustomAuthorizationValidationRule : AuthorizationValidationRule Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValidationRule` in your DI container. -**Option 2.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from +**Option 2.** The same as above + custom `AuthorizationError/AuthorizationErrorMessageBuilder`. + +```csharp +public class CustomAuthorizationValidationRule : AuthorizationValidationRule +{ + public CustomAuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) + : base(authorizationService, claimsPrincipalAccessor, policyProvider) + { + } + + protected override void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + { + // no message built here, now it's built inside a custom message builder called from MyAuthorizationError constructor + context.ReportError(new MyAuthorizationError(node, context, operationType, result)); + } +} + +public class MyAuthorizationError : ValidationError +{ + private static readonly MyAuthorizationErrorMessageBuilder _builder = new MyAuthorizationErrorMessageBuilder(); + + public MyAuthorizationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) + : this(node, context, _builder.Build(operationType, result), result) + { + OperationType = operationType; + } +} + +public class MyAuthorizationErrorMessageBuilder : AuthorizationErrorMessageBuilder +{ + public override void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) + { + switch (authorizationRequirement) + { + case MySpecialRequirement special: + error.Append("My special error message"); + break; + + default: + base.AppendFailureLine(error, authorizationRequirement); + } + } +} +``` + +This approach allows you to separate the code for building an error message from the error +class itself while continue to handle known authorization requirements. It can be convenient +if you have many custom authorization requirements. + +**Option 3.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from the main GraphQL.NET repository. For convenience you may use `ErrorInfoProvider` base class. ```csharp public class CustomErrorInfoProvider : ErrorInfoProvider { + private static readonly AuthorizationErrorMessageBuilder _builder = new AuthorizationErrorMessageBuilder(); + public override ErrorInfo GetInfo(ExecutionError executionError) { var info = base.GetInfo(executionError); @@ -193,7 +244,7 @@ public class CustomErrorInfoProvider : ErrorInfoProvider private string GetAuthorizationErrorMessage(AuthorizationError error) { var errorMessage = new StringBuilder(); - AuthorizationError.AppendFailureHeader(errorMessage, error.OperationType); + _builder.AppendFailureHeader(errorMessage, error.OperationType); foreach (var failedRequirement in error.AuthorizationResult.Failure.FailedRequirements) { @@ -204,7 +255,7 @@ public class CustomErrorInfoProvider : ErrorInfoProvider errorMessage.Append("Access is allowed only on Mondays."); break; default: - AuthorizationError.AppendFailureLine(errorMessage, failedRequirement); + _builder.AppendFailureLine(errorMessage, failedRequirement); break; } } diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index 8cec7e5..505829b 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -11,8 +11,13 @@ namespace GraphQL.Authorization public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, string message, GraphQL.Authorization.AuthorizationResult result) { } public virtual GraphQL.Authorization.AuthorizationResult AuthorizationResult { get; } public GraphQL.Language.AST.OperationType? OperationType { get; } - public static void AppendFailureHeader(System.Text.StringBuilder error, GraphQL.Language.AST.OperationType? operationType) { } - public static void AppendFailureLine(System.Text.StringBuilder error, GraphQL.Authorization.IAuthorizationRequirement authorizationRequirement) { } + } + public class AuthorizationErrorMessageBuilder + { + public AuthorizationErrorMessageBuilder() { } + public virtual void AppendFailureHeader(System.Text.StringBuilder error, GraphQL.Language.AST.OperationType? operationType) { } + public virtual void AppendFailureLine(System.Text.StringBuilder error, GraphQL.Authorization.IAuthorizationRequirement authorizationRequirement) { } + public virtual string Build(GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } } public class AuthorizationFailure { diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs index 8bd1d57..6df137a 100644 --- a/src/GraphQL.Authorization/AuthorizationError.cs +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -1,6 +1,4 @@ using System; -using System.Linq; -using System.Text; using GraphQL.Language.AST; using GraphQL.Validation; @@ -11,11 +9,13 @@ namespace GraphQL.Authorization /// public class AuthorizationError : ValidationError { + private static readonly AuthorizationErrorMessageBuilder _builder = new AuthorizationErrorMessageBuilder(); + /// /// Initializes a new instance of the class for a specified authorization result. /// public AuthorizationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - : this(node, context, GenerateMessage(operationType, result), result) + : this(node, context, _builder.Build(operationType, result), result) { OperationType = operationType; } @@ -39,80 +39,5 @@ public AuthorizationError(INode? node, ValidationContext context, string message /// The GraphQL operation type. /// public OperationType? OperationType { get; } - - private static string GenerateMessage(OperationType? operationType, AuthorizationResult result) - { - var error = new StringBuilder(); - AppendFailureHeader(error, operationType); - - if (result.Failure != null) - { - foreach (var failure in result.Failure.FailedRequirements) - { - AppendFailureLine(error, failure); - } - } - - return error.ToString(); - } - - private static string GetOperationType(OperationType? operationType) - { - return operationType switch - { - Language.AST.OperationType.Query => "query", - Language.AST.OperationType.Mutation => "mutation", - Language.AST.OperationType.Subscription => "subscription", - _ => "operation", - }; - } - - /// - /// Appends the error message header for this to the provided . - /// - /// The error message . - /// The GraphQL operation type. - public static void AppendFailureHeader(StringBuilder error, OperationType? operationType) - { - error.Append("You are not authorized to run this ") - .Append(GetOperationType(operationType)) - .Append('.'); - } - - /// - /// Appends a description of the failed to the supplied . - /// - /// The which is used to compose the error message. - /// The failed . - public static void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) - { - error.AppendLine(); - - switch (authorizationRequirement) - { - case DefinedPolicyRequirement definedPolicyRequirement: - error.Append($"Required policy '{definedPolicyRequirement.PolicyName}' is not present."); - break; - - case AuthenticatedUserRequirement _: - error.Append("An authenticated user is required."); - break; - - case ClaimsAuthorizationRequirement claimsAuthorizationRequirement: - error.Append("Required claim '"); - error.Append(claimsAuthorizationRequirement.ClaimType); - if (claimsAuthorizationRequirement.AllowedValues == null || !claimsAuthorizationRequirement.AllowedValues.Any()) - { - error.Append("' is not present."); - } - else - { - error.Append("' with any value of '"); - error.Append(string.Join(", ", claimsAuthorizationRequirement.AllowedValues ?? claimsAuthorizationRequirement.DisplayValues)); - error.Append("' is not present."); - } - break; - } - } } } diff --git a/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs b/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs new file mode 100644 index 0000000..fe7ac1a --- /dev/null +++ b/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs @@ -0,0 +1,90 @@ +using System.Linq; +using System.Text; +using GraphQL.Language.AST; + +namespace GraphQL.Authorization +{ + /// + /// Builds error message for and its descendants. + /// + public class AuthorizationErrorMessageBuilder + { + /// + /// Builds error message for the specified operation type and authorization result. + /// + public virtual string Build(OperationType? operationType, AuthorizationResult result) + { + var error = new StringBuilder(); + AppendFailureHeader(error, operationType); + + if (result.Failure != null) + { + foreach (var failure in result.Failure.FailedRequirements) + { + AppendFailureLine(error, failure); + } + } + + return error.ToString(); + } + + /// + /// Appends the error message header for this to the provided . + /// + /// The error message . + /// The GraphQL operation type. + public virtual void AppendFailureHeader(StringBuilder error, OperationType? operationType) + { + error.Append("You are not authorized to run this ") + .Append(GetOperationType(operationType)) + .Append('.'); + + static string GetOperationType(OperationType? operationType) + { + return operationType switch + { + OperationType.Query => "query", + OperationType.Mutation => "mutation", + OperationType.Subscription => "subscription", + _ => "operation", + }; + } + } + + /// + /// Appends a description of the failed to the supplied . + /// + /// The which is used to compose the error message. + /// The failed . + public virtual void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) + { + error.AppendLine(); + + switch (authorizationRequirement) + { + case DefinedPolicyRequirement definedPolicyRequirement: + error.Append($"Required policy '{definedPolicyRequirement.PolicyName}' is not present."); + break; + + case AuthenticatedUserRequirement _: + error.Append("An authenticated user is required."); + break; + + case ClaimsAuthorizationRequirement claimsAuthorizationRequirement: + error.Append("Required claim '"); + error.Append(claimsAuthorizationRequirement.ClaimType); + if (claimsAuthorizationRequirement.AllowedValues == null || !claimsAuthorizationRequirement.AllowedValues.Any()) + { + error.Append("' is not present."); + } + else + { + error.Append("' with any value of '"); + error.Append(string.Join(", ", claimsAuthorizationRequirement.AllowedValues ?? claimsAuthorizationRequirement.DisplayValues)); + error.Append("' is not present."); + } + break; + } + } + } +} From 7944e7a6449ff08b30529031c65bdbd204186d17 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Wed, 24 Mar 2021 13:19:06 +0300 Subject: [PATCH 20/22] apply format --- src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs b/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs index fe7ac1a..5a49595 100644 --- a/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs +++ b/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs @@ -1,4 +1,4 @@ -using System.Linq; +using System.Linq; using System.Text; using GraphQL.Language.AST; From 51de6ed3de3479796b187ade12bc2b3c8fd1e2a1 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Thu, 25 Mar 2021 01:08:56 +0300 Subject: [PATCH 21/22] simplify error messages --- README.md | 116 ++++-------------- .../GraphQL.Authorization.approved.txt | 26 ++-- .../AuthorizationError.cs | 16 +-- .../AuthorizationErrorMessageBuilder.cs | 90 -------------- .../AuthorizationValidationRule.cs | 40 +++++- .../AuthenticatedUserRequirement.cs | 5 +- .../ClaimsAuthorizationRequirement.cs | 27 +++- .../Requirements/IAuthorizationRequirement.cs | 12 ++ .../Requirements/PolicyExistsRequirement.cs | 7 +- 9 files changed, 125 insertions(+), 214 deletions(-) delete mode 100644 src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs diff --git a/README.md b/README.md index f1c9fd5..e6a70f7 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # GraphQL Authorization +![License](https://img.shields.io/github/license/graphql-dotnet/authorization) + [![Join the chat at https://gitter.im/graphql-dotnet/graphql-dotnet](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/graphql-dotnet/graphql-dotnet?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![Run code tests](https://github.com/graphql-dotnet/authorization/actions/workflows/test.yml/badge.svg)](https://github.com/graphql-dotnet/authorization/actions/workflows/test.yml) @@ -141,127 +143,55 @@ public static void ConfigureAuthorizationServices(ServiceCollection services) #### How to change error messages -All authorization requirements (built-in or custom ones) only check the compliance of -the current execution state to their criteria. If the requirement is satisfied, then +Authorization requirement (`IAuthorizationRequirement`) only checks the compliance of +the current execution state to some criteria. If the requirement is satisfied, then it is marked as 'passed' and the next requirement is checked. If all requirements are satisfied, then the validation rule returns a successful result. Otherwise for each unsatisfied requirement, the validation rule will add an authorization error in the `ValidationContext`. The text of this error may not suit you, especially if you write -your own authorization requirements. In this case, you can override the default behavior. +your own authorization requirements because by default you will see only _You are not +authorized to run this query_ text which does not contain any details about your +requirement. This is done for security reasons but you can override the default behavior. -**Option 1.** Create a descendant from `AuthorizationValidationRule` and override -`AddValidationError` method passing a message built manually. +**Option 1.** If you are satisfied with the existing error messages and you only want +to add error message for your own authorization requirement, then inherit your authorization +requirement from `IAuthorizationRequirementWithErrorMessage` interface. ```csharp -public class CustomAuthorizationValidationRule : AuthorizationValidationRule +public class OnlyMondayRequirement : IAuthorizationRequirementWithErrorMessage { - public CustomAuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) - : base(authorizationService, claimsPrincipalAccessor, policyProvider) - { - } - - protected override void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - { - if (result.Failure.FailedRequirements.Any(r => r is MySpecialRequirement)) - context.ReportError(new AuthorizationError(node, context, "My special error message", result)); - else - base.AddValidationError(node, context, operationType, result); - } -} -``` - -Then register `CustomAuthorizationValidationRule` instead of `AuthorizationValidationRule` -in your DI container. - -**Option 2.** The same as above + custom `AuthorizationError/AuthorizationErrorMessageBuilder`. - -```csharp -public class CustomAuthorizationValidationRule : AuthorizationValidationRule -{ - public CustomAuthorizationValidationRule(IAuthorizationService authorizationService, IClaimsPrincipalAccessor claimsPrincipalAccessor, IAuthorizationPolicyProvider policyProvider) - : base(authorizationService, claimsPrincipalAccessor, policyProvider) - { - } - - protected override void AddValidationError(INode node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - { - // no message built here, now it's built inside a custom message builder called from MyAuthorizationError constructor - context.ReportError(new MyAuthorizationError(node, context, operationType, result)); - } -} - -public class MyAuthorizationError : ValidationError -{ - private static readonly MyAuthorizationErrorMessageBuilder _builder = new MyAuthorizationErrorMessageBuilder(); - - public MyAuthorizationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - : this(node, context, _builder.Build(operationType, result), result) + public Task Authorize(IAuthorizationContext context) { - OperationType = operationType; + if (DateTime.Now.DayOfWeek == DayOfWeek.Monday) + context.Succeed(this); } -} - -public class MyAuthorizationErrorMessageBuilder : AuthorizationErrorMessageBuilder -{ - public override void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) - { - switch (authorizationRequirement) - { - case MySpecialRequirement special: - error.Append("My special error message"); - break; - default: - base.AppendFailureLine(error, authorizationRequirement); - } - } + public string ErrorMessage => "Access is allowed only on Mondays."; } ``` -This approach allows you to separate the code for building an error message from the error -class itself while continue to handle known authorization requirements. It can be convenient -if you have many custom authorization requirements. +**Option 2.** If you want to get full control over the whole error message for authorization +process then inherit from `AuthorizationValidationRule` and override `AddValidationError` +or `BuildErrorMessage` methods. Then register `CustomAuthorizationValidationRule` class +instead of `AuthorizationValidationRule` class in your DI container. -**Option 3.** Implement `IErrorInfoProvider` interface. This is one of the interfaces from -the main GraphQL.NET repository. For convenience you may use the `ErrorInfoProvider` base class. +**Option 3.** Another way to get full control over the whole error message sent to client +is to implement `IErrorInfoProvider` interface. This is one of the interfaces from the +main GraphQL.NET repository. For convenience you may use the `ErrorInfoProvider` base class. ```csharp public class CustomErrorInfoProvider : ErrorInfoProvider { - private static readonly AuthorizationErrorMessageBuilder _builder = new AuthorizationErrorMessageBuilder(); - public override ErrorInfo GetInfo(ExecutionError executionError) { var info = base.GetInfo(executionError); info.Message = executionError switch { - AuthorizationError authorizationError => GetAuthorizationErrorMessage(authorizationError), + AuthorizationError authorizationError => "You shall not pass!", _ => info.Message, }; return info; } - - private string GetAuthorizationErrorMessage(AuthorizationError error) - { - var errorMessage = new StringBuilder(); - _builder.AppendFailureHeader(errorMessage, error.OperationType); - - foreach (var failedRequirement in error.AuthorizationResult.Failure.FailedRequirements) - { - switch (failedRequirement) - { - case OnlyMondayRequirement onlyMondayRequirement: - errorMessage.AppendLine(); - errorMessage.Append("Access is allowed only on Mondays."); - break; - default: - _builder.AppendFailureLine(errorMessage, failedRequirement); - break; - } - } - - return errorMessage.ToString(); - } } ``` diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index 505829b..ff4b93a 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -1,24 +1,17 @@ namespace GraphQL.Authorization { - public class AuthenticatedUserRequirement : GraphQL.Authorization.IAuthorizationRequirement + public class AuthenticatedUserRequirement : GraphQL.Authorization.IAuthorizationRequirement, GraphQL.Authorization.IAuthorizationRequirementWithErrorMessage { public AuthenticatedUserRequirement() { } + public string ErrorMessage { get; } public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } } public class AuthorizationError : GraphQL.Validation.ValidationError { - public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } - public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, string message, GraphQL.Authorization.AuthorizationResult result) { } + public AuthorizationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, string message, GraphQL.Authorization.AuthorizationResult result) { } public virtual GraphQL.Authorization.AuthorizationResult AuthorizationResult { get; } public GraphQL.Language.AST.OperationType? OperationType { get; } } - public class AuthorizationErrorMessageBuilder - { - public AuthorizationErrorMessageBuilder() { } - public virtual void AppendFailureHeader(System.Text.StringBuilder error, GraphQL.Language.AST.OperationType? operationType) { } - public virtual void AppendFailureLine(System.Text.StringBuilder error, GraphQL.Authorization.IAuthorizationRequirement authorizationRequirement) { } - public virtual string Build(GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } - } public class AuthorizationFailure { public bool FailCalled { get; } @@ -69,10 +62,11 @@ namespace GraphQL.Authorization { public AuthorizationValidationRule(GraphQL.Authorization.IAuthorizationService authorizationService, GraphQL.Authorization.IClaimsPrincipalAccessor claimsPrincipalAccessor, GraphQL.Authorization.IAuthorizationPolicyProvider policyProvider) { } protected virtual void AddValidationError(GraphQL.Language.AST.INode? node, GraphQL.Validation.ValidationContext context, GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } + protected virtual string BuildErrorMessage(GraphQL.Language.AST.OperationType? operationType, GraphQL.Authorization.AuthorizationResult result) { } protected virtual GraphQL.Authorization.IAuthorizationContext CreateAuthorizationContext(GraphQL.Validation.ValidationContext context, string policyName) { } public System.Threading.Tasks.Task ValidateAsync(GraphQL.Validation.ValidationContext context) { } } - public class ClaimsAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement + public class ClaimsAuthorizationRequirement : GraphQL.Authorization.IAuthorizationRequirement, GraphQL.Authorization.IAuthorizationRequirementWithErrorMessage { public ClaimsAuthorizationRequirement(string claimType) { } public ClaimsAuthorizationRequirement(string claimType, System.Collections.Generic.IEnumerable allowedValues) { } @@ -81,6 +75,7 @@ namespace GraphQL.Authorization public System.Collections.Generic.IEnumerable? AllowedValues { get; } public string ClaimType { get; } public System.Collections.Generic.IEnumerable? DisplayValues { get; } + public string ErrorMessage { get; } public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } } public class DefaultAuthorizationContext : GraphQL.Authorization.IAuthorizationContext @@ -111,11 +106,12 @@ namespace GraphQL.Authorization public DefaultClaimsPrincipalAccessor() { } public System.Security.Claims.ClaimsPrincipal? GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } } - public class DefinedPolicyRequirement : GraphQL.Authorization.IAuthorizationRequirement + public class DefinedPolicyRequirement : GraphQL.Authorization.IAuthorizationRequirement, GraphQL.Authorization.IAuthorizationRequirementWithErrorMessage { public DefinedPolicyRequirement(string policyName) { } + public string ErrorMessage { get; } public string PolicyName { get; } - public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context) { } + public System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext _) { } } public class DelegatedRequirement : GraphQL.Authorization.IAuthorizationRequirement { @@ -153,6 +149,10 @@ namespace GraphQL.Authorization { System.Threading.Tasks.Task Authorize(GraphQL.Authorization.IAuthorizationContext context); } + public interface IAuthorizationRequirementWithErrorMessage : GraphQL.Authorization.IAuthorizationRequirement + { + string ErrorMessage { get; } + } public interface IAuthorizationService { System.Threading.Tasks.Task AuthorizeAsync(GraphQL.Authorization.IAuthorizationContext context); diff --git a/src/GraphQL.Authorization/AuthorizationError.cs b/src/GraphQL.Authorization/AuthorizationError.cs index 6df137a..f2d9fd2 100644 --- a/src/GraphQL.Authorization/AuthorizationError.cs +++ b/src/GraphQL.Authorization/AuthorizationError.cs @@ -9,24 +9,14 @@ namespace GraphQL.Authorization /// public class AuthorizationError : ValidationError { - private static readonly AuthorizationErrorMessageBuilder _builder = new AuthorizationErrorMessageBuilder(); - - /// - /// Initializes a new instance of the class for a specified authorization result. - /// - public AuthorizationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - : this(node, context, _builder.Build(operationType, result), result) - { - OperationType = operationType; - } - /// - /// Initializes a new instance of the class for a specified authorization result with a specific error message. + /// Initializes a new instance of the class with the specified parameters. /// - public AuthorizationError(INode? node, ValidationContext context, string message, AuthorizationResult result) + public AuthorizationError(INode? node, ValidationContext context, OperationType? operationType, string message, AuthorizationResult result) : base(context.Document.OriginalQuery, "6.1.1", message, node == null ? Array.Empty() : new INode[] { node }) { Code = "authorization"; + OperationType = operationType; AuthorizationResult = result; } diff --git a/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs b/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs deleted file mode 100644 index 5a49595..0000000 --- a/src/GraphQL.Authorization/AuthorizationErrorMessageBuilder.cs +++ /dev/null @@ -1,90 +0,0 @@ -using System.Linq; -using System.Text; -using GraphQL.Language.AST; - -namespace GraphQL.Authorization -{ - /// - /// Builds error message for and its descendants. - /// - public class AuthorizationErrorMessageBuilder - { - /// - /// Builds error message for the specified operation type and authorization result. - /// - public virtual string Build(OperationType? operationType, AuthorizationResult result) - { - var error = new StringBuilder(); - AppendFailureHeader(error, operationType); - - if (result.Failure != null) - { - foreach (var failure in result.Failure.FailedRequirements) - { - AppendFailureLine(error, failure); - } - } - - return error.ToString(); - } - - /// - /// Appends the error message header for this to the provided . - /// - /// The error message . - /// The GraphQL operation type. - public virtual void AppendFailureHeader(StringBuilder error, OperationType? operationType) - { - error.Append("You are not authorized to run this ") - .Append(GetOperationType(operationType)) - .Append('.'); - - static string GetOperationType(OperationType? operationType) - { - return operationType switch - { - OperationType.Query => "query", - OperationType.Mutation => "mutation", - OperationType.Subscription => "subscription", - _ => "operation", - }; - } - } - - /// - /// Appends a description of the failed to the supplied . - /// - /// The which is used to compose the error message. - /// The failed . - public virtual void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) - { - error.AppendLine(); - - switch (authorizationRequirement) - { - case DefinedPolicyRequirement definedPolicyRequirement: - error.Append($"Required policy '{definedPolicyRequirement.PolicyName}' is not present."); - break; - - case AuthenticatedUserRequirement _: - error.Append("An authenticated user is required."); - break; - - case ClaimsAuthorizationRequirement claimsAuthorizationRequirement: - error.Append("Required claim '"); - error.Append(claimsAuthorizationRequirement.ClaimType); - if (claimsAuthorizationRequirement.AllowedValues == null || !claimsAuthorizationRequirement.AllowedValues.Any()) - { - error.Append("' is not present."); - } - else - { - error.Append("' with any value of '"); - error.Append(string.Join(", ", claimsAuthorizationRequirement.AllowedValues ?? claimsAuthorizationRequirement.DisplayValues)); - error.Append("' is not present."); - } - break; - } - } - } -} diff --git a/src/GraphQL.Authorization/AuthorizationValidationRule.cs b/src/GraphQL.Authorization/AuthorizationValidationRule.cs index a822764..488fa88 100644 --- a/src/GraphQL.Authorization/AuthorizationValidationRule.cs +++ b/src/GraphQL.Authorization/AuthorizationValidationRule.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Security.Claims; +using System.Text; using System.Threading.Tasks; using GraphQL.Language.AST; using GraphQL.Types; @@ -128,7 +129,44 @@ private async Task AuthorizeAsync(INode? node, IProvideMetadata provider, Valida /// protected virtual void AddValidationError(INode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) { - context.ReportError(new AuthorizationError(node, context, operationType, result)); + context.ReportError(new AuthorizationError(node, context, operationType, BuildErrorMessage(operationType, result), result)); + } + + /// + /// Builds error message for the specified operation type and authorization result. + /// + protected virtual string BuildErrorMessage(OperationType? operationType, AuthorizationResult result) + { + static string GetOperationType(OperationType? operationType) + { + return operationType switch + { + OperationType.Query => "query", + OperationType.Mutation => "mutation", + OperationType.Subscription => "subscription", + _ => "operation", + }; + } + + var error = new StringBuilder(); + + error.Append("You are not authorized to run this ") + .Append(GetOperationType(operationType)) + .Append('.'); + + if (result.Failure != null) + { + foreach (var failure in result.Failure.FailedRequirements) + { + if (failure is IAuthorizationRequirementWithErrorMessage requirementWitErrorMessage) + { + error.AppendLine(); + error.Append(requirementWitErrorMessage.ErrorMessage); + } + } + } + + return error.ToString(); } } } diff --git a/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs b/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs index 92f1b0d..846bd88 100644 --- a/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/AuthenticatedUserRequirement.cs @@ -7,7 +7,7 @@ namespace GraphQL.Authorization /// Implements an which requires that /// current user from must be authenticated. /// - public class AuthenticatedUserRequirement : IAuthorizationRequirement + public class AuthenticatedUserRequirement : IAuthorizationRequirementWithErrorMessage { internal static readonly AuthenticatedUserRequirement Instance = new AuthenticatedUserRequirement(); @@ -19,5 +19,8 @@ public Task Authorize(IAuthorizationContext context) return Task.CompletedTask; } + + /// + public string ErrorMessage => "An authenticated user is required."; } } diff --git a/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs index 04fa44d..cdb321a 100644 --- a/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/ClaimsAuthorizationRequirement.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using System.Threading.Tasks; namespace GraphQL.Authorization @@ -9,7 +10,7 @@ namespace GraphQL.Authorization /// Implements an which requires an instance of the specified /// claim type, and, if allowed values are specified, the claim value must be any of the allowed values. /// - public class ClaimsAuthorizationRequirement : IAuthorizationRequirement + public class ClaimsAuthorizationRequirement : IAuthorizationRequirementWithErrorMessage { /// /// Creates a new instance of with @@ -95,5 +96,29 @@ public Task Authorize(IAuthorizationContext context) return Task.CompletedTask; } + + /// + public string ErrorMessage + { + get + { + var error = new StringBuilder(); + + error.Append("Required claim '"); + error.Append(ClaimType); + if (AllowedValues == null || !AllowedValues.Any()) + { + error.Append("' is not present."); + } + else + { + error.Append("' with any value of '"); + error.Append(string.Join(", ", AllowedValues ?? DisplayValues)); + error.Append("' is not present."); + } + + return error.ToString(); + } + } } } diff --git a/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs b/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs index 2835524..f545ec5 100644 --- a/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/IAuthorizationRequirement.cs @@ -14,4 +14,16 @@ public interface IAuthorizationRequirement /// Task Authorize(IAuthorizationContext context); } + + /// + /// Represents an authorization requirement that provides error message to include + /// in resulting GraphQL error. + /// + public interface IAuthorizationRequirementWithErrorMessage : IAuthorizationRequirement + { + /// + /// Gets error message. + /// + string ErrorMessage { get; } + } } diff --git a/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs index 635cace..a73f9cf 100644 --- a/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs +++ b/src/GraphQL.Authorization/Requirements/PolicyExistsRequirement.cs @@ -6,7 +6,7 @@ namespace GraphQL.Authorization /// Implements an which requires that /// the specified policy must be defined. /// - public class DefinedPolicyRequirement : IAuthorizationRequirement + public class DefinedPolicyRequirement : IAuthorizationRequirementWithErrorMessage { /// /// Creates a new instance of with @@ -25,6 +25,9 @@ public DefinedPolicyRequirement(string policyName) /// /// Execute requirement. This requirement always isn't met by design. /// - public Task Authorize(IAuthorizationContext context) => Task.CompletedTask; + public Task Authorize(IAuthorizationContext _) => Task.CompletedTask; + + /// + public string ErrorMessage => $"Required policy '{PolicyName}' is not present."; } } From c7944e7e203f8680c3a297f1a3629ed702975680 Mon Sep 17 00:00:00 2001 From: Ivan Maximov Date: Mon, 6 Dec 2021 19:25:38 +0300 Subject: [PATCH 22/22] move policy in ctor arg --- src/BasicSample/Program.cs | 2 +- .../GraphQL.Authorization.approved.txt | 4 ++-- .../AuthorizationSchemaBuilderTests.cs | 4 ++-- .../AuthorizationValidationRuleTests.cs | 6 +++--- .../GraphQLAuthorizeAttribute.cs | 12 +++++++++++- src/Harness/GraphQL.cs | 2 +- 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/BasicSample/Program.cs b/src/BasicSample/Program.cs index 1bd996e..3f36ac7 100644 --- a/src/BasicSample/Program.cs +++ b/src/BasicSample/Program.cs @@ -80,7 +80,7 @@ public class Query /// /// Resolver for 'Query.viewer' field. /// - [GraphQLAuthorize(Policy = "AdminPolicy")] + [GraphQLAuthorize("AdminPolicy")] public User Viewer() => new User { Id = Guid.NewGuid().ToString(), Name = "Quinn" }; /// diff --git a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt index ff4b93a..3a97330 100644 --- a/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt +++ b/src/GraphQL.Authorization.ApiTests/GraphQL.Authorization.approved.txt @@ -120,8 +120,8 @@ namespace GraphQL.Authorization } public class GraphQLAuthorizeAttribute : GraphQL.GraphQLAttribute { - public GraphQLAuthorizeAttribute() { } - public string Policy { get; set; } + public GraphQLAuthorizeAttribute(string policy) { } + public string Policy { get; } public override void Modify(GraphQL.Utilities.FieldConfig field) { } public override void Modify(GraphQL.Utilities.TypeConfig type) { } } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs index 2adb4b2..722e545 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs @@ -30,10 +30,10 @@ type Query { } [GraphQLMetadata("Query")] - [GraphQLAuthorize(Policy = "ClassPolicy")] + [GraphQLAuthorize("ClassPolicy")] public class QueryWithAttributes { - [GraphQLAuthorize(Policy = "FieldPolicy")] + [GraphQLAuthorize("FieldPolicy")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "test")] public string Post(string id) => ""; } diff --git a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs index d817fc0..96d5a13 100644 --- a/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs +++ b/src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs @@ -222,10 +222,10 @@ type Query { } [GraphQLMetadata("Query")] - [GraphQLAuthorize(Policy = "ClassPolicy")] + [GraphQLAuthorize("ClassPolicy")] public class BasicQueryWithAttributes { - [GraphQLAuthorize(Policy = "FieldPolicy")] + [GraphQLAuthorize("FieldPolicy")] [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "test")] public string Post(string id) => ""; } @@ -262,7 +262,7 @@ public class NestedQueryWithAttributes public IEnumerable? PostsNonNull() => null; } - [GraphQLAuthorize(Policy = "PostPolicy")] + [GraphQLAuthorize("PostPolicy")] public class Post { public string? Id { get; set; } diff --git a/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs b/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs index 57460b8..f7bb852 100644 --- a/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs +++ b/src/GraphQL.Authorization/GraphQLAuthorizeAttribute.cs @@ -7,10 +7,20 @@ namespace GraphQL.Authorization /// public class GraphQLAuthorizeAttribute : GraphQLAttribute { + /// + /// Creates an instance of + /// with the specified policy name. + /// + /// + public GraphQLAuthorizeAttribute(string policy) + { + Policy = policy; + } + /// /// The name of policy to apply. /// - public string Policy { get; set; } = null!; + public string Policy { get; } /// public override void Modify(TypeConfig type) => type.AuthorizeWith(Policy); diff --git a/src/Harness/GraphQL.cs b/src/Harness/GraphQL.cs index 97992da..a06c3cb 100644 --- a/src/Harness/GraphQL.cs +++ b/src/Harness/GraphQL.cs @@ -22,7 +22,7 @@ public class Query /// /// Resolver for 'Query.viewer' field. /// - [GraphQLAuthorize(Policy = "AdminPolicy")] + [GraphQLAuthorize("AdminPolicy")] public User Viewer() => new User { Id = Guid.NewGuid().ToString(), Name = "Quinn" }; ///