From 9e95e01db6fec8ebe270a79ec864ff4e7c432483 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Fri, 20 May 2022 20:42:43 -0400 Subject: [PATCH 01/16] Replace ASP.Net Core validation rule --- GraphQL.Server.sln | 6 - .../Authorization.AspNetCore.csproj | 14 - .../AuthorizationError.cs | 34 -- .../AuthorizationValidationRule.cs | 214 ------- ...DefaultAuthorizationErrorMessageBuilder.cs | 112 ---- .../DefaultClaimsPrincipalAccessor.cs | 32 - .../GraphQLBuilderAuthorizationExtensions.cs | 45 -- .../IAuthorizationErrorMessageBuilder.cs | 32 - .../IClaimsPrincipalAccessor.cs | 17 - .../AuthorizationValidationRule.cs | 550 ++++++++++++++++++ .../Authorization.AspNetCore.Tests.csproj | 2 +- .../AuthorizationValidationRuleTests.cs | 29 +- .../ValidationTestBase.cs | 6 +- 13 files changed, 565 insertions(+), 528 deletions(-) delete mode 100644 src/Authorization.AspNetCore/Authorization.AspNetCore.csproj delete mode 100644 src/Authorization.AspNetCore/AuthorizationError.cs delete mode 100644 src/Authorization.AspNetCore/AuthorizationValidationRule.cs delete mode 100644 src/Authorization.AspNetCore/DefaultAuthorizationErrorMessageBuilder.cs delete mode 100644 src/Authorization.AspNetCore/DefaultClaimsPrincipalAccessor.cs delete mode 100644 src/Authorization.AspNetCore/GraphQLBuilderAuthorizationExtensions.cs delete mode 100644 src/Authorization.AspNetCore/IAuthorizationErrorMessageBuilder.cs delete mode 100644 src/Authorization.AspNetCore/IClaimsPrincipalAccessor.cs create mode 100644 src/Transports.AspNetCore/AuthorizationValidationRule.cs diff --git a/GraphQL.Server.sln b/GraphQL.Server.sln index 5e2a6522..4a475734 100644 --- a/GraphQL.Server.sln +++ b/GraphQL.Server.sln @@ -34,8 +34,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Transports.Subscriptions.Ab EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Ui.Voyager", "src\Ui.Voyager\Ui.Voyager.csproj", "{B2C278E4-6A1A-4F83-AE53-C9469B4056EE}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Authorization.AspNetCore", "src\Authorization.AspNetCore\Authorization.AspNetCore.csproj", "{7A71AF0D-FE5F-4607-A6F6-960FD98CF840}" -EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Authorization.AspNetCore.Tests", "tests\Authorization.AspNetCore.Tests\Authorization.AspNetCore.Tests.csproj", "{741DEEE6-FD0B-4F99-8A6F-43584B3E8D5F}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Samples.Server.Tests", "tests\Samples.Server.Tests\Samples.Server.Tests.csproj", "{62E7B30D-CB34-45EA-A410-2CAE127385D7}" @@ -119,10 +117,6 @@ Global {B2C278E4-6A1A-4F83-AE53-C9469B4056EE}.Debug|Any CPU.Build.0 = Debug|Any CPU {B2C278E4-6A1A-4F83-AE53-C9469B4056EE}.Release|Any CPU.ActiveCfg = Release|Any CPU {B2C278E4-6A1A-4F83-AE53-C9469B4056EE}.Release|Any CPU.Build.0 = Release|Any CPU - {7A71AF0D-FE5F-4607-A6F6-960FD98CF840}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {7A71AF0D-FE5F-4607-A6F6-960FD98CF840}.Debug|Any CPU.Build.0 = Debug|Any CPU - {7A71AF0D-FE5F-4607-A6F6-960FD98CF840}.Release|Any CPU.ActiveCfg = Release|Any CPU - {7A71AF0D-FE5F-4607-A6F6-960FD98CF840}.Release|Any CPU.Build.0 = Release|Any CPU {741DEEE6-FD0B-4F99-8A6F-43584B3E8D5F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {741DEEE6-FD0B-4F99-8A6F-43584B3E8D5F}.Debug|Any CPU.Build.0 = Debug|Any CPU {741DEEE6-FD0B-4F99-8A6F-43584B3E8D5F}.Release|Any CPU.ActiveCfg = Release|Any CPU diff --git a/src/Authorization.AspNetCore/Authorization.AspNetCore.csproj b/src/Authorization.AspNetCore/Authorization.AspNetCore.csproj deleted file mode 100644 index 8650f910..00000000 --- a/src/Authorization.AspNetCore/Authorization.AspNetCore.csproj +++ /dev/null @@ -1,14 +0,0 @@ - - - - net6;net5;netcoreapp3.1 - Integration of GraphQL.NET validation subsystem into ASP.NET Core - GraphQL;authentication;authorization;validation - - - - - - - - diff --git a/src/Authorization.AspNetCore/AuthorizationError.cs b/src/Authorization.AspNetCore/AuthorizationError.cs deleted file mode 100644 index a02f2126..00000000 --- a/src/Authorization.AspNetCore/AuthorizationError.cs +++ /dev/null @@ -1,34 +0,0 @@ -#nullable enable - -using GraphQL.Validation; -using GraphQLParser.AST; -using Microsoft.AspNetCore.Authorization; - -namespace GraphQL.Server.Authorization.AspNetCore; - -/// -/// 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 with a specific error message. - /// - public AuthorizationError(ASTNode? node, ValidationContext context, string message, AuthorizationResult result, OperationType? operationType = null) - : base(context.Document.Source, "6.1.1", message, node == null ? Array.Empty() : new ASTNode[] { node }) - { - Code = "authorization"; - AuthorizationResult = result; - OperationType = operationType; - } - - /// - /// Returns the result of the ASP.NET Core authorization request. - /// - public virtual AuthorizationResult AuthorizationResult { get; } - - /// - /// The GraphQL operation type. - /// - public OperationType? OperationType { get; } -} diff --git a/src/Authorization.AspNetCore/AuthorizationValidationRule.cs b/src/Authorization.AspNetCore/AuthorizationValidationRule.cs deleted file mode 100644 index 4934d7cb..00000000 --- a/src/Authorization.AspNetCore/AuthorizationValidationRule.cs +++ /dev/null @@ -1,214 +0,0 @@ -#nullable enable - -using System.Security.Claims; -using GraphQL.Types; -using GraphQL.Validation; -using GraphQLParser; -using GraphQLParser.AST; -using GraphQLParser.Visitors; -using Microsoft.AspNetCore.Authorization; - -namespace GraphQL.Server.Authorization.AspNetCore; - -/// -/// GraphQL authorization validation rule which integrates to ASP.NET Core authorization mechanism. -/// For more information see https://docs.microsoft.com/en-us/aspnet/core/security/authorization/introduction. -/// -public class AuthorizationValidationRule : IValidationRule -{ - private readonly IAuthorizationService _authorizationService; - private readonly IClaimsPrincipalAccessor _claimsPrincipalAccessor; - private readonly IAuthorizationErrorMessageBuilder _messageBuilder; - - /// - /// Creates an instance of . - /// - /// ASP.NET Core to authorize against. - /// The which provides the for authorization. - /// The which is used to generate the message for an . - public AuthorizationValidationRule( - IAuthorizationService authorizationService, - IClaimsPrincipalAccessor claimsPrincipalAccessor, - IAuthorizationErrorMessageBuilder messageBuilder) - { - _authorizationService = authorizationService; - _claimsPrincipalAccessor = claimsPrincipalAccessor; - _messageBuilder = messageBuilder; - } - - private bool ShouldBeSkipped(GraphQLOperationDefinition actualOperation, ValidationContext context) - { - if (context.Document.OperationsCount() <= 1) - { - return false; - } - - int i = 0; - do - { - var ancestor = context.TypeInfo.GetAncestor(i++); - - if (ancestor == actualOperation) - { - return false; - } - - if (ancestor == context.Document) - { - return true; - } - - if (ancestor is GraphQLFragmentDefinition fragment) - { - //TODO: may be rewritten completely later - var c = new FragmentBelongsToOperationVisitorContext(fragment); - _visitor.VisitAsync(actualOperation, c).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this - return !c.Found; - } - } while (true); - } - - private sealed class FragmentBelongsToOperationVisitorContext : IASTVisitorContext - { - public FragmentBelongsToOperationVisitorContext(GraphQLFragmentDefinition fragment) - { - Fragment = fragment; - } - - public GraphQLFragmentDefinition Fragment { get; } - - public bool Found { get; set; } - - public CancellationToken CancellationToken => default; - } - - private static readonly FragmentBelongsToOperationVisitor _visitor = new(); - - private sealed class FragmentBelongsToOperationVisitor : ASTVisitor - { - protected override ValueTask VisitFragmentSpreadAsync(GraphQLFragmentSpread fragmentSpread, FragmentBelongsToOperationVisitorContext context) - { - context.Found = context.Fragment.FragmentName.Name == fragmentSpread.FragmentName.Name; - return default; - } - - public override ValueTask VisitAsync(ASTNode? node, FragmentBelongsToOperationVisitorContext context) - { - return context.Found ? default : base.VisitAsync(node, context); - } - } - - /// - public async ValueTask ValidateAsync(ValidationContext context) - { - await AuthorizeAsync(null, context.Schema, context, null); - var operationType = OperationType.Query; - - // this could leak info about hidden fields or types in error messages - // it would be better to implement a filter on the Schema so it - // acts as if they just don't exist vs. an auth denied error - // - filtering the Schema is not currently supported - // TODO: apply ISchemaFilter - context.Schema.Filter.AllowXXX - return new NodeVisitors( - new MatchingNodeVisitor((astType, context) => - { - if (context.Document.OperationsCount() > 1 && astType.Name != context.Operation.Name) - { - return; - } - - operationType = astType.Operation; - - var type = context.TypeInfo.GetLastType(); - AuthorizeAsync(astType, type, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this - }), - - new MatchingNodeVisitor((objectFieldAst, context) => - { - if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is IComplexGraphType argumentType && !ShouldBeSkipped(context.Operation, context)) - { - var fieldType = argumentType.GetField(objectFieldAst.Name); - AuthorizeAsync(objectFieldAst, fieldType, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this - } - }), - - new MatchingNodeVisitor((fieldAst, context) => - { - var fieldDef = context.TypeInfo.GetFieldDef(); - - if (fieldDef == null || ShouldBeSkipped(context.Operation, context)) - return; - - // check target field - AuthorizeAsync(fieldAst, fieldDef, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this - // check returned graph type - AuthorizeAsync(fieldAst, fieldDef.ResolvedType?.GetNamedType(), context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this - }), - - new MatchingNodeVisitor((variableRef, context) => - { - if (context.TypeInfo.GetArgument()?.ResolvedType?.GetNamedType() is not IComplexGraphType variableType || ShouldBeSkipped(context.Operation, context)) - return; - - AuthorizeAsync(variableRef, variableType, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; - - // Check each supplied field in the variable that exists in the variable type. - // If some supplied field does not exist in the variable type then some other - // validation rule should check that but here we should just ignore that - // "unknown" field. - if (context.Variables != null && - context.Variables.TryGetValue(variableRef.Name.StringValue, out object? input) && //ISSUE:allocation - input is Dictionary fieldsValues) - { - foreach (var field in variableType.Fields) - { - if (fieldsValues.ContainsKey(field.Name)) - { - AuthorizeAsync(variableRef, field, context, operationType).GetAwaiter().GetResult(); // TODO: need to think of something to avoid this; - } - } - } - }) - ); - } - - private async Task AuthorizeAsync(ASTNode? node, IProvideMetadata? provider, ValidationContext context, OperationType? operationType) - { - var policyNames = provider?.GetPolicies(); - - if (policyNames?.Count == 1) - { - // small optimization for the single policy - no 'new List<>()', no 'await Task.WhenAll()' - var authorizationResult = await _authorizationService.AuthorizeAsync(_claimsPrincipalAccessor.GetClaimsPrincipal(context), policyNames[0]); - if (!authorizationResult.Succeeded) - AddValidationError(node, context, operationType, authorizationResult); - } - else if (policyNames?.Count > 1) - { - var claimsPrincipal = _claimsPrincipalAccessor.GetClaimsPrincipal(context); - var tasks = new List>(policyNames.Count); - foreach (string policyName in policyNames) - { - var task = _authorizationService.AuthorizeAsync(claimsPrincipal, policyName); - tasks.Add(task); - } - - var authorizationResults = await Task.WhenAll(tasks); - - foreach (var result in authorizationResults) - { - if (!result.Succeeded) - AddValidationError(node, context, operationType, result); - } - } - } - - /// - /// Adds an authorization failure error to the document response - /// - protected virtual void AddValidationError(ASTNode? node, ValidationContext context, OperationType? operationType, AuthorizationResult result) - { - string message = _messageBuilder.GenerateMessage(operationType, result); - context.ReportError(new AuthorizationError(node, context, message, result, operationType)); - } -} diff --git a/src/Authorization.AspNetCore/DefaultAuthorizationErrorMessageBuilder.cs b/src/Authorization.AspNetCore/DefaultAuthorizationErrorMessageBuilder.cs deleted file mode 100644 index 14905947..00000000 --- a/src/Authorization.AspNetCore/DefaultAuthorizationErrorMessageBuilder.cs +++ /dev/null @@ -1,112 +0,0 @@ -#nullable enable - -using System.Text; -using GraphQLParser.AST; -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Authorization.Infrastructure; - -namespace GraphQL.Server.Authorization.AspNetCore; - -public class DefaultAuthorizationErrorMessageBuilder : IAuthorizationErrorMessageBuilder -{ - /// - public virtual string GenerateMessage(OperationType? operationType, AuthorizationResult result) - { - if (result.Succeeded) - return "Success!"; - - var error = new StringBuilder(); - AppendFailureHeader(error, operationType); - - if (result.Failure != null) - { - foreach (var requirement in result.Failure.FailedRequirements) - { - AppendFailureLine(error, requirement); - } - } - - return error.ToString(); - } - - private string GetOperationType(OperationType? operationType) - { - return operationType switch - { - OperationType.Query => "query", - OperationType.Mutation => "mutation", - OperationType.Subscription => "subscription", - _ => "operation", - }; - } - - /// - public virtual void AppendFailureHeader(StringBuilder error, OperationType? operationType) - { - error - .Append("You are not authorized to run this ") - .Append(GetOperationType(operationType)) - .Append('.'); - } - - /// - public virtual void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement) - { - error.AppendLine(); - - switch (authorizationRequirement) - { - 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)); - error.Append("' is not present."); - } - break; - - case DenyAnonymousAuthorizationRequirement _: - error.Append("The current user must be authenticated."); - break; - - case NameAuthorizationRequirement nameAuthorizationRequirement: - error.Append("The current user name must match the name '"); - error.Append(nameAuthorizationRequirement.RequiredName); - error.Append("'."); - break; - - case OperationAuthorizationRequirement operationAuthorizationRequirement: - error.Append("Required operation '"); - error.Append(operationAuthorizationRequirement.Name); - error.Append("' was not present."); - break; - - case RolesAuthorizationRequirement rolesAuthorizationRequirement: - if (rolesAuthorizationRequirement.AllowedRoles == null || !rolesAuthorizationRequirement.AllowedRoles.Any()) - { - // This should never happen. - error.Append("Required roles are not present."); - } - else - { - error.Append("Required roles '"); - error.Append(string.Join(", ", rolesAuthorizationRequirement.AllowedRoles)); - error.Append("' are not present."); - } - break; - - case AssertionRequirement _: - default: - error.Append("Requirement '"); - error.Append(authorizationRequirement.GetType().Name); - error.Append("' was not satisfied."); - break; - } - } -} diff --git a/src/Authorization.AspNetCore/DefaultClaimsPrincipalAccessor.cs b/src/Authorization.AspNetCore/DefaultClaimsPrincipalAccessor.cs deleted file mode 100644 index b3204eae..00000000 --- a/src/Authorization.AspNetCore/DefaultClaimsPrincipalAccessor.cs +++ /dev/null @@ -1,32 +0,0 @@ -using System.Security.Claims; -using GraphQL.Validation; -using Microsoft.AspNetCore.Http; - -namespace GraphQL.Server.Authorization.AspNetCore; - -/// -/// The default claims principal accessor. -/// -public class DefaultClaimsPrincipalAccessor : IClaimsPrincipalAccessor -{ - private readonly IHttpContextAccessor _contextAccessor; - - /// - /// Creates an instance of . - /// - /// ASP.NET Core to take claims principal () from. - public DefaultClaimsPrincipalAccessor(IHttpContextAccessor contextAccessor) - { - _contextAccessor = contextAccessor ?? throw new ArgumentNullException(nameof(contextAccessor)); - } - - /// - /// Returns the . - /// - /// - /// - public ClaimsPrincipal GetClaimsPrincipal(ValidationContext context) - { - return _contextAccessor.HttpContext?.User; - } -} diff --git a/src/Authorization.AspNetCore/GraphQLBuilderAuthorizationExtensions.cs b/src/Authorization.AspNetCore/GraphQLBuilderAuthorizationExtensions.cs deleted file mode 100644 index 307052a8..00000000 --- a/src/Authorization.AspNetCore/GraphQLBuilderAuthorizationExtensions.cs +++ /dev/null @@ -1,45 +0,0 @@ -#nullable enable - -using GraphQL.DI; -using GraphQL.Server.Authorization.AspNetCore; -using Microsoft.AspNetCore.Authorization; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.DependencyInjection.Extensions; - -namespace GraphQL.Server; - -public static class GraphQLBuilderAuthorizationExtensions -{ - /// - /// Adds the GraphQL authorization. - /// - /// The GraphQL builder. - /// Reference to the passed . - public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder builder) - => builder.AddGraphQLAuthorization(_ => { }); - - /// - /// Adds the GraphQL authorization. - /// - /// The GraphQL builder. - /// An action delegate to configure the provided . - /// Reference to the passed . - public static IGraphQLBuilder AddGraphQLAuthorization(this IGraphQLBuilder builder, Action? configure) - { - if (builder.Services is not IServiceCollection services) - throw new NotSupportedException("This method only supports the MicrosoftDI implementation of IGraphQLBuilder."); - - services.TryAddTransient(); - services.TryAddTransient(); - services.AddHttpContextAccessor(); - - if (configure != null) - services.AddAuthorizationCore(configure); - else - services.AddAuthorizationCore(); - - builder.AddValidationRule(); - - return builder; - } -} diff --git a/src/Authorization.AspNetCore/IAuthorizationErrorMessageBuilder.cs b/src/Authorization.AspNetCore/IAuthorizationErrorMessageBuilder.cs deleted file mode 100644 index 8bfdbbf0..00000000 --- a/src/Authorization.AspNetCore/IAuthorizationErrorMessageBuilder.cs +++ /dev/null @@ -1,32 +0,0 @@ -#nullable enable - -using System.Text; -using GraphQLParser.AST; -using Microsoft.AspNetCore.Authorization; - -namespace GraphQL.Server.Authorization.AspNetCore; - -public interface IAuthorizationErrorMessageBuilder -{ - /// - /// Generates an authorization error message based on the provided - /// - /// The GraphQL operation type. - /// The which is used to generate the message. - /// The generated error message. - string GenerateMessage(OperationType? operationType, AuthorizationResult result); - - /// - /// Appends the error message header to the provided . - /// - /// The error message . - /// The GraphQL operation type. - void AppendFailureHeader(StringBuilder error, OperationType? operationType); - - /// - /// Appends a description of the failed to the supplied . - /// - /// The which is used to compose the error message. - /// The failed . - void AppendFailureLine(StringBuilder error, IAuthorizationRequirement authorizationRequirement); -} diff --git a/src/Authorization.AspNetCore/IClaimsPrincipalAccessor.cs b/src/Authorization.AspNetCore/IClaimsPrincipalAccessor.cs deleted file mode 100644 index 3d475d08..00000000 --- a/src/Authorization.AspNetCore/IClaimsPrincipalAccessor.cs +++ /dev/null @@ -1,17 +0,0 @@ -using System.Security.Claims; -using GraphQL.Validation; - -namespace GraphQL.Server.Authorization.AspNetCore; - -/// -/// 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/Transports.AspNetCore/AuthorizationValidationRule.cs b/src/Transports.AspNetCore/AuthorizationValidationRule.cs new file mode 100644 index 00000000..39d51ed8 --- /dev/null +++ b/src/Transports.AspNetCore/AuthorizationValidationRule.cs @@ -0,0 +1,550 @@ +using System.Security.Claims; +using GraphQL.Server.Transports.AspNetCore.Errors; +using GraphQL.Types; +using GraphQL.Validation; +using GraphQLParser.AST; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; + +namespace GraphQL.Server.Transports.AspNetCore; + +/// +/// Validates a document against the configured set of policy and role requirements. +/// +public class AuthorizationValidationRule : IValidationRule +{ + private readonly IHttpContextAccessor _contextAccessor; + + /// + public AuthorizationValidationRule(IHttpContextAccessor httpContextAccessor) + { + _contextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); + } + + /// + public ValueTask ValidateAsync(ValidationContext context) + { + var httpContext = _contextAccessor.HttpContext ?? NoHttpContext(); + var user = httpContext.User ?? NoUser(); + var provider = context.RequestServices ?? NoRequestServices(); + var authService = provider.GetService() ?? NoAuthServiceError(); + + var visitor = new AuthorizationVisitor(context, user, authService); + return visitor.ValidateSchema(context) ? new(visitor) : default; + } + + private static HttpContext NoHttpContext() + => throw new InvalidOperationException("HttpContext could not be retrieved from IHttpContextAccessor."); + + private static ClaimsPrincipal NoUser() + => throw new InvalidOperationException("ClaimsPrincipal could not be retrieved from HttpContext.User."); + + private static IServiceProvider NoRequestServices() + => throw new MissingRequestServicesException(); + + private static IAuthorizationService NoAuthServiceError() + => throw new InvalidOperationException("An instance of IAuthorizationService could not be pulled from the dependency injection framework."); + + /// + public class AuthorizationVisitor : INodeVisitor + { + /// + public AuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPrincipal, IAuthorizationService authorizationService) + { + if (context == null) + throw new ArgumentNullException(nameof(context)); + ClaimsPrincipal = claimsPrincipal ?? throw new ArgumentNullException(nameof(claimsPrincipal)); + if (claimsPrincipal.Identity == null) + throw new InvalidOperationException($"{nameof(claimsPrincipal)}.Identity cannot be null."); + AuthorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); + _fragmentDefinitionsToCheck = context.GetRecursivelyReferencedFragments(context.Operation); + _userIsAuthenticated = claimsPrincipal.Identity.IsAuthenticated; + } + + /// + /// The user that this authorization visitor will authenticate against. + /// + public ClaimsPrincipal ClaimsPrincipal { get; } + + /// + /// The authorization service that is used to authorize policy requests. + /// + public IAuthorizationService AuthorizationService { get; } + + private bool _checkTree; // used to skip processing fragments that do not apply + private ASTNode? _checkUntil; + private readonly List? _fragmentDefinitionsToCheck; // contains a list of fragments to process, or null if none + private Dictionary? _policyResults; // contains a dictionary of policies that have been checked + private Dictionary? _roleResults; // contains a dictionary of roles that have been checked + private readonly Stack _onlyAnonymousSelected = new(); + private readonly bool _userIsAuthenticated; + private Dictionary? _fragments; + private List? _todo; + + private class TypeInfo + { + public bool AnyAuthenticated; + public bool AnyAnonymous; + public List? WaitingOnFragments; + } + + private class TodoInfo + { + public ValidationInfo ValidationInfo { get; } + public bool AnyAuthenticated; + public bool AnyAnonymous; + public List WaitingOnFragments; + + public TodoInfo(ValidationInfo vi, TypeInfo ti) + { + ValidationInfo = vi; + AnyAuthenticated = ti.AnyAuthenticated; + AnyAnonymous = ti.AnyAnonymous; + WaitingOnFragments = ti.WaitingOnFragments ?? NoWaitingOnFragments(); + } + + private static List NoWaitingOnFragments() + => throw new InvalidOperationException("Waiting on fragments must not be null."); + } + + /// + public virtual void Enter(ASTNode node, ValidationContext context) + { + if (node == context.Operation || (node is GraphQLFragmentDefinition fragmentDefinition && _fragmentDefinitionsToCheck != null && _fragmentDefinitionsToCheck.Contains(fragmentDefinition))) + { + var type = context.TypeInfo.GetLastType()?.GetNamedType(); + if (type != null) + { + // if type is null that means that no type was configured for this operation in the schema; will produce a separate validation error + _onlyAnonymousSelected.Push(new()); + _checkTree = true; + } + } + else if (_checkTree) + { + if (node is GraphQLField fieldNode) + { + if (SkipField(fieldNode, context)) + { + _checkTree = false; + _checkUntil = node; + } + else + { + var field = context.TypeInfo.GetFieldDef(); + // might be null if no match was found in the schema + // and skip processing for __typeName + if (field != null && field != context.Schema.TypeNameMetaFieldType) + { + var fieldAnonymousAllowed = field.IsAnonymousAllowed() || field == context.Schema.TypeMetaFieldType || field == context.Schema.SchemaMetaFieldType; + var ti = _onlyAnonymousSelected.Peek(); + if (fieldAnonymousAllowed) + ti.AnyAnonymous = true; + else + ti.AnyAuthenticated = true; + + if (!fieldAnonymousAllowed) + { + Validate(field, node, context); + } + } + // prep for descendants, if any + _onlyAnonymousSelected.Push(new()); + } + } + else if (node is GraphQLFragmentSpread fragmentSpread) + { + var ti = _onlyAnonymousSelected.Peek(); + var fragmentName = fragmentSpread.FragmentName.Name.StringValue; + if (_fragments?.TryGetValue(fragmentName, out var fragmentInfo) == true) + { + ti.AnyAuthenticated |= fragmentInfo.AnyAuthenticated; + ti.AnyAnonymous |= fragmentInfo.AnyAnonymous; + if (fragmentInfo.WaitingOnFragments?.Count > 0) + { + ti.WaitingOnFragments ??= new(); + ti.WaitingOnFragments.AddRange(fragmentInfo.WaitingOnFragments); + } + } + else + { + ti.WaitingOnFragments ??= new(); + ti.WaitingOnFragments.Add(fragmentName); + } + } + else if (node is GraphQLArgument) + { + // ignore arguments of directives + if (context.TypeInfo.GetAncestor(2)?.Kind == ASTNodeKind.Field) + { + // verify field argument + var arg = context.TypeInfo.GetArgument(); + if (arg != null) + { + Validate(arg, node, context); + } + } + } + } + } + + /// + public virtual void Leave(ASTNode node, ValidationContext context) + { + if (!_checkTree) + { + if (_checkUntil == node) + { + _checkTree = true; + _checkUntil = null; + } + return; + } + if (node == context.Operation) + { + _checkTree = false; + PopAndProcess(); + } + else if (node is GraphQLFragmentDefinition fragmentDefinition) + { + _checkTree = false; + var fragmentName = fragmentDefinition.FragmentName.Name.StringValue; + var ti = _onlyAnonymousSelected.Pop(); + RecursiveResolve(fragmentName, ti, context); + _fragments ??= new(); + _fragments.TryAdd(fragmentName, ti); + } + else if (_checkTree && node is GraphQLField) + { + PopAndProcess(); + } + + void PopAndProcess() + { + var info = _onlyAnonymousSelected.Pop(); + var type = context.TypeInfo.GetLastType()?.GetNamedType(); + if (type == null) + return; + if (info.AnyAuthenticated || (!info.AnyAnonymous && (info.WaitingOnFragments?.Count ?? 0) == 0)) + { + Validate(type, node, context); + } + else if (info.WaitingOnFragments?.Count > 0) + { + _todo ??= new(); + _todo.Add(new(BuildValidationInfo(type, node, context), info)); + } + } + } + + /// + /// Indicates if the specified field should skip authentication processing. + /// Default implementation looks at @skip and @include directives only. + /// + protected virtual bool SkipField(GraphQLField node, ValidationContext context) + { + // check + var skipDirective = node.Directives?.FirstOrDefault(x => x.Name == "skip"); + if (skipDirective != null) + { + var value = GetDirectiveValue(skipDirective, context, false); + if (value) + return true; + } + + var includeDirective = node.Directives?.FirstOrDefault(x => x.Name == "include"); + if (includeDirective != null) + { + var value = GetDirectiveValue(includeDirective, context, true); + if (!value) + return true; + } + + return false; + + static bool GetDirectiveValue(GraphQLDirective directive, ValidationContext context, bool defaultValue) + { + var ifArg = directive.Arguments?.FirstOrDefault(x => x.Name == "if"); + if (ifArg != null) + { + if (ifArg.Value is GraphQLBooleanValue boolValue) + { + return boolValue.BoolValue; + } + else if (ifArg.Value is GraphQLVariable variable) + { + if (context.Operation.Variables != null) + { + var varDef = context.Operation.Variables.FirstOrDefault(x => x.Variable.Name == variable.Name); + if (varDef != null && varDef.Type.Name() == "Boolean") + { + if (context.Variables.TryGetValue(variable.Name.StringValue, out var value)) + { + if (value is bool boolValue2) + return boolValue2; + } + if (varDef.DefaultValue is GraphQLBooleanValue boolValue3) + { + return boolValue3.BoolValue; + } + } + } + } + } + return defaultValue; + } + } + + // runs when a fragment is added or updated; the fragment might not be waiting on any + // other fragments, or it still might be + private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContext context) + { + // first see if any other fragments are waiting on this fragment + if (_fragments != null) + { + foreach (var fragment in _fragments) + { + var ti2 = fragment.Value; + if (ti2.WaitingOnFragments != null && ti2.WaitingOnFragments.Remove(fragmentName)) + { + ti2.AnyAuthenticated |= ti.AnyAuthenticated; + ti2.AnyAnonymous |= ti.AnyAnonymous; + RecursiveResolve(fragment.Key, ti2, context); + } + } + } + // then, if this fragment is fully resolved, check to see if any nodes are waiting for this fragment + if ((ti.WaitingOnFragments?.Count ?? 0) == 0) + { + if (_todo != null) + { + var count = _todo.Count; + for (var i = 0; i < count; i++) + { + var todo = _todo[i]; + if (todo.WaitingOnFragments.Remove(fragmentName)) + { + todo.AnyAuthenticated |= ti.AnyAuthenticated; + todo.AnyAnonymous |= ti.AnyAnonymous; + if (todo.WaitingOnFragments.Count == 0) + { + _todo.RemoveAt(i); + count--; + if (todo.AnyAuthenticated || !todo.AnyAnonymous) + { + Validate(todo.ValidationInfo); + } + } + } + } + } + } + } + + /// + /// Validates authorization rules for the schema. + /// Returns a value indicating if validation was successful. + /// + public virtual bool ValidateSchema(ValidationContext context) + => Validate(context.Schema, null, context); + + /// + /// Validate a node that is current within the context. + /// + private bool Validate(IProvideMetadata obj, ASTNode? node, ValidationContext context) + => Validate(BuildValidationInfo(obj, node, context)); + + private static ValidationInfo BuildValidationInfo(IProvideMetadata obj, ASTNode? node, ValidationContext context) + { + IFieldType? parentFieldType = null; + IGraphType? parentGraphType = null; + if (node is GraphQLField) + { + if (obj is IGraphType) + { + parentFieldType = context.TypeInfo.GetFieldDef(0); + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + else if (obj is IFieldType) + { + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + } + else if (node is GraphQLArgument) + { + parentFieldType = context.TypeInfo.GetFieldDef(); + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + return new(obj, node, parentFieldType, parentGraphType, context); + } + + /// Provides contextual information to the schema, graph, field, or query argument being validated. + /// The schema, graph type, field type, or query argument being validated. May be an interface type if fragments are in use. + /// Null for a schema validation; otherwise the , , or being validated. + /// The validaion context; but may not be applicable for node being validated. + /// For graph types other than operations, the field where this type was referenced; for query arguments, the field to which this argument belongs. + /// For graph types, the graph type for the field where this type was referenced; for field types, the graph type to which this field belongs; for query arguments, the graph type for the field to which this argument belongs. + public record struct ValidationInfo(IProvideMetadata Obj, ASTNode? Node, IFieldType? ParentFieldType, IGraphType? ParentGraphType, ValidationContext Context); + + /// + /// Validates authorization rules for the specified schema, graph, field or query argument. + /// Does not consider . + /// Returns a value indicating if validation was successful for this node. + /// + protected virtual bool Validate(ValidationInfo info) + { + bool requiresAuthorization = info.Obj.IsAuthorizationRequired(); + if (!requiresAuthorization) + return true; + + var success = true; + var policies = info.Obj.GetPolicies(); + if (policies?.Count > 0) + { + requiresAuthorization = false; + _policyResults ??= new Dictionary(); + foreach (var policy in policies) + { + if (!_policyResults.TryGetValue(policy, out var result)) + { + result = AuthorizePolicy(policy); + _policyResults.Add(policy, result); + } + if (!result.Succeeded) + { + HandleNodeNotInPolicy(info, policy, result); + success = false; + } + } + } + + var roles = info.Obj.GetRoles(); + if (roles?.Count > 0) + { + requiresAuthorization = false; + _roleResults ??= new Dictionary(); + foreach (var role in roles) + { + if (!_roleResults.TryGetValue(role, out var result)) + { + result = AuthorizeRole(role); + _roleResults.Add(role, result); + } + if (result) + goto PassRoles; + } + HandleNodeNotInRoles(info, roles); + success = false; + } + PassRoles: + + if (requiresAuthorization) + { + if (!Authorize()) + { + HandleNodeNotAuthorized(info); + success = false; + } + } + + return success; + } + + /// + protected virtual bool Authorize() + => _userIsAuthenticated; + + /// + protected virtual bool AuthorizeRole(string role) + => ClaimsPrincipal.IsInRole(role); + + /// + protected virtual AuthorizationResult AuthorizePolicy(string policy) + => AuthorizePolicyAsync(policy).GetAwaiter().GetResult(); + + /// + /// Adds a error to the validation context indicating that the user is not authenticated + /// as required by this graph, field or query argument. + /// + /// Information about the node being validated. + protected virtual void HandleNodeNotAuthorized(ValidationInfo info) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + info.Context.ReportError(err); + } + + /// + /// Adds a error to the validation context indicating that the user is not a member of any of + /// the roles required by this graph, field or query argument. + /// + /// Information about the node being validated. + /// The list of roles of which the user must be a member. + protected virtual void HandleNodeNotInRoles(ValidationInfo info, List roles) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + err.RolesRequired = roles; + info.Context.ReportError(err); + } + + /// + /// Adds a error to the validation context indicating that the user is not a member of any of + /// the roles required by this graph, field or query argument. + /// + /// Information about the node being validated. + /// The policy which these nodes are being authenticated against. + /// The result of the authentication request. + protected virtual void HandleNodeNotInPolicy(ValidationInfo info, string policy, AuthorizationResult authorizationResult) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + err.PolicyRequired = policy; + err.PolicyAuthorizationResult = authorizationResult; + info.Context.ReportError(err); + } + + /// + /// Generates a friendly name for a specified graph, field or query argument. + /// + protected virtual string GenerateResourceDescription(ValidationInfo info) + { + if (info.Obj is ISchema) + { + return "schema"; + } + else if (info.Obj is IGraphType graphType) + { + if (info.Node is GraphQLField) + { + return $"type '{graphType.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else if (info.Node is GraphQLOperationDefinition op) + { + return $"type '{graphType.Name}' for {op.Operation.ToString().ToLower(System.Globalization.CultureInfo.InvariantCulture)} operation{(!string.IsNullOrEmpty(op.Name?.StringValue) ? $" '{op.Name}'" : null)}"; + } + else + { + return $"type '{graphType.Name}'"; + } + } + else if (info.Obj is IFieldType fieldType) + { + return $"field '{fieldType.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else if (info.Obj is QueryArgument queryArgument) + { + return $"argument '{queryArgument.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else + { + return info.Node?.GetType().Name ?? "unknown"; + } + } + + /// + protected virtual Task AuthorizePolicyAsync(string policy) + => AuthorizationService.AuthorizeAsync(ClaimsPrincipal, policy); + } +} diff --git a/tests/Authorization.AspNetCore.Tests/Authorization.AspNetCore.Tests.csproj b/tests/Authorization.AspNetCore.Tests/Authorization.AspNetCore.Tests.csproj index 468839db..3e8806d3 100644 --- a/tests/Authorization.AspNetCore.Tests/Authorization.AspNetCore.Tests.csproj +++ b/tests/Authorization.AspNetCore.Tests/Authorization.AspNetCore.Tests.csproj @@ -7,7 +7,7 @@ - + diff --git a/tests/Authorization.AspNetCore.Tests/AuthorizationValidationRuleTests.cs b/tests/Authorization.AspNetCore.Tests/AuthorizationValidationRuleTests.cs index a152cda1..cf674462 100644 --- a/tests/Authorization.AspNetCore.Tests/AuthorizationValidationRuleTests.cs +++ b/tests/Authorization.AspNetCore.Tests/AuthorizationValidationRuleTests.cs @@ -48,8 +48,7 @@ public void policy_on_schema_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this operation. -Required claim 'some' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for schema."); }; }); } @@ -82,8 +81,7 @@ public void class_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for type 'Query' for query operation."); }; }); } @@ -132,8 +130,7 @@ public void method_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for field 'post' on type 'Query'."); }; }); } @@ -150,8 +147,7 @@ public void property_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for field 'post' on type 'Query'."); }; }); } @@ -184,8 +180,7 @@ public void nested_type_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for type 'Post' for field 'post' on type 'Query'."); }; }); } @@ -218,8 +213,7 @@ public void nested_type_list_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for type 'Post' for field 'posts' on type 'Query'."); }; }); } @@ -236,13 +230,12 @@ public void nested_type_list_non_null_policy_fail() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for type 'Post' for field 'postsNonNull' on type 'Query'."); }; }); } - [Fact] + [Fact(Skip = "Authorization for input types is not yet supported")] public void fails_on_missing_claim_on_input_type() { ConfigureAuthorizationOptions(options => options.AddPolicy("FieldPolicy", x => x.RequireClaim("admin"))); @@ -254,8 +247,7 @@ public void fails_on_missing_claim_on_input_type() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for type 'AuthorInputType' for field 'author' on type 'Query'."); }; }); } @@ -289,8 +281,7 @@ public void fails_on_missing_claim_on_connection_type() config.ValidateResult = result => { result.Errors.Count.ShouldBe(1); - result.Errors[0].Message.ShouldBe(@"You are not authorized to run this query. -Required claim 'admin' is not present."); + result.Errors[0].Message.ShouldBe(@"Access denied for field 'posts' on type 'Object'."); }; }); } diff --git a/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs b/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs index 6994c5b9..55e8000b 100644 --- a/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs +++ b/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using GraphQL.Execution; +using GraphQL.Server.Transports.AspNetCore; using GraphQL.Validation; using GraphQLParser.AST; using Microsoft.AspNetCore.Authorization; @@ -20,7 +21,7 @@ protected void ConfigureAuthorizationOptions(Action setupO { var (authorizationService, httpContextAccessor) = BuildServices(setupOptions); HttpContext = httpContextAccessor.HttpContext; - Rule = new AuthorizationValidationRule(authorizationService, new DefaultClaimsPrincipalAccessor(httpContextAccessor), new DefaultAuthorizationErrorMessageBuilder()); + Rule = new AuthorizationValidationRule(httpContextAccessor); } protected void ShouldPassRule(Action configure) @@ -92,7 +93,8 @@ private IValidationResult Validate(ValidationTestConfig config) Document = document, Operation = document.Definitions.OfType().First(), Rules = config.Rules, - Variables = config.Variables + Variables = config.Variables, + RequestServices = ServiceProvider, }).GetAwaiter().GetResult().validationResult; } From f07bc7d0e8af47de622bb717fa7cbc47a50d84e4 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 21 May 2022 10:35:46 -0400 Subject: [PATCH 02/16] Updates --- .../Samples.Server/CustomErrorInfoProvider.cs | 27 ++++-------- samples/Samples.Server/Samples.Server.csproj | 1 - samples/Samples.Server/Startup.cs | 4 +- samples/Samples.Server/StartupWithRouting.cs | 4 +- ...ensions.cs => GraphQLBuilderExtensions.cs} | 13 +++++- .../GraphQLHttpMiddleware.cs | 4 +- tests/ApiApprovalTests/ApiApprovalTests.cs | 1 - ....Server.Transports.AspNetCore.approved.txt | 43 ++++++++++++++++++- 8 files changed, 66 insertions(+), 31 deletions(-) rename src/Transports.AspNetCore/Extensions/{GraphQLBuilderUserContextExtensions.cs => GraphQLBuilderExtensions.cs} (89%) diff --git a/samples/Samples.Server/CustomErrorInfoProvider.cs b/samples/Samples.Server/CustomErrorInfoProvider.cs index 533750ea..4e869477 100644 --- a/samples/Samples.Server/CustomErrorInfoProvider.cs +++ b/samples/Samples.Server/CustomErrorInfoProvider.cs @@ -1,6 +1,6 @@ using System.Text; using GraphQL.Execution; -using GraphQL.Server.Authorization.AspNetCore; +using GraphQL.Server.Transports.AspNetCore.Errors; using Microsoft.AspNetCore.Authorization; namespace GraphQL.Samples.Server; @@ -11,30 +11,22 @@ namespace GraphQL.Samples.Server; /// public class CustomErrorInfoProvider : ErrorInfoProvider { - private readonly IAuthorizationErrorMessageBuilder _messageBuilder; - - public CustomErrorInfoProvider(IAuthorizationErrorMessageBuilder messageBuilder) - { - _messageBuilder = messageBuilder; - } - public override ErrorInfo GetInfo(ExecutionError executionError) { var info = base.GetInfo(executionError); - info.Message = executionError switch - { - AuthorizationError authorizationError => GetAuthorizationErrorMessage(authorizationError), - _ => info.Message, - }; + + if (executionError is AccessDeniedError accessDeniedError) + info.Message = GetAuthorizationErrorMessage(accessDeniedError); + return info; } - private string GetAuthorizationErrorMessage(AuthorizationError error) + private string GetAuthorizationErrorMessage(AccessDeniedError error) { var errorMessage = new StringBuilder(); - _messageBuilder.AppendFailureHeader(errorMessage, error.OperationType); + errorMessage.Append(error.Message); - foreach (var failedRequirement in error.AuthorizationResult.Failure.FailedRequirements) + foreach (var failedRequirement in error.PolicyAuthorizationResult.Failure.FailedRequirements) { switch (failedRequirement) { @@ -44,9 +36,6 @@ private string GetAuthorizationErrorMessage(AuthorizationError error) errorMessage.Append(minimumAgeRequirement.MinimumAge); errorMessage.Append(" years old."); break; - default: - _messageBuilder.AppendFailureLine(errorMessage, failedRequirement); - break; } } diff --git a/samples/Samples.Server/Samples.Server.csproj b/samples/Samples.Server/Samples.Server.csproj index 30ee78e8..f06278e9 100644 --- a/samples/Samples.Server/Samples.Server.csproj +++ b/samples/Samples.Server/Samples.Server.csproj @@ -18,7 +18,6 @@ - diff --git a/samples/Samples.Server/Startup.cs b/samples/Samples.Server/Startup.cs index ee80f2e8..b4bfd3ff 100644 --- a/samples/Samples.Server/Startup.cs +++ b/samples/Samples.Server/Startup.cs @@ -3,7 +3,6 @@ using GraphQL.MicrosoftDI; using GraphQL.Samples.Schemas.Chat; using GraphQL.Server; -using GraphQL.Server.Authorization.AspNetCore; using GraphQL.Server.Transports.AspNetCore; using GraphQL.Server.Ui.Altair; using GraphQL.Server.Ui.GraphiQL; @@ -30,8 +29,7 @@ public void ConfigureServices(IServiceCollection services) { services .AddSingleton() - .Configure(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment()) - .AddTransient(); // required by CustomErrorInfoProvider + .Configure(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment()); services.AddGraphQL(builder => builder .AddApolloTracing() diff --git a/samples/Samples.Server/StartupWithRouting.cs b/samples/Samples.Server/StartupWithRouting.cs index 4df68fa0..394669ca 100644 --- a/samples/Samples.Server/StartupWithRouting.cs +++ b/samples/Samples.Server/StartupWithRouting.cs @@ -3,7 +3,6 @@ using GraphQL.MicrosoftDI; using GraphQL.Samples.Schemas.Chat; using GraphQL.Server; -using GraphQL.Server.Authorization.AspNetCore; using GraphQL.Server.Ui.Altair; using GraphQL.Server.Ui.GraphiQL; using GraphQL.Server.Ui.Playground; @@ -30,8 +29,7 @@ public void ConfigureServices(IServiceCollection services) services .AddRouting() .AddSingleton() - .Configure(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment()) - .AddTransient(); // required by CustomErrorInfoProvider + .Configure(opt => opt.ExposeExceptionStackTrace = Environment.IsDevelopment()); services.AddGraphQL(builder => builder .AddApolloTracing() diff --git a/src/Transports.AspNetCore/Extensions/GraphQLBuilderUserContextExtensions.cs b/src/Transports.AspNetCore/Extensions/GraphQLBuilderExtensions.cs similarity index 89% rename from src/Transports.AspNetCore/Extensions/GraphQLBuilderUserContextExtensions.cs rename to src/Transports.AspNetCore/Extensions/GraphQLBuilderExtensions.cs index b563a789..6ea73c9a 100644 --- a/src/Transports.AspNetCore/Extensions/GraphQLBuilderUserContextExtensions.cs +++ b/src/Transports.AspNetCore/Extensions/GraphQLBuilderExtensions.cs @@ -6,7 +6,7 @@ namespace GraphQL.Server; -public static class GraphQLBuilderUserContextExtensions +public static class GraphQLBuilderExtensions { /// /// Adds an as a singleton. @@ -92,4 +92,15 @@ public static IGraphQLBuilder AddDefaultEndpointSelectorPolicy(this IGraphQLBuil return builder; } + + /// + /// Registers with the dependency injection framework + /// and configures it to be used when executing a request. + /// + public static IGraphQLBuilder AddAuthorization(this IGraphQLBuilder builder) + { + builder.AddValidationRule(true); + builder.Services.TryRegister(DI.ServiceLifetime.Singleton); + return builder; + } } diff --git a/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs b/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs index e540f906..2449ea13 100644 --- a/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs +++ b/src/Transports.AspNetCore/GraphQLHttpMiddleware.cs @@ -15,7 +15,7 @@ namespace GraphQL.Server.Transports.AspNetCore; /// Type of GraphQL schema that is used to validate and process requests. /// This may be a typed schema as well as . In both cases registered schemas will be pulled from /// the dependency injection framework. Note that when specifying the first schema registered via -/// AddSchema +/// AddSchema /// will be pulled (the "default" schema). /// public class GraphQLHttpMiddleware : GraphQLHttpMiddleware @@ -473,7 +473,7 @@ protected virtual async Task HandleBatchRequestAsync( /// . ///

/// To tailor the user context individually for each request, call - /// + /// /// to set or modify the user context, pulling the HTTP context from /// via /// if needed. diff --git a/tests/ApiApprovalTests/ApiApprovalTests.cs b/tests/ApiApprovalTests/ApiApprovalTests.cs index b85f02b4..5db8451b 100644 --- a/tests/ApiApprovalTests/ApiApprovalTests.cs +++ b/tests/ApiApprovalTests/ApiApprovalTests.cs @@ -13,7 +13,6 @@ public class ApiApprovalTests [InlineData(typeof(Server.Ui.GraphiQL.GraphiQLMiddleware))] [InlineData(typeof(Server.Ui.Playground.PlaygroundMiddleware))] [InlineData(typeof(Server.Ui.Voyager.VoyagerMiddleware))] - [InlineData(typeof(Server.Authorization.AspNetCore.AuthorizationValidationRule))] [InlineData(typeof(Server.Transports.AspNetCore.GraphQLHttpMiddleware<>))] [InlineData(typeof(Server.Transports.Subscriptions.Abstractions.SubscriptionServer))] [InlineData(typeof(Server.Transports.WebSockets.WebSocketTransport))] diff --git a/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt index 7699de2f..6dc17d17 100644 --- a/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -1,7 +1,8 @@ namespace GraphQL.Server { - public static class GraphQLBuilderUserContextExtensions + public static class GraphQLBuilderExtensions { + public static GraphQL.DI.IGraphQLBuilder AddAuthorization(this GraphQL.DI.IGraphQLBuilder builder) { } public static GraphQL.DI.IGraphQLBuilder AddDefaultEndpointSelectorPolicy(this GraphQL.DI.IGraphQLBuilder builder) { } public static GraphQL.DI.IGraphQLBuilder AddUserContextBuilder(this GraphQL.DI.IGraphQLBuilder builder) where TUserContextBuilder : class, GraphQL.Server.Transports.AspNetCore.IUserContextBuilder { } @@ -28,6 +29,46 @@ namespace GraphQL.Server.Transports.AspNetCore public System.Func? OnNotAuthorizedPolicy { get; } public System.Func? OnNotAuthorizedRole { get; } } + public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule + { + public AuthorizationValidationRule(Microsoft.AspNetCore.Http.IHttpContextAccessor httpContextAccessor) { } + public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } + public class AuthorizationVisitor : GraphQL.Validation.INodeVisitor + { + public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } + public Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } + public System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } + protected virtual bool Authorize() { } + protected virtual Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy) { } + protected virtual System.Threading.Tasks.Task AuthorizePolicyAsync(string policy) { } + protected virtual bool AuthorizeRole(string role) { } + public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } + protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } + protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } + protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } + protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info, System.Collections.Generic.List roles) { } + public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } + protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } + protected virtual bool Validate(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } + public virtual bool ValidateSchema(GraphQL.Validation.ValidationContext context) { } + public struct ValidationInfo : System.IEquatable + { + public ValidationInfo(GraphQL.Types.IProvideMetadata Obj, GraphQLParser.AST.ASTNode? Node, GraphQL.Types.IFieldType? ParentFieldType, GraphQL.Types.IGraphType? ParentGraphType, GraphQL.Validation.ValidationContext Context) { } + public GraphQL.Validation.ValidationContext Context { get; set; } + public GraphQLParser.AST.ASTNode? Node { get; set; } + public GraphQL.Types.IProvideMetadata Obj { get; set; } + public GraphQL.Types.IFieldType? ParentFieldType { get; set; } + public GraphQL.Types.IGraphType? ParentGraphType { get; set; } + public void Deconstruct(out GraphQL.Types.IProvideMetadata Obj, out GraphQLParser.AST.ASTNode? Node, out GraphQL.Types.IFieldType? ParentFieldType, out GraphQL.Types.IGraphType? ParentGraphType, out GraphQL.Validation.ValidationContext Context) { } + public bool Equals(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo other) { } + public override bool Equals(object obj) { } + public override int GetHashCode() { } + public override string ToString() { } + public static bool operator !=(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo right) { } + public static bool operator ==(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo right) { } + } + } + } public abstract class GraphQLHttpMiddleware { public GraphQLHttpMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, GraphQL.IGraphQLTextSerializer serializer, GraphQL.Server.Transports.AspNetCore.GraphQLHttpMiddlewareOptions options) { } From e173d7c9f7383da07b8bc55b51d4304ef99eec4e Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 21 May 2022 18:18:22 -0400 Subject: [PATCH 03/16] Refactor & code splitting --- .../AuthorizationValidationRule.cs | 531 +----------------- .../AuthorizationVisitor.cs | 41 ++ .../AuthorizationVisitorBase.Validation.cs | 222 ++++++++ .../AuthorizationVisitorBase.cs | 285 ++++++++++ 4 files changed, 557 insertions(+), 522 deletions(-) create mode 100644 src/Transports.AspNetCore/AuthorizationVisitor.cs create mode 100644 src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs create mode 100644 src/Transports.AspNetCore/AuthorizationVisitorBase.cs diff --git a/src/Transports.AspNetCore/AuthorizationValidationRule.cs b/src/Transports.AspNetCore/AuthorizationValidationRule.cs index 39d51ed8..1d946125 100644 --- a/src/Transports.AspNetCore/AuthorizationValidationRule.cs +++ b/src/Transports.AspNetCore/AuthorizationValidationRule.cs @@ -1,8 +1,4 @@ -using System.Security.Claims; -using GraphQL.Server.Transports.AspNetCore.Errors; -using GraphQL.Types; using GraphQL.Validation; -using GraphQLParser.AST; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -25,526 +21,17 @@ public AuthorizationValidationRule(IHttpContextAccessor httpContextAccessor) /// public ValueTask ValidateAsync(ValidationContext context) { - var httpContext = _contextAccessor.HttpContext ?? NoHttpContext(); - var user = httpContext.User ?? NoUser(); - var provider = context.RequestServices ?? NoRequestServices(); - var authService = provider.GetService() ?? NoAuthServiceError(); + var httpContext = _contextAccessor.HttpContext + ?? throw new InvalidOperationException("HttpContext could not be retrieved from IHttpContextAccessor."); + var user = httpContext.User + ?? throw new InvalidOperationException("ClaimsPrincipal could not be retrieved from HttpContext.User."); + var provider = context.RequestServices + ?? throw new MissingRequestServicesException(); + var authService = provider.GetService() + ?? throw new InvalidOperationException("An instance of IAuthorizationService could not be pulled from the dependency injection framework."); var visitor = new AuthorizationVisitor(context, user, authService); + // if the schema fails authentication, report the error and do not perform any additional authorization checks. return visitor.ValidateSchema(context) ? new(visitor) : default; } - - private static HttpContext NoHttpContext() - => throw new InvalidOperationException("HttpContext could not be retrieved from IHttpContextAccessor."); - - private static ClaimsPrincipal NoUser() - => throw new InvalidOperationException("ClaimsPrincipal could not be retrieved from HttpContext.User."); - - private static IServiceProvider NoRequestServices() - => throw new MissingRequestServicesException(); - - private static IAuthorizationService NoAuthServiceError() - => throw new InvalidOperationException("An instance of IAuthorizationService could not be pulled from the dependency injection framework."); - - /// - public class AuthorizationVisitor : INodeVisitor - { - /// - public AuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPrincipal, IAuthorizationService authorizationService) - { - if (context == null) - throw new ArgumentNullException(nameof(context)); - ClaimsPrincipal = claimsPrincipal ?? throw new ArgumentNullException(nameof(claimsPrincipal)); - if (claimsPrincipal.Identity == null) - throw new InvalidOperationException($"{nameof(claimsPrincipal)}.Identity cannot be null."); - AuthorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); - _fragmentDefinitionsToCheck = context.GetRecursivelyReferencedFragments(context.Operation); - _userIsAuthenticated = claimsPrincipal.Identity.IsAuthenticated; - } - - /// - /// The user that this authorization visitor will authenticate against. - /// - public ClaimsPrincipal ClaimsPrincipal { get; } - - /// - /// The authorization service that is used to authorize policy requests. - /// - public IAuthorizationService AuthorizationService { get; } - - private bool _checkTree; // used to skip processing fragments that do not apply - private ASTNode? _checkUntil; - private readonly List? _fragmentDefinitionsToCheck; // contains a list of fragments to process, or null if none - private Dictionary? _policyResults; // contains a dictionary of policies that have been checked - private Dictionary? _roleResults; // contains a dictionary of roles that have been checked - private readonly Stack _onlyAnonymousSelected = new(); - private readonly bool _userIsAuthenticated; - private Dictionary? _fragments; - private List? _todo; - - private class TypeInfo - { - public bool AnyAuthenticated; - public bool AnyAnonymous; - public List? WaitingOnFragments; - } - - private class TodoInfo - { - public ValidationInfo ValidationInfo { get; } - public bool AnyAuthenticated; - public bool AnyAnonymous; - public List WaitingOnFragments; - - public TodoInfo(ValidationInfo vi, TypeInfo ti) - { - ValidationInfo = vi; - AnyAuthenticated = ti.AnyAuthenticated; - AnyAnonymous = ti.AnyAnonymous; - WaitingOnFragments = ti.WaitingOnFragments ?? NoWaitingOnFragments(); - } - - private static List NoWaitingOnFragments() - => throw new InvalidOperationException("Waiting on fragments must not be null."); - } - - /// - public virtual void Enter(ASTNode node, ValidationContext context) - { - if (node == context.Operation || (node is GraphQLFragmentDefinition fragmentDefinition && _fragmentDefinitionsToCheck != null && _fragmentDefinitionsToCheck.Contains(fragmentDefinition))) - { - var type = context.TypeInfo.GetLastType()?.GetNamedType(); - if (type != null) - { - // if type is null that means that no type was configured for this operation in the schema; will produce a separate validation error - _onlyAnonymousSelected.Push(new()); - _checkTree = true; - } - } - else if (_checkTree) - { - if (node is GraphQLField fieldNode) - { - if (SkipField(fieldNode, context)) - { - _checkTree = false; - _checkUntil = node; - } - else - { - var field = context.TypeInfo.GetFieldDef(); - // might be null if no match was found in the schema - // and skip processing for __typeName - if (field != null && field != context.Schema.TypeNameMetaFieldType) - { - var fieldAnonymousAllowed = field.IsAnonymousAllowed() || field == context.Schema.TypeMetaFieldType || field == context.Schema.SchemaMetaFieldType; - var ti = _onlyAnonymousSelected.Peek(); - if (fieldAnonymousAllowed) - ti.AnyAnonymous = true; - else - ti.AnyAuthenticated = true; - - if (!fieldAnonymousAllowed) - { - Validate(field, node, context); - } - } - // prep for descendants, if any - _onlyAnonymousSelected.Push(new()); - } - } - else if (node is GraphQLFragmentSpread fragmentSpread) - { - var ti = _onlyAnonymousSelected.Peek(); - var fragmentName = fragmentSpread.FragmentName.Name.StringValue; - if (_fragments?.TryGetValue(fragmentName, out var fragmentInfo) == true) - { - ti.AnyAuthenticated |= fragmentInfo.AnyAuthenticated; - ti.AnyAnonymous |= fragmentInfo.AnyAnonymous; - if (fragmentInfo.WaitingOnFragments?.Count > 0) - { - ti.WaitingOnFragments ??= new(); - ti.WaitingOnFragments.AddRange(fragmentInfo.WaitingOnFragments); - } - } - else - { - ti.WaitingOnFragments ??= new(); - ti.WaitingOnFragments.Add(fragmentName); - } - } - else if (node is GraphQLArgument) - { - // ignore arguments of directives - if (context.TypeInfo.GetAncestor(2)?.Kind == ASTNodeKind.Field) - { - // verify field argument - var arg = context.TypeInfo.GetArgument(); - if (arg != null) - { - Validate(arg, node, context); - } - } - } - } - } - - /// - public virtual void Leave(ASTNode node, ValidationContext context) - { - if (!_checkTree) - { - if (_checkUntil == node) - { - _checkTree = true; - _checkUntil = null; - } - return; - } - if (node == context.Operation) - { - _checkTree = false; - PopAndProcess(); - } - else if (node is GraphQLFragmentDefinition fragmentDefinition) - { - _checkTree = false; - var fragmentName = fragmentDefinition.FragmentName.Name.StringValue; - var ti = _onlyAnonymousSelected.Pop(); - RecursiveResolve(fragmentName, ti, context); - _fragments ??= new(); - _fragments.TryAdd(fragmentName, ti); - } - else if (_checkTree && node is GraphQLField) - { - PopAndProcess(); - } - - void PopAndProcess() - { - var info = _onlyAnonymousSelected.Pop(); - var type = context.TypeInfo.GetLastType()?.GetNamedType(); - if (type == null) - return; - if (info.AnyAuthenticated || (!info.AnyAnonymous && (info.WaitingOnFragments?.Count ?? 0) == 0)) - { - Validate(type, node, context); - } - else if (info.WaitingOnFragments?.Count > 0) - { - _todo ??= new(); - _todo.Add(new(BuildValidationInfo(type, node, context), info)); - } - } - } - - /// - /// Indicates if the specified field should skip authentication processing. - /// Default implementation looks at @skip and @include directives only. - /// - protected virtual bool SkipField(GraphQLField node, ValidationContext context) - { - // check - var skipDirective = node.Directives?.FirstOrDefault(x => x.Name == "skip"); - if (skipDirective != null) - { - var value = GetDirectiveValue(skipDirective, context, false); - if (value) - return true; - } - - var includeDirective = node.Directives?.FirstOrDefault(x => x.Name == "include"); - if (includeDirective != null) - { - var value = GetDirectiveValue(includeDirective, context, true); - if (!value) - return true; - } - - return false; - - static bool GetDirectiveValue(GraphQLDirective directive, ValidationContext context, bool defaultValue) - { - var ifArg = directive.Arguments?.FirstOrDefault(x => x.Name == "if"); - if (ifArg != null) - { - if (ifArg.Value is GraphQLBooleanValue boolValue) - { - return boolValue.BoolValue; - } - else if (ifArg.Value is GraphQLVariable variable) - { - if (context.Operation.Variables != null) - { - var varDef = context.Operation.Variables.FirstOrDefault(x => x.Variable.Name == variable.Name); - if (varDef != null && varDef.Type.Name() == "Boolean") - { - if (context.Variables.TryGetValue(variable.Name.StringValue, out var value)) - { - if (value is bool boolValue2) - return boolValue2; - } - if (varDef.DefaultValue is GraphQLBooleanValue boolValue3) - { - return boolValue3.BoolValue; - } - } - } - } - } - return defaultValue; - } - } - - // runs when a fragment is added or updated; the fragment might not be waiting on any - // other fragments, or it still might be - private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContext context) - { - // first see if any other fragments are waiting on this fragment - if (_fragments != null) - { - foreach (var fragment in _fragments) - { - var ti2 = fragment.Value; - if (ti2.WaitingOnFragments != null && ti2.WaitingOnFragments.Remove(fragmentName)) - { - ti2.AnyAuthenticated |= ti.AnyAuthenticated; - ti2.AnyAnonymous |= ti.AnyAnonymous; - RecursiveResolve(fragment.Key, ti2, context); - } - } - } - // then, if this fragment is fully resolved, check to see if any nodes are waiting for this fragment - if ((ti.WaitingOnFragments?.Count ?? 0) == 0) - { - if (_todo != null) - { - var count = _todo.Count; - for (var i = 0; i < count; i++) - { - var todo = _todo[i]; - if (todo.WaitingOnFragments.Remove(fragmentName)) - { - todo.AnyAuthenticated |= ti.AnyAuthenticated; - todo.AnyAnonymous |= ti.AnyAnonymous; - if (todo.WaitingOnFragments.Count == 0) - { - _todo.RemoveAt(i); - count--; - if (todo.AnyAuthenticated || !todo.AnyAnonymous) - { - Validate(todo.ValidationInfo); - } - } - } - } - } - } - } - - /// - /// Validates authorization rules for the schema. - /// Returns a value indicating if validation was successful. - /// - public virtual bool ValidateSchema(ValidationContext context) - => Validate(context.Schema, null, context); - - /// - /// Validate a node that is current within the context. - /// - private bool Validate(IProvideMetadata obj, ASTNode? node, ValidationContext context) - => Validate(BuildValidationInfo(obj, node, context)); - - private static ValidationInfo BuildValidationInfo(IProvideMetadata obj, ASTNode? node, ValidationContext context) - { - IFieldType? parentFieldType = null; - IGraphType? parentGraphType = null; - if (node is GraphQLField) - { - if (obj is IGraphType) - { - parentFieldType = context.TypeInfo.GetFieldDef(0); - parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); - } - else if (obj is IFieldType) - { - parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); - } - } - else if (node is GraphQLArgument) - { - parentFieldType = context.TypeInfo.GetFieldDef(); - parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); - } - return new(obj, node, parentFieldType, parentGraphType, context); - } - - /// Provides contextual information to the schema, graph, field, or query argument being validated. - /// The schema, graph type, field type, or query argument being validated. May be an interface type if fragments are in use. - /// Null for a schema validation; otherwise the , , or being validated. - /// The validaion context; but may not be applicable for node being validated. - /// For graph types other than operations, the field where this type was referenced; for query arguments, the field to which this argument belongs. - /// For graph types, the graph type for the field where this type was referenced; for field types, the graph type to which this field belongs; for query arguments, the graph type for the field to which this argument belongs. - public record struct ValidationInfo(IProvideMetadata Obj, ASTNode? Node, IFieldType? ParentFieldType, IGraphType? ParentGraphType, ValidationContext Context); - - /// - /// Validates authorization rules for the specified schema, graph, field or query argument. - /// Does not consider . - /// Returns a value indicating if validation was successful for this node. - /// - protected virtual bool Validate(ValidationInfo info) - { - bool requiresAuthorization = info.Obj.IsAuthorizationRequired(); - if (!requiresAuthorization) - return true; - - var success = true; - var policies = info.Obj.GetPolicies(); - if (policies?.Count > 0) - { - requiresAuthorization = false; - _policyResults ??= new Dictionary(); - foreach (var policy in policies) - { - if (!_policyResults.TryGetValue(policy, out var result)) - { - result = AuthorizePolicy(policy); - _policyResults.Add(policy, result); - } - if (!result.Succeeded) - { - HandleNodeNotInPolicy(info, policy, result); - success = false; - } - } - } - - var roles = info.Obj.GetRoles(); - if (roles?.Count > 0) - { - requiresAuthorization = false; - _roleResults ??= new Dictionary(); - foreach (var role in roles) - { - if (!_roleResults.TryGetValue(role, out var result)) - { - result = AuthorizeRole(role); - _roleResults.Add(role, result); - } - if (result) - goto PassRoles; - } - HandleNodeNotInRoles(info, roles); - success = false; - } - PassRoles: - - if (requiresAuthorization) - { - if (!Authorize()) - { - HandleNodeNotAuthorized(info); - success = false; - } - } - - return success; - } - - /// - protected virtual bool Authorize() - => _userIsAuthenticated; - - /// - protected virtual bool AuthorizeRole(string role) - => ClaimsPrincipal.IsInRole(role); - - /// - protected virtual AuthorizationResult AuthorizePolicy(string policy) - => AuthorizePolicyAsync(policy).GetAwaiter().GetResult(); - - /// - /// Adds a error to the validation context indicating that the user is not authenticated - /// as required by this graph, field or query argument. - /// - /// Information about the node being validated. - protected virtual void HandleNodeNotAuthorized(ValidationInfo info) - { - var resource = GenerateResourceDescription(info); - var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); - info.Context.ReportError(err); - } - - /// - /// Adds a error to the validation context indicating that the user is not a member of any of - /// the roles required by this graph, field or query argument. - /// - /// Information about the node being validated. - /// The list of roles of which the user must be a member. - protected virtual void HandleNodeNotInRoles(ValidationInfo info, List roles) - { - var resource = GenerateResourceDescription(info); - var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); - err.RolesRequired = roles; - info.Context.ReportError(err); - } - - /// - /// Adds a error to the validation context indicating that the user is not a member of any of - /// the roles required by this graph, field or query argument. - /// - /// Information about the node being validated. - /// The policy which these nodes are being authenticated against. - /// The result of the authentication request. - protected virtual void HandleNodeNotInPolicy(ValidationInfo info, string policy, AuthorizationResult authorizationResult) - { - var resource = GenerateResourceDescription(info); - var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); - err.PolicyRequired = policy; - err.PolicyAuthorizationResult = authorizationResult; - info.Context.ReportError(err); - } - - /// - /// Generates a friendly name for a specified graph, field or query argument. - /// - protected virtual string GenerateResourceDescription(ValidationInfo info) - { - if (info.Obj is ISchema) - { - return "schema"; - } - else if (info.Obj is IGraphType graphType) - { - if (info.Node is GraphQLField) - { - return $"type '{graphType.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; - } - else if (info.Node is GraphQLOperationDefinition op) - { - return $"type '{graphType.Name}' for {op.Operation.ToString().ToLower(System.Globalization.CultureInfo.InvariantCulture)} operation{(!string.IsNullOrEmpty(op.Name?.StringValue) ? $" '{op.Name}'" : null)}"; - } - else - { - return $"type '{graphType.Name}'"; - } - } - else if (info.Obj is IFieldType fieldType) - { - return $"field '{fieldType.Name}' on type '{info.ParentGraphType?.Name}'"; - } - else if (info.Obj is QueryArgument queryArgument) - { - return $"argument '{queryArgument.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; - } - else - { - return info.Node?.GetType().Name ?? "unknown"; - } - } - - /// - protected virtual Task AuthorizePolicyAsync(string policy) - => AuthorizationService.AuthorizeAsync(ClaimsPrincipal, policy); - } } diff --git a/src/Transports.AspNetCore/AuthorizationVisitor.cs b/src/Transports.AspNetCore/AuthorizationVisitor.cs new file mode 100644 index 00000000..f1e67f1f --- /dev/null +++ b/src/Transports.AspNetCore/AuthorizationVisitor.cs @@ -0,0 +1,41 @@ +using System.Security.Claims; +using GraphQL.Validation; +using Microsoft.AspNetCore.Authorization; + +namespace GraphQL.Server.Transports.AspNetCore; + +/// +public class AuthorizationVisitor : AuthorizationVisitorBase +{ + /// + public AuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPrincipal, IAuthorizationService authorizationService) + : base(context) + { + if (context == null) + throw new ArgumentNullException(nameof(context)); + ClaimsPrincipal = claimsPrincipal ?? throw new ArgumentNullException(nameof(claimsPrincipal)); + AuthorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); + } + + /// + /// Gets the user that this authorization visitor will authenticate against. + /// + protected ClaimsPrincipal ClaimsPrincipal { get; } + + /// + /// Gets the authorization service that is used to authorize policy requests. + /// + protected IAuthorizationService AuthorizationService { get; } + + /// + protected override bool Authorize() + => ClaimsPrincipal.Identity?.IsAuthenticated ?? false; + + /// + protected override bool AuthorizeRole(string role) + => ClaimsPrincipal.IsInRole(role); + + /// + protected override AuthorizationResult AuthorizePolicy(string policy) + => AuthorizationService.AuthorizeAsync(ClaimsPrincipal, policy).GetAwaiter().GetResult(); +} diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs new file mode 100644 index 00000000..992ec0e5 --- /dev/null +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -0,0 +1,222 @@ +using System.Security.Claims; +using System.Security.Principal; +using GraphQL.Server.Transports.AspNetCore.Errors; +using GraphQL.Types; +using GraphQL.Validation; +using GraphQLParser.AST; +using Microsoft.AspNetCore.Authorization; + +namespace GraphQL.Server.Transports.AspNetCore; + +public partial class AuthorizationVisitorBase +{ + /// + /// Validates authorization rules for the schema. + /// Returns a value indicating if validation was successful. + /// + public virtual bool ValidateSchema(ValidationContext context) + => Validate(context.Schema, null, context); + + /// + /// Validate a node that is current within the context. + /// + private bool Validate(IProvideMetadata obj, ASTNode? node, ValidationContext context) + => Validate(BuildValidationInfo(obj, node, context)); + + private static ValidationInfo BuildValidationInfo(IProvideMetadata obj, ASTNode? node, ValidationContext context) + { + IFieldType? parentFieldType = null; + IGraphType? parentGraphType = null; + if (node is GraphQLField) + { + if (obj is IGraphType) + { + parentFieldType = context.TypeInfo.GetFieldDef(0); + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + else if (obj is IFieldType) + { + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + } + else if (node is GraphQLArgument) + { + parentFieldType = context.TypeInfo.GetFieldDef(); + parentGraphType = context.TypeInfo.GetLastType(1)?.GetNamedType(); + } + return new(obj, node, parentFieldType, parentGraphType, context); + } + + /// Provides contextual information to the schema, graph, field, or query argument being validated. + /// The schema, graph type, field type, or query argument being validated. May be an interface type if fragments are in use. + /// Null for a schema validation; otherwise the , , or being validated. + /// The validaion context; but may not be applicable for node being validated. + /// For graph types other than operations, the field where this type was referenced; for query arguments, the field to which this argument belongs. + /// For graph types, the graph type for the field where this type was referenced; for field types, the graph type to which this field belongs; for query arguments, the graph type for the field to which this argument belongs. + public readonly record struct ValidationInfo( + IProvideMetadata Obj, + ASTNode? Node, + IFieldType? ParentFieldType, + IGraphType? ParentGraphType, + ValidationContext Context); + + // contains cached authorization results + private Dictionary? _roleResults; // contains a dictionary of roles that have been checked + private Dictionary? _policyResults; // contains a dictionary of policies that have been checked + private bool? _userIsAuthorized; + + /// + /// Validates authorization rules for the specified schema, graph, field or query argument. + /// Does not consider + /// as this is handled elsewhere. + /// Returns a value indicating if validation was successful for this node. + /// + protected virtual bool Validate(ValidationInfo info) + { + bool requiresAuthorization = info.Obj.IsAuthorizationRequired(); + if (!requiresAuthorization) + return true; + + var success = true; + var policies = info.Obj.GetPolicies(); + if (policies?.Count > 0) + { + requiresAuthorization = false; + _policyResults ??= new Dictionary(); + foreach (var policy in policies) + { + if (!_policyResults.TryGetValue(policy, out var result)) + { + result = AuthorizePolicy(policy); + _policyResults.Add(policy, result); + } + if (!result.Succeeded) + { + HandleNodeNotInPolicy(info, policy, result); + success = false; + } + } + } + + var roles = info.Obj.GetRoles(); + if (roles?.Count > 0) + { + requiresAuthorization = false; + _roleResults ??= new Dictionary(); + foreach (var role in roles) + { + if (!_roleResults.TryGetValue(role, out var result)) + { + result = AuthorizeRole(role); + _roleResults.Add(role, result); + } + if (result) + goto PassRoles; + } + HandleNodeNotInRoles(info, roles); + success = false; + } + PassRoles: + + if (requiresAuthorization) + { + var authorized = _userIsAuthorized ??= Authorize(); + if (!authorized) + { + HandleNodeNotAuthorized(info); + success = false; + } + } + + return success; + } + + /// + protected abstract bool Authorize(); + + /// + protected abstract bool AuthorizeRole(string role); + + /// + protected abstract AuthorizationResult AuthorizePolicy(string policy); + + /// + /// Adds a error to the validation context indicating that the user is not authenticated + /// as required by this graph, field or query argument. + /// + /// Information about the node being validated. + protected virtual void HandleNodeNotAuthorized(ValidationInfo info) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + info.Context.ReportError(err); + } + + /// + /// Adds a error to the validation context indicating that the user is not a member of any of + /// the roles required by this graph, field or query argument. + /// + /// Information about the node being validated. + /// The list of roles of which the user must be a member. + protected virtual void HandleNodeNotInRoles(ValidationInfo info, List roles) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + err.RolesRequired = roles; + info.Context.ReportError(err); + } + + /// + /// Adds a error to the validation context indicating that the user is not a member of any of + /// the roles required by this graph, field or query argument. + /// + /// Information about the node being validated. + /// The policy which these nodes are being authenticated against. + /// The result of the authentication request. + protected virtual void HandleNodeNotInPolicy(ValidationInfo info, string policy, AuthorizationResult authorizationResult) + { + var resource = GenerateResourceDescription(info); + var err = info.Node == null ? new AccessDeniedError(resource) : new AccessDeniedError(resource, info.Context.Document.Source, info.Node); + err.PolicyRequired = policy; + err.PolicyAuthorizationResult = authorizationResult; + info.Context.ReportError(err); + } + + /// + /// Generates a friendly name for a specified graph, field or query argument. + /// + protected virtual string GenerateResourceDescription(ValidationInfo info) + { + if (info.Obj is ISchema) + { + return "schema"; + } + else if (info.Obj is IGraphType graphType) + { + if (info.Node is GraphQLField) + { + return $"type '{graphType.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else if (info.Node is GraphQLOperationDefinition op) + { + return $"type '{graphType.Name}' for {op.Operation.ToString().ToLower(System.Globalization.CultureInfo.InvariantCulture)} operation{(!string.IsNullOrEmpty(op.Name?.StringValue) ? $" '{op.Name}'" : null)}"; + } + else + { + return $"type '{graphType.Name}'"; + } + } + else if (info.Obj is IFieldType fieldType) + { + return $"field '{fieldType.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else if (info.Obj is QueryArgument queryArgument) + { + return $"argument '{queryArgument.Name}' for field '{info.ParentFieldType?.Name}' on type '{info.ParentGraphType?.Name}'"; + } + else + { + return info.Node?.GetType().Name ?? "unknown"; + } + } +} diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs new file mode 100644 index 00000000..ed97de76 --- /dev/null +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -0,0 +1,285 @@ +using GraphQL.Types; +using GraphQL.Validation; +using GraphQLParser.AST; +using Microsoft.AspNetCore.Authorization; + +namespace GraphQL.Server.Transports.AspNetCore; + +/// +public abstract partial class AuthorizationVisitorBase : INodeVisitor +{ + /// + public AuthorizationVisitorBase(ValidationContext context) + { + if (context == null) + throw new ArgumentNullException(nameof(context)); + _fragmentDefinitionsToCheck = context.GetRecursivelyReferencedFragments(context.Operation); + } + + private bool _checkTree; // used to skip processing fragments or operations that do not apply + private ASTNode? _checkUntil; + private readonly List? _fragmentDefinitionsToCheck; // contains a list of fragments to process, or null if none + private readonly Stack _onlyAnonymousSelected = new(); + private Dictionary? _fragments; + private List? _todo; + + /// + public virtual void Enter(ASTNode node, ValidationContext context) + { + if (node == context.Operation || (node is GraphQLFragmentDefinition fragmentDefinition && _fragmentDefinitionsToCheck != null && _fragmentDefinitionsToCheck.Contains(fragmentDefinition))) + { + var type = context.TypeInfo.GetLastType()?.GetNamedType(); + if (type != null) + { + // if type is null that means that no type was configured for this operation in the schema; will produce a separate validation error + _onlyAnonymousSelected.Push(new()); + _checkTree = true; + } + } + else if (_checkTree) + { + if (node is GraphQLField fieldNode) + { + if (SkipField(fieldNode, context)) + { + _checkTree = false; + _checkUntil = node; + } + else + { + var field = context.TypeInfo.GetFieldDef(); + // might be null if no match was found in the schema + // and skip processing for __typeName + if (field != null && field != context.Schema.TypeNameMetaFieldType) + { + var fieldAnonymousAllowed = field.IsAnonymousAllowed() || field == context.Schema.TypeMetaFieldType || field == context.Schema.SchemaMetaFieldType; + var ti = _onlyAnonymousSelected.Peek(); + if (fieldAnonymousAllowed) + ti.AnyAnonymous = true; + else + ti.AnyAuthenticated = true; + + if (!fieldAnonymousAllowed) + { + Validate(field, node, context); + } + } + // prep for descendants, if any + _onlyAnonymousSelected.Push(new()); + } + } + else if (node is GraphQLFragmentSpread fragmentSpread) + { + var ti = _onlyAnonymousSelected.Peek(); + var fragmentName = fragmentSpread.FragmentName.Name.StringValue; + if (_fragments?.TryGetValue(fragmentName, out var fragmentInfo) == true) + { + ti.AnyAuthenticated |= fragmentInfo.AnyAuthenticated; + ti.AnyAnonymous |= fragmentInfo.AnyAnonymous; + if (fragmentInfo.WaitingOnFragments?.Count > 0) + { + ti.WaitingOnFragments ??= new(); + ti.WaitingOnFragments.AddRange(fragmentInfo.WaitingOnFragments); + } + } + else + { + ti.WaitingOnFragments ??= new(); + ti.WaitingOnFragments.Add(fragmentName); + } + } + else if (node is GraphQLArgument) + { + // ignore arguments of directives + if (context.TypeInfo.GetAncestor(2)?.Kind == ASTNodeKind.Field) + { + // verify field argument + var arg = context.TypeInfo.GetArgument(); + if (arg != null) + { + Validate(arg, node, context); + } + } + } + } + } + + /// + public virtual void Leave(ASTNode node, ValidationContext context) + { + if (!_checkTree) + { + if (_checkUntil == node) + { + _checkTree = true; + _checkUntil = null; + } + return; + } + if (node == context.Operation) + { + _checkTree = false; + PopAndProcess(); + } + else if (node is GraphQLFragmentDefinition fragmentDefinition) + { + _checkTree = false; + var fragmentName = fragmentDefinition.FragmentName.Name.StringValue; + var ti = _onlyAnonymousSelected.Pop(); + RecursiveResolve(fragmentName, ti, context); + _fragments ??= new(); + _fragments.TryAdd(fragmentName, ti); + } + else if (_checkTree && node is GraphQLField) + { + PopAndProcess(); + } + + void PopAndProcess() + { + var info = _onlyAnonymousSelected.Pop(); + var type = context.TypeInfo.GetLastType()?.GetNamedType(); + if (type == null) + return; + if (info.AnyAuthenticated || (!info.AnyAnonymous && (info.WaitingOnFragments?.Count ?? 0) == 0)) + { + Validate(type, node, context); + } + else if (info.WaitingOnFragments?.Count > 0) + { + _todo ??= new(); + _todo.Add(new(BuildValidationInfo(type, node, context), info)); + } + } + } + + /// + /// Indicates if the specified field should skip authentication processing. + /// Default implementation looks at @skip and @include directives only. + /// + protected virtual bool SkipField(GraphQLField node, ValidationContext context) + { + // check + var skipDirective = node.Directives?.FirstOrDefault(x => x.Name == "skip"); + if (skipDirective != null) + { + var value = GetDirectiveValue(skipDirective, context, false); + if (value) + return true; + } + + var includeDirective = node.Directives?.FirstOrDefault(x => x.Name == "include"); + if (includeDirective != null) + { + var value = GetDirectiveValue(includeDirective, context, true); + if (!value) + return true; + } + + return false; + + static bool GetDirectiveValue(GraphQLDirective directive, ValidationContext context, bool defaultValue) + { + var ifArg = directive.Arguments?.FirstOrDefault(x => x.Name == "if"); + if (ifArg != null) + { + if (ifArg.Value is GraphQLBooleanValue boolValue) + { + return boolValue.BoolValue; + } + else if (ifArg.Value is GraphQLVariable variable) + { + if (context.Operation.Variables != null) + { + var varDef = context.Operation.Variables.FirstOrDefault(x => x.Variable.Name == variable.Name); + if (varDef != null && varDef.Type.Name() == "Boolean") + { + if (context.Variables.TryGetValue(variable.Name.StringValue, out var value)) + { + if (value is bool boolValue2) + return boolValue2; + } + if (varDef.DefaultValue is GraphQLBooleanValue boolValue3) + { + return boolValue3.BoolValue; + } + } + } + } + } + return defaultValue; + } + } + + // runs when a fragment is added or updated; the fragment might not be waiting on any + // other fragments, or it still might be + private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContext context) + { + // first see if any other fragments are waiting on this fragment + if (_fragments != null) + { + foreach (var fragment in _fragments) + { + var ti2 = fragment.Value; + if (ti2.WaitingOnFragments != null && ti2.WaitingOnFragments.Remove(fragmentName)) + { + ti2.AnyAuthenticated |= ti.AnyAuthenticated; + ti2.AnyAnonymous |= ti.AnyAnonymous; + RecursiveResolve(fragment.Key, ti2, context); + } + } + } + // then, if this fragment is fully resolved, check to see if any nodes are waiting for this fragment + if ((ti.WaitingOnFragments?.Count ?? 0) == 0) + { + if (_todo != null) + { + var count = _todo.Count; + for (var i = 0; i < count; i++) + { + var todo = _todo[i]; + if (todo.WaitingOnFragments.Remove(fragmentName)) + { + todo.AnyAuthenticated |= ti.AnyAuthenticated; + todo.AnyAnonymous |= ti.AnyAnonymous; + if (todo.WaitingOnFragments.Count == 0) + { + _todo.RemoveAt(i); + count--; + if (todo.AnyAuthenticated || !todo.AnyAnonymous) + { + Validate(todo.ValidationInfo); + } + } + } + } + } + } + } + + private class TypeInfo + { + public bool AnyAuthenticated; + public bool AnyAnonymous; + public List? WaitingOnFragments; + } + + private class TodoInfo + { + public ValidationInfo ValidationInfo { get; } + public bool AnyAuthenticated; + public bool AnyAnonymous; + public List WaitingOnFragments; + + public TodoInfo(ValidationInfo vi, TypeInfo ti) + { + ValidationInfo = vi; + AnyAuthenticated = ti.AnyAuthenticated; + AnyAnonymous = ti.AnyAnonymous; + WaitingOnFragments = ti.WaitingOnFragments ?? NoWaitingOnFragments(); + } + + private static List NoWaitingOnFragments() + => throw new InvalidOperationException("Waiting on fragments must not be null."); + } +} From 1a4b0504c529f33847071920bcc78cc979f1199f Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 21 May 2022 18:26:20 -0400 Subject: [PATCH 04/16] Update --- src/Transports.AspNetCore/AuthorizationVisitorBase.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index ed97de76..fceaf247 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -1,7 +1,6 @@ using GraphQL.Types; using GraphQL.Validation; using GraphQLParser.AST; -using Microsoft.AspNetCore.Authorization; namespace GraphQL.Server.Transports.AspNetCore; From faf9ff998ee81ea77c5bcf279b711c9bd832dddc Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 21 May 2022 19:16:26 -0400 Subject: [PATCH 05/16] Add comments --- .../AuthorizationVisitorBase.cs | 58 ++++++++++++++----- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index fceaf247..312b1b4f 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -25,6 +25,8 @@ public AuthorizationVisitorBase(ValidationContext context) /// public virtual void Enter(ASTNode node, ValidationContext context) { + // if the node is the selected operation, or if it is a fragment referenced by the current operation, + // then enable authorization checks on decendant nodes (_checkTree = true) if (node == context.Operation || (node is GraphQLFragmentDefinition fragmentDefinition && _fragmentDefinitionsToCheck != null && _fragmentDefinitionsToCheck.Contains(fragmentDefinition))) { var type = context.TypeInfo.GetLastType()?.GetNamedType(); @@ -39,6 +41,7 @@ public virtual void Enter(ASTNode node, ValidationContext context) { if (node is GraphQLField fieldNode) { + // if a directive indicates to skip this field, skip authorization checks until Leave() is called for this node if (SkipField(fieldNode, context)) { _checkTree = false; @@ -47,10 +50,18 @@ public virtual void Enter(ASTNode node, ValidationContext context) else { var field = context.TypeInfo.GetFieldDef(); - // might be null if no match was found in the schema - // and skip processing for __typeName + // Note: 'field' might be null here if no match was found in the schema (which causes a different validation error). + // Also, skip processing for __typeName entirely; do not consider it an anonymous field or + // a field that would require authentication for the type -- in this manner, a selection for + // only __typename will require authentication, but a selection for __typename and an anonymous + // field will not. if (field != null && field != context.Schema.TypeNameMetaFieldType) { + // If the field is marked as AllowAnonymous, record that we have encountered a field for this type + // which is anonymous; if it is not, record that we have encountered a field for this type + // which will require the type to be authenticated. + // Also, __type and __schema are implicitly marked as AllowAnonymous; the schema can be marked + // with authorization requirements if introspection queries are to be disallowed. var fieldAnonymousAllowed = field.IsAnonymousAllowed() || field == context.Schema.TypeMetaFieldType || field == context.Schema.SchemaMetaFieldType; var ti = _onlyAnonymousSelected.Peek(); if (fieldAnonymousAllowed) @@ -58,34 +69,47 @@ public virtual void Enter(ASTNode node, ValidationContext context) else ti.AnyAuthenticated = true; + // Fields, unlike types, are validated immediately. if (!fieldAnonymousAllowed) { Validate(field, node, context); } } + // prep for descendants, if any + // note: this causes a heap allocation for each node; struct is possible but changes + // to the struct (which occur for each field) will require a pop and push; there are a + // number of other ideas that could be implemented here to reduce allocations _onlyAnonymousSelected.Push(new()); } } else if (node is GraphQLFragmentSpread fragmentSpread) { var ti = _onlyAnonymousSelected.Peek(); - var fragmentName = fragmentSpread.FragmentName.Name.StringValue; - if (_fragments?.TryGetValue(fragmentName, out var fragmentInfo) == true) + // if the type already requires authentication, it doesn't matter if the fragment fields + // are marked as anonymous or not. (note that fragment fields will still get authenticated) + if (!ti.AnyAuthenticated) { - ti.AnyAuthenticated |= fragmentInfo.AnyAuthenticated; - ti.AnyAnonymous |= fragmentInfo.AnyAnonymous; - if (fragmentInfo.WaitingOnFragments?.Count > 0) + // check processed fragments to see if the specified fragment has already been processed; + // if so, copy the fragment information in here; otherwise mark this type as being dependent + // on fragment fields + var fragmentName = fragmentSpread.FragmentName.Name.StringValue; + if (_fragments?.TryGetValue(fragmentName, out var fragmentInfo) == true) + { + ti.AnyAuthenticated |= fragmentInfo.AnyAuthenticated; + ti.AnyAnonymous |= fragmentInfo.AnyAnonymous; + if (fragmentInfo.WaitingOnFragments?.Count > 0) + { + ti.WaitingOnFragments ??= new(); + ti.WaitingOnFragments.AddRange(fragmentInfo.WaitingOnFragments); + } + } + else { ti.WaitingOnFragments ??= new(); - ti.WaitingOnFragments.AddRange(fragmentInfo.WaitingOnFragments); + ti.WaitingOnFragments.Add(fragmentName); } } - else - { - ti.WaitingOnFragments ??= new(); - ti.WaitingOnFragments.Add(fragmentName); - } } else if (node is GraphQLArgument) { @@ -108,11 +132,14 @@ public virtual void Leave(ASTNode node, ValidationContext context) { if (!_checkTree) { + // if we are within a field skipped by a directive, resume auth checks at the appropriate time if (_checkUntil == node) { _checkTree = true; _checkUntil = null; } + // in any case if this tree is not being checked (not the selected operation or not a fragment spread in use), + // then return (no auth checks) return; } if (node == context.Operation) @@ -122,6 +149,8 @@ public virtual void Leave(ASTNode node, ValidationContext context) } else if (node is GraphQLFragmentDefinition fragmentDefinition) { + // once a fragment is done being processed, apply it to all types waiting on fragment checks, + // and process checks for types that are not waiting on any fragments _checkTree = false; var fragmentName = fragmentDefinition.FragmentName.Name.StringValue; var ti = _onlyAnonymousSelected.Pop(); @@ -134,6 +163,8 @@ public virtual void Leave(ASTNode node, ValidationContext context) PopAndProcess(); } + // pop the current type info, and validate the type if it does not contain only fields marked + // with AllowAnonymous (assuming it is not waiting on fragments) void PopAndProcess() { var info = _onlyAnonymousSelected.Pop(); @@ -158,7 +189,6 @@ void PopAndProcess() /// protected virtual bool SkipField(GraphQLField node, ValidationContext context) { - // check var skipDirective = node.Directives?.FirstOrDefault(x => x.Name == "skip"); if (skipDirective != null) { From 04578d8f588a73f2b3c13752ef8110784177666a Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 21 May 2022 23:56:42 -0400 Subject: [PATCH 06/16] Add a whole lot of comments --- src/Transports.AspNetCore/AuthorizationVisitorBase.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index 312b1b4f..8e93b6c7 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -77,9 +77,6 @@ public virtual void Enter(ASTNode node, ValidationContext context) } // prep for descendants, if any - // note: this causes a heap allocation for each node; struct is possible but changes - // to the struct (which occur for each field) will require a pop and push; there are a - // number of other ideas that could be implemented here to reduce allocations _onlyAnonymousSelected.Push(new()); } } @@ -286,6 +283,8 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex } } + // note: having TypeInfo a class causes a heap allocation for each node; struct is possible + // but requires a lot of changes to the code; todo in separate PR private class TypeInfo { public bool AnyAuthenticated; @@ -293,6 +292,8 @@ private class TypeInfo public List? WaitingOnFragments; } + // an allocation for TodoInfo only occurs when a field references a fragment that has not + // yet been encountered private class TodoInfo { public ValidationInfo ValidationInfo { get; } From bdacff15054f9cd490be6b40d332e39c26f2d4f7 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sun, 22 May 2022 01:53:54 -0400 Subject: [PATCH 07/16] Update --- .../AuthorizationVisitorBase.cs | 33 +++++---- .../Compatibility/IsExternalInit.cs | 9 +++ ....Server.Transports.AspNetCore.approved.txt | 72 ++++++++++--------- 3 files changed, 64 insertions(+), 50 deletions(-) create mode 100644 src/Transports.AspNetCore/Compatibility/IsExternalInit.cs diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index 8e93b6c7..eba0d677 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -16,11 +16,11 @@ public AuthorizationVisitorBase(ValidationContext context) } private bool _checkTree; // used to skip processing fragments or operations that do not apply - private ASTNode? _checkUntil; + private ASTNode? _checkUntil; // used to resume processing after a skipped field (skipped by a directive) private readonly List? _fragmentDefinitionsToCheck; // contains a list of fragments to process, or null if none private readonly Stack _onlyAnonymousSelected = new(); private Dictionary? _fragments; - private List? _todo; + private List? _todos; /// public virtual void Enter(ASTNode node, ValidationContext context) @@ -63,11 +63,12 @@ public virtual void Enter(ASTNode node, ValidationContext context) // Also, __type and __schema are implicitly marked as AllowAnonymous; the schema can be marked // with authorization requirements if introspection queries are to be disallowed. var fieldAnonymousAllowed = field.IsAnonymousAllowed() || field == context.Schema.TypeMetaFieldType || field == context.Schema.SchemaMetaFieldType; - var ti = _onlyAnonymousSelected.Peek(); + var ti = _onlyAnonymousSelected.Pop(); if (fieldAnonymousAllowed) ti.AnyAnonymous = true; else ti.AnyAuthenticated = true; + _onlyAnonymousSelected.Push(ti); // Fields, unlike types, are validated immediately. if (!fieldAnonymousAllowed) @@ -82,7 +83,7 @@ public virtual void Enter(ASTNode node, ValidationContext context) } else if (node is GraphQLFragmentSpread fragmentSpread) { - var ti = _onlyAnonymousSelected.Peek(); + var ti = _onlyAnonymousSelected.Pop(); // if the type already requires authentication, it doesn't matter if the fragment fields // are marked as anonymous or not. (note that fragment fields will still get authenticated) if (!ti.AnyAuthenticated) @@ -107,6 +108,7 @@ public virtual void Enter(ASTNode node, ValidationContext context) ti.WaitingOnFragments.Add(fragmentName); } } + _onlyAnonymousSelected.Push(ti); } else if (node is GraphQLArgument) { @@ -174,8 +176,8 @@ void PopAndProcess() } else if (info.WaitingOnFragments?.Count > 0) { - _todo ??= new(); - _todo.Add(new(BuildValidationInfo(type, node, context), info)); + _todos ??= new(); + _todos.Add(new(BuildValidationInfo(type, node, context), info)); } } } @@ -251,6 +253,7 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex { ti2.AnyAuthenticated |= ti.AnyAuthenticated; ti2.AnyAnonymous |= ti.AnyAnonymous; + _fragments[fragment.Key] = ti2; RecursiveResolve(fragment.Key, ti2, context); } } @@ -258,19 +261,19 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex // then, if this fragment is fully resolved, check to see if any nodes are waiting for this fragment if ((ti.WaitingOnFragments?.Count ?? 0) == 0) { - if (_todo != null) + if (_todos != null) { - var count = _todo.Count; + var count = _todos.Count; for (var i = 0; i < count; i++) { - var todo = _todo[i]; + var todo = _todos[i]; if (todo.WaitingOnFragments.Remove(fragmentName)) { todo.AnyAuthenticated |= ti.AnyAuthenticated; todo.AnyAnonymous |= ti.AnyAnonymous; if (todo.WaitingOnFragments.Count == 0) { - _todo.RemoveAt(i); + _todos.RemoveAt(i); count--; if (todo.AnyAuthenticated || !todo.AnyAnonymous) { @@ -283,23 +286,19 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex } } - // note: having TypeInfo a class causes a heap allocation for each node; struct is possible - // but requires a lot of changes to the code; todo in separate PR - private class TypeInfo + private struct TypeInfo { public bool AnyAuthenticated; public bool AnyAnonymous; public List? WaitingOnFragments; } - // an allocation for TodoInfo only occurs when a field references a fragment that has not - // yet been encountered private class TodoInfo { - public ValidationInfo ValidationInfo { get; } + public readonly ValidationInfo ValidationInfo; public bool AnyAuthenticated; public bool AnyAnonymous; - public List WaitingOnFragments; + public readonly List WaitingOnFragments; public TodoInfo(ValidationInfo vi, TypeInfo ti) { diff --git a/src/Transports.AspNetCore/Compatibility/IsExternalInit.cs b/src/Transports.AspNetCore/Compatibility/IsExternalInit.cs new file mode 100644 index 00000000..f16546a5 --- /dev/null +++ b/src/Transports.AspNetCore/Compatibility/IsExternalInit.cs @@ -0,0 +1,9 @@ +#if !NET5_0_OR_GREATER + +namespace System.Runtime.CompilerServices; + +internal static class IsExternalInit +{ +} + +#endif diff --git a/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt index 6dc17d17..c77bce5b 100644 --- a/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -33,40 +33,46 @@ namespace GraphQL.Server.Transports.AspNetCore { public AuthorizationValidationRule(Microsoft.AspNetCore.Http.IHttpContextAccessor httpContextAccessor) { } public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } - public class AuthorizationVisitor : GraphQL.Validation.INodeVisitor + } + public class AuthorizationVisitor : GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase + { + public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } + protected Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } + protected System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } + protected override bool Authorize() { } + protected override Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy) { } + protected override bool AuthorizeRole(string role) { } + } + public abstract class AuthorizationVisitorBase : GraphQL.Validation.INodeVisitor + { + public AuthorizationVisitorBase(GraphQL.Validation.ValidationContext context) { } + protected abstract bool Authorize(); + protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy); + protected abstract bool AuthorizeRole(string role); + public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } + protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } + protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } + protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } + protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, System.Collections.Generic.List roles) { } + public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } + protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } + protected virtual bool Validate(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } + public virtual bool ValidateSchema(GraphQL.Validation.ValidationContext context) { } + public readonly struct ValidationInfo : System.IEquatable { - public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } - public Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } - public System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } - protected virtual bool Authorize() { } - protected virtual Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy) { } - protected virtual System.Threading.Tasks.Task AuthorizePolicyAsync(string policy) { } - protected virtual bool AuthorizeRole(string role) { } - public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } - protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } - protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } - protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } - protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info, System.Collections.Generic.List roles) { } - public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } - protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } - protected virtual bool Validate(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo info) { } - public virtual bool ValidateSchema(GraphQL.Validation.ValidationContext context) { } - public struct ValidationInfo : System.IEquatable - { - public ValidationInfo(GraphQL.Types.IProvideMetadata Obj, GraphQLParser.AST.ASTNode? Node, GraphQL.Types.IFieldType? ParentFieldType, GraphQL.Types.IGraphType? ParentGraphType, GraphQL.Validation.ValidationContext Context) { } - public GraphQL.Validation.ValidationContext Context { get; set; } - public GraphQLParser.AST.ASTNode? Node { get; set; } - public GraphQL.Types.IProvideMetadata Obj { get; set; } - public GraphQL.Types.IFieldType? ParentFieldType { get; set; } - public GraphQL.Types.IGraphType? ParentGraphType { get; set; } - public void Deconstruct(out GraphQL.Types.IProvideMetadata Obj, out GraphQLParser.AST.ASTNode? Node, out GraphQL.Types.IFieldType? ParentFieldType, out GraphQL.Types.IGraphType? ParentGraphType, out GraphQL.Validation.ValidationContext Context) { } - public bool Equals(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo other) { } - public override bool Equals(object obj) { } - public override int GetHashCode() { } - public override string ToString() { } - public static bool operator !=(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo right) { } - public static bool operator ==(GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationValidationRule.AuthorizationVisitor.ValidationInfo right) { } - } + public ValidationInfo(GraphQL.Types.IProvideMetadata Obj, GraphQLParser.AST.ASTNode? Node, GraphQL.Types.IFieldType? ParentFieldType, GraphQL.Types.IGraphType? ParentGraphType, GraphQL.Validation.ValidationContext Context) { } + public GraphQL.Validation.ValidationContext Context { get; set; } + public GraphQLParser.AST.ASTNode? Node { get; set; } + public GraphQL.Types.IProvideMetadata Obj { get; set; } + public GraphQL.Types.IFieldType? ParentFieldType { get; set; } + public GraphQL.Types.IGraphType? ParentGraphType { get; set; } + public void Deconstruct(out GraphQL.Types.IProvideMetadata Obj, out GraphQLParser.AST.ASTNode? Node, out GraphQL.Types.IFieldType? ParentFieldType, out GraphQL.Types.IGraphType? ParentGraphType, out GraphQL.Validation.ValidationContext Context) { } + public bool Equals(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo other) { } + public override bool Equals(object obj) { } + public override int GetHashCode() { } + public override string ToString() { } + public static bool operator !=(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo right) { } + public static bool operator ==(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo left, GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo right) { } } } public abstract class GraphQLHttpMiddleware From c79b3270865e08defe017ea4989e2bdda0e05474 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sun, 22 May 2022 02:13:57 -0400 Subject: [PATCH 08/16] Update --- src/Transports.AspNetCore/AuthorizationVisitorBase.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index eba0d677..4de93474 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -246,6 +246,7 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex // first see if any other fragments are waiting on this fragment if (_fragments != null) { + Retry: foreach (var fragment in _fragments) { var ti2 = fragment.Value; @@ -255,6 +256,7 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex ti2.AnyAnonymous |= ti.AnyAnonymous; _fragments[fragment.Key] = ti2; RecursiveResolve(fragment.Key, ti2, context); + goto Retry; // modifying a collection at runtime is not supported } } } From a32fb91c110e9ac59b615a34956b7382b363259b Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sun, 22 May 2022 02:23:49 -0400 Subject: [PATCH 09/16] Update --- ...rver.Authorization.AspNetCore.approved.txt | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 tests/ApiApprovalTests/GraphQL.Server.Authorization.AspNetCore.approved.txt diff --git a/tests/ApiApprovalTests/GraphQL.Server.Authorization.AspNetCore.approved.txt b/tests/ApiApprovalTests/GraphQL.Server.Authorization.AspNetCore.approved.txt deleted file mode 100644 index 0ffea07b..00000000 --- a/tests/ApiApprovalTests/GraphQL.Server.Authorization.AspNetCore.approved.txt +++ /dev/null @@ -1,45 +0,0 @@ -namespace GraphQL.Server.Authorization.AspNetCore -{ - public class AuthorizationError : GraphQL.Validation.ValidationError - { - public AuthorizationError(GraphQLParser.AST.ASTNode? node, GraphQL.Validation.ValidationContext context, string message, Microsoft.AspNetCore.Authorization.AuthorizationResult result, GraphQLParser.AST.OperationType? operationType = default) { } - public virtual Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizationResult { get; } - public GraphQLParser.AST.OperationType? OperationType { get; } - } - public class AuthorizationValidationRule : GraphQL.Validation.IValidationRule - { - public AuthorizationValidationRule(Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService, GraphQL.Server.Authorization.AspNetCore.IClaimsPrincipalAccessor claimsPrincipalAccessor, GraphQL.Server.Authorization.AspNetCore.IAuthorizationErrorMessageBuilder messageBuilder) { } - protected virtual void AddValidationError(GraphQLParser.AST.ASTNode? node, GraphQL.Validation.ValidationContext context, GraphQLParser.AST.OperationType? operationType, Microsoft.AspNetCore.Authorization.AuthorizationResult result) { } - public System.Threading.Tasks.ValueTask ValidateAsync(GraphQL.Validation.ValidationContext context) { } - } - public class DefaultAuthorizationErrorMessageBuilder : GraphQL.Server.Authorization.AspNetCore.IAuthorizationErrorMessageBuilder - { - public DefaultAuthorizationErrorMessageBuilder() { } - public virtual void AppendFailureHeader(System.Text.StringBuilder error, GraphQLParser.AST.OperationType? operationType) { } - public virtual void AppendFailureLine(System.Text.StringBuilder error, Microsoft.AspNetCore.Authorization.IAuthorizationRequirement authorizationRequirement) { } - public virtual string GenerateMessage(GraphQLParser.AST.OperationType? operationType, Microsoft.AspNetCore.Authorization.AuthorizationResult result) { } - } - public class DefaultClaimsPrincipalAccessor : GraphQL.Server.Authorization.AspNetCore.IClaimsPrincipalAccessor - { - public DefaultClaimsPrincipalAccessor(Microsoft.AspNetCore.Http.IHttpContextAccessor contextAccessor) { } - public System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context) { } - } - public interface IAuthorizationErrorMessageBuilder - { - void AppendFailureHeader(System.Text.StringBuilder error, GraphQLParser.AST.OperationType? operationType); - void AppendFailureLine(System.Text.StringBuilder error, Microsoft.AspNetCore.Authorization.IAuthorizationRequirement authorizationRequirement); - string GenerateMessage(GraphQLParser.AST.OperationType? operationType, Microsoft.AspNetCore.Authorization.AuthorizationResult result); - } - public interface IClaimsPrincipalAccessor - { - System.Security.Claims.ClaimsPrincipal GetClaimsPrincipal(GraphQL.Validation.ValidationContext context); - } -} -namespace GraphQL.Server -{ - public static class GraphQLBuilderAuthorizationExtensions - { - public static GraphQL.DI.IGraphQLBuilder AddGraphQLAuthorization(this GraphQL.DI.IGraphQLBuilder builder) { } - public static GraphQL.DI.IGraphQLBuilder AddGraphQLAuthorization(this GraphQL.DI.IGraphQLBuilder builder, System.Action? configure) { } - } -} \ No newline at end of file From b7e083d72a74e0ce0d41f21d7f7ca3b7f5268003 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Sat, 18 Jun 2022 21:23:01 -0400 Subject: [PATCH 10/16] Updates --- src/All/All.csproj | 1 - src/Transports.AspNetCore/AuthorizationValidationRule.cs | 5 ----- src/Transports.AspNetCore/AuthorizationVisitor.cs | 4 ---- .../AuthorizationVisitorBase.Validation.cs | 8 -------- src/Transports.AspNetCore/AuthorizationVisitorBase.cs | 4 ---- 5 files changed, 22 deletions(-) diff --git a/src/All/All.csproj b/src/All/All.csproj index 9678122d..cd96cb27 100644 --- a/src/All/All.csproj +++ b/src/All/All.csproj @@ -9,7 +9,6 @@ - diff --git a/src/Transports.AspNetCore/AuthorizationValidationRule.cs b/src/Transports.AspNetCore/AuthorizationValidationRule.cs index 1d946125..c6080612 100644 --- a/src/Transports.AspNetCore/AuthorizationValidationRule.cs +++ b/src/Transports.AspNetCore/AuthorizationValidationRule.cs @@ -1,8 +1,3 @@ -using GraphQL.Validation; -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; - namespace GraphQL.Server.Transports.AspNetCore; /// diff --git a/src/Transports.AspNetCore/AuthorizationVisitor.cs b/src/Transports.AspNetCore/AuthorizationVisitor.cs index f1e67f1f..c78b0f0f 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitor.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitor.cs @@ -1,7 +1,3 @@ -using System.Security.Claims; -using GraphQL.Validation; -using Microsoft.AspNetCore.Authorization; - namespace GraphQL.Server.Transports.AspNetCore; /// diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs index 992ec0e5..13d41b0b 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -1,11 +1,3 @@ -using System.Security.Claims; -using System.Security.Principal; -using GraphQL.Server.Transports.AspNetCore.Errors; -using GraphQL.Types; -using GraphQL.Validation; -using GraphQLParser.AST; -using Microsoft.AspNetCore.Authorization; - namespace GraphQL.Server.Transports.AspNetCore; public partial class AuthorizationVisitorBase diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index 4de93474..40c4c617 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -1,7 +1,3 @@ -using GraphQL.Types; -using GraphQL.Validation; -using GraphQLParser.AST; - namespace GraphQL.Server.Transports.AspNetCore; /// From 6d3bb17a1fe570ef0c22ff2f85228c1647e125c4 Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Wed, 22 Jun 2022 08:54:58 -0400 Subject: [PATCH 11/16] Update src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs Co-authored-by: Ivan Maximov --- .../AuthorizationVisitorBase.Validation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs index 13d41b0b..84765a1f 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -42,7 +42,7 @@ private static ValidationInfo BuildValidationInfo(IProvideMetadata obj, ASTNode? /// Provides contextual information to the schema, graph, field, or query argument being validated. /// The schema, graph type, field type, or query argument being validated. May be an interface type if fragments are in use. /// Null for a schema validation; otherwise the , , or being validated. - /// The validaion context; but may not be applicable for node being validated. + /// The validation context; but may not be applicable for node being validated. /// For graph types other than operations, the field where this type was referenced; for query arguments, the field to which this argument belongs. /// For graph types, the graph type for the field where this type was referenced; for field types, the graph type to which this field belongs; for query arguments, the graph type for the field to which this argument belongs. public readonly record struct ValidationInfo( From b77a68ae1bbd52872ef7647a277bcc05cdad857d Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Wed, 22 Jun 2022 08:55:07 -0400 Subject: [PATCH 12/16] Update src/Transports.AspNetCore/AuthorizationValidationRule.cs Co-authored-by: Ivan Maximov --- src/Transports.AspNetCore/AuthorizationValidationRule.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Transports.AspNetCore/AuthorizationValidationRule.cs b/src/Transports.AspNetCore/AuthorizationValidationRule.cs index c6080612..11d57667 100644 --- a/src/Transports.AspNetCore/AuthorizationValidationRule.cs +++ b/src/Transports.AspNetCore/AuthorizationValidationRule.cs @@ -19,7 +19,7 @@ public AuthorizationValidationRule(IHttpContextAccessor httpContextAccessor) var httpContext = _contextAccessor.HttpContext ?? throw new InvalidOperationException("HttpContext could not be retrieved from IHttpContextAccessor."); var user = httpContext.User - ?? throw new InvalidOperationException("ClaimsPrincipal could not be retrieved from HttpContext.User."); + ?? throw new InvalidOperationException("ClaimsPrincipal could not be retrieved from HttpContext."); var provider = context.RequestServices ?? throw new MissingRequestServicesException(); var authService = provider.GetService() From b7a16ca1bc44e9ae7e834c8aa238eba636b93a11 Mon Sep 17 00:00:00 2001 From: Shane32 Date: Wed, 22 Jun 2022 09:43:50 -0400 Subject: [PATCH 13/16] Update xml comments --- .../AuthorizationVisitorBase.Validation.cs | 14 ++++-- .../AuthorizationVisitorBase.cs | 46 +++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs index 84765a1f..57667bf3 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -13,9 +13,15 @@ public virtual bool ValidateSchema(ValidationContext context) /// Validate a node that is current within the context. /// private bool Validate(IProvideMetadata obj, ASTNode? node, ValidationContext context) - => Validate(BuildValidationInfo(obj, node, context)); + => Validate(BuildValidationInfo(node, obj, context)); - private static ValidationInfo BuildValidationInfo(IProvideMetadata obj, ASTNode? node, ValidationContext context) + /// + /// Initializes a new instance for the specified node. + /// + /// The specified . + /// The , or which has been matched to the node specified in . + /// The validation context. + private static ValidationInfo BuildValidationInfo(ASTNode? node, IProvideMetadata obj, ValidationContext context) { IFieldType? parentFieldType = null; IGraphType? parentGraphType = null; @@ -159,8 +165,8 @@ protected virtual void HandleNodeNotInRoles(ValidationInfo info, List ro } /// - /// Adds a error to the validation context indicating that the user is not a member of any of - /// the roles required by this graph, field or query argument. + /// Adds a error to the validation context indicating that the user does not meet the + /// authorization policy required by this graph, field or query argument. /// /// Information about the node being validated. /// The policy which these nodes are being authenticated against. diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs index 40c4c617..74d1c953 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.cs @@ -173,7 +173,7 @@ void PopAndProcess() else if (info.WaitingOnFragments?.Count > 0) { _todos ??= new(); - _todos.Add(new(BuildValidationInfo(type, node, context), info)); + _todos.Add(new(BuildValidationInfo(node, type, context), info)); } } } @@ -235,8 +235,10 @@ static bool GetDirectiveValue(GraphQLDirective directive, ValidationContext cont } } - // runs when a fragment is added or updated; the fragment might not be waiting on any - // other fragments, or it still might be + /// + /// Runs when a fragment is added or updated; the fragment might not be waiting on any + /// other fragments, or it still might be. + /// private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContext context) { // first see if any other fragments are waiting on this fragment @@ -284,18 +286,56 @@ private void RecursiveResolve(string fragmentName, TypeInfo ti, ValidationContex } } + /// + /// Stores information about the current graph type being examined + /// to know if all selected fields have been marked with + /// AllowAnonymous, + /// in which case authentication checks are skipped for the current graph type. + /// private struct TypeInfo { + /// + /// Indicates if any fields have been selected for the graph type which require authentication. + /// This includes any fields which are not marked with + /// AllowAnonymous. + /// Does not include introspection fields. + /// public bool AnyAuthenticated; + + /// + /// Indicates if any fields have been selected for the graph type which are marked with + /// AllowAnonymous. + /// Does not include introspection fields. + /// public bool AnyAnonymous; + + /// + /// A list of fragments referenced in the selection set which have not yet been encountered while + /// walking the document nodes. + /// public List? WaitingOnFragments; } + /// + /// Stores information about a graph type containing fragment(s) which have not yet + /// been encountered while walking the document nodes. + ///

+ /// Once the fragments have all been encountered, authentication checks occur if necessary for the + /// graph type -- specifically, if any authenticated fields were selected, or if no anonymous fields + /// were selected. + /// private class TodoInfo { + /// public readonly ValidationInfo ValidationInfo; + + /// public bool AnyAuthenticated; + + /// public bool AnyAnonymous; + + /// public readonly List WaitingOnFragments; public TodoInfo(ValidationInfo vi, TypeInfo ti) From 6f84b7e9ecf10acf7c61e67b294392839c65e8a5 Mon Sep 17 00:00:00 2001 From: Shane32 Date: Wed, 22 Jun 2022 16:13:25 -0400 Subject: [PATCH 14/16] Rename Authorize methods --- src/Transports.AspNetCore/AuthorizationVisitor.cs | 6 +++--- .../AuthorizationVisitorBase.Validation.cs | 12 ++++++------ ...GraphQL.Server.Transports.AspNetCore.approved.txt | 12 ++++++------ ...GraphQL.Server.Transports.AspNetCore.approved.txt | 12 ++++++------ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitor.cs b/src/Transports.AspNetCore/AuthorizationVisitor.cs index c78b0f0f..f10a4d06 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitor.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitor.cs @@ -24,14 +24,14 @@ public AuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPri protected IAuthorizationService AuthorizationService { get; } /// - protected override bool Authorize() + protected override bool IsAuthenticated() => ClaimsPrincipal.Identity?.IsAuthenticated ?? false; /// - protected override bool AuthorizeRole(string role) + protected override bool IsInRole(string role) => ClaimsPrincipal.IsInRole(role); /// - protected override AuthorizationResult AuthorizePolicy(string policy) + protected override AuthorizationResult Authorize(string policy) => AuthorizationService.AuthorizeAsync(ClaimsPrincipal, policy).GetAwaiter().GetResult(); } diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs index 57667bf3..b9322cd5 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -85,7 +85,7 @@ protected virtual bool Validate(ValidationInfo info) { if (!_policyResults.TryGetValue(policy, out var result)) { - result = AuthorizePolicy(policy); + result = Authorize(policy); _policyResults.Add(policy, result); } if (!result.Succeeded) @@ -105,7 +105,7 @@ protected virtual bool Validate(ValidationInfo info) { if (!_roleResults.TryGetValue(role, out var result)) { - result = AuthorizeRole(role); + result = IsInRole(role); _roleResults.Add(role, result); } if (result) @@ -118,7 +118,7 @@ protected virtual bool Validate(ValidationInfo info) if (requiresAuthorization) { - var authorized = _userIsAuthorized ??= Authorize(); + var authorized = _userIsAuthorized ??= IsAuthenticated(); if (!authorized) { HandleNodeNotAuthorized(info); @@ -130,13 +130,13 @@ protected virtual bool Validate(ValidationInfo info) } /// - protected abstract bool Authorize(); + protected abstract bool IsAuthenticated(); /// - protected abstract bool AuthorizeRole(string role); + protected abstract bool IsInRole(string role); /// - protected abstract AuthorizationResult AuthorizePolicy(string policy); + protected abstract AuthorizationResult Authorize(string policy); /// /// Adds a error to the validation context indicating that the user is not authenticated diff --git a/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt index 656d5cd7..68b2df62 100644 --- a/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -46,21 +46,21 @@ namespace GraphQL.Server.Transports.AspNetCore public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } protected Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } protected System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } - protected override bool Authorize() { } - protected override Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy) { } - protected override bool AuthorizeRole(string role) { } + protected override Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy) { } + protected override bool IsAuthenticated() { } + protected override bool IsInRole(string role) { } } public abstract class AuthorizationVisitorBase : GraphQL.Validation.INodeVisitor { public AuthorizationVisitorBase(GraphQL.Validation.ValidationContext context) { } - protected abstract bool Authorize(); - protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy); - protected abstract bool AuthorizeRole(string role); + protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy); public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, System.Collections.Generic.List roles) { } + protected abstract bool IsAuthenticated(); + protected abstract bool IsInRole(string role); public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } protected virtual bool Validate(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } diff --git a/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt index 55bbeb4a..2222c75a 100644 --- a/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -50,21 +50,21 @@ namespace GraphQL.Server.Transports.AspNetCore public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } protected Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } protected System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } - protected override bool Authorize() { } - protected override Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy) { } - protected override bool AuthorizeRole(string role) { } + protected override Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy) { } + protected override bool IsAuthenticated() { } + protected override bool IsInRole(string role) { } } public abstract class AuthorizationVisitorBase : GraphQL.Validation.INodeVisitor { public AuthorizationVisitorBase(GraphQL.Validation.ValidationContext context) { } - protected abstract bool Authorize(); - protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult AuthorizePolicy(string policy); - protected abstract bool AuthorizeRole(string role); + protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy); public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, System.Collections.Generic.List roles) { } + protected abstract bool IsAuthenticated(); + protected abstract bool IsInRole(string role); public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } protected virtual bool Validate(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } From 22b295abdb3497f387fcc108bbacde4425823656 Mon Sep 17 00:00:00 2001 From: Shane32 Date: Wed, 22 Jun 2022 16:14:15 -0400 Subject: [PATCH 15/16] Change method to get prop --- src/Transports.AspNetCore/AuthorizationVisitor.cs | 2 +- .../AuthorizationVisitorBase.Validation.cs | 4 ++-- .../GraphQL.Server.Transports.AspNetCore.approved.txt | 4 ++-- .../GraphQL.Server.Transports.AspNetCore.approved.txt | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Transports.AspNetCore/AuthorizationVisitor.cs b/src/Transports.AspNetCore/AuthorizationVisitor.cs index f10a4d06..33c047c3 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitor.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitor.cs @@ -24,7 +24,7 @@ public AuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPri protected IAuthorizationService AuthorizationService { get; } /// - protected override bool IsAuthenticated() + protected override bool IsAuthenticated => ClaimsPrincipal.Identity?.IsAuthenticated ?? false; /// diff --git a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs index b9322cd5..8c890ba5 100644 --- a/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs +++ b/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs @@ -118,7 +118,7 @@ protected virtual bool Validate(ValidationInfo info) if (requiresAuthorization) { - var authorized = _userIsAuthorized ??= IsAuthenticated(); + var authorized = _userIsAuthorized ??= IsAuthenticated; if (!authorized) { HandleNodeNotAuthorized(info); @@ -130,7 +130,7 @@ protected virtual bool Validate(ValidationInfo info) } /// - protected abstract bool IsAuthenticated(); + protected abstract bool IsAuthenticated { get; } /// protected abstract bool IsInRole(string role); diff --git a/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt index 68b2df62..3ff358d2 100644 --- a/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/net5+netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -46,20 +46,20 @@ namespace GraphQL.Server.Transports.AspNetCore public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } protected Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } protected System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } + protected override bool IsAuthenticated { get; } protected override Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy) { } - protected override bool IsAuthenticated() { } protected override bool IsInRole(string role) { } } public abstract class AuthorizationVisitorBase : GraphQL.Validation.INodeVisitor { public AuthorizationVisitorBase(GraphQL.Validation.ValidationContext context) { } + protected abstract bool IsAuthenticated { get; } protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy); public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, System.Collections.Generic.List roles) { } - protected abstract bool IsAuthenticated(); protected abstract bool IsInRole(string role); public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } diff --git a/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt b/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt index 2222c75a..3238b21f 100644 --- a/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt +++ b/tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt @@ -50,20 +50,20 @@ namespace GraphQL.Server.Transports.AspNetCore public AuthorizationVisitor(GraphQL.Validation.ValidationContext context, System.Security.Claims.ClaimsPrincipal claimsPrincipal, Microsoft.AspNetCore.Authorization.IAuthorizationService authorizationService) { } protected Microsoft.AspNetCore.Authorization.IAuthorizationService AuthorizationService { get; } protected System.Security.Claims.ClaimsPrincipal ClaimsPrincipal { get; } + protected override bool IsAuthenticated { get; } protected override Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy) { } - protected override bool IsAuthenticated() { } protected override bool IsInRole(string role) { } } public abstract class AuthorizationVisitorBase : GraphQL.Validation.INodeVisitor { public AuthorizationVisitorBase(GraphQL.Validation.ValidationContext context) { } + protected abstract bool IsAuthenticated { get; } protected abstract Microsoft.AspNetCore.Authorization.AuthorizationResult Authorize(string policy); public virtual void Enter(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotAuthorized(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { } protected virtual void HandleNodeNotInPolicy(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, string policy, Microsoft.AspNetCore.Authorization.AuthorizationResult authorizationResult) { } protected virtual void HandleNodeNotInRoles(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info, System.Collections.Generic.List roles) { } - protected abstract bool IsAuthenticated(); protected abstract bool IsInRole(string role); public virtual void Leave(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { } protected virtual bool SkipField(GraphQLParser.AST.GraphQLField node, GraphQL.Validation.ValidationContext context) { } From 46e36dd9b97e96aca7eca0b81fd47a6cce850fdc Mon Sep 17 00:00:00 2001 From: Shane Krueger Date: Wed, 22 Jun 2022 16:16:09 -0400 Subject: [PATCH 16/16] Update tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs Co-authored-by: Ivan Maximov --- tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs b/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs index 55e8000b..1db187b7 100644 --- a/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs +++ b/tests/Authorization.AspNetCore.Tests/ValidationTestBase.cs @@ -94,7 +94,7 @@ private IValidationResult Validate(ValidationTestConfig config) Operation = document.Definitions.OfType().First(), Rules = config.Rules, Variables = config.Variables, - RequestServices = ServiceProvider, + RequestServices = ServiceProvider, // root provider here instead of scoped one, but since nothing registered is scoped, it makes a little difference so we just use root provider directly }).GetAwaiter().GetResult().validationResult; }