Remove GetRecursivelyReferencedUsedFragments and SkipNode

docs/migration/migration8.md

@@ -34,6 +34,11 @@
   in the response prefer the same status code.  For practical purposes, this means that the included
   errors triggered by the authorization validation rule will now return 401 or 403 when appropriate.
 - The `SelectResponseContentType` method now returns a `MediaTypeHeaderValue` instead of a string.
+- The `AuthorizationVisitorBase.GetRecursivelyReferencedUsedFragments` method has been removed as
+  `ValidationContext` now provides an overload to `GetRecursivelyReferencedFragments` which will only
+  return fragments in use by the specified operation.
+- The `AuthorizationVisitorBase.SkipNode` method has been removed as `ValidationContext` now provides
+  a `ShouldIncludeNode` method.
 
 ## Other changes
 

src/Transports.AspNetCore/AuthorizationVisitorBase.cs

@@ -8,7 +8,7 @@ public AuthorizationVisitorBase(ValidationContext context)
     {
         if (context == null)
             throw new ArgumentNullException(nameof(context));
-        _fragmentDefinitionsToCheck = GetRecursivelyReferencedUsedFragments(context);
+        _fragmentDefinitionsToCheck = context.GetRecursivelyReferencedFragments(context.Operation, true);
     }
 
     private bool _checkTree; // used to skip processing fragments or operations that do not apply
@@ -36,7 +36,7 @@ public virtual async ValueTask EnterAsync(ASTNode node, ValidationContext contex
         else if (_checkTree)
         {
             // if a directive indicates to skip this node, skip authorization checks until Leave() is called for this node
-            if (SkipNode(node, context))
+            if (!context.ShouldIncludeNode(node))
             {
                 _checkTree = false;
                 _checkUntil = node;
@@ -175,72 +175,6 @@ async ValueTask PopAndProcessAsync()
         }
     }
 
-    /// <summary>
-    /// Indicates if the specified node should skip authentication processing.
-    /// Default implementation looks at @skip and @include directives only.
-    /// </summary>
-    protected virtual bool SkipNode(ASTNode node, ValidationContext context)
-    {
-        // according to GraphQL spec, directives with the same name may be defined so long as they cannot be
-        // placed on the same node types as other directives with the same name; so here we verify that the
-        // node is a field, fragment spread, or inline fragment, the only nodes allowed by the built-in @skip
-        // and @include directives
-        if (node is not GraphQLField && node is not GraphQLFragmentSpread && node is not GraphQLInlineFragment)
-            return false;
-
-        var directivesNode = (IHasDirectivesNode)node;
-
-        var skipDirective = directivesNode.Directives?.FirstOrDefault(x => x.Name == "skip");
-        if (skipDirective != null)
-        {
-            var value = GetDirectiveValue(skipDirective, context, false);
-            if (value)
-                return true;
-        }
-
-        var includeDirective = directivesNode.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;
-        }
-    }
-
     /// <summary>
     /// Runs when a fragment is added or updated; the fragment might not be waiting on any
     /// other fragments, or it still might be.

tests/ApiApprovalTests/net50+net60+net80/GraphQL.Server.Transports.AspNetCore.approved.txt

@@ -36,13 +36,11 @@ namespace GraphQL.Server.Transports.AspNetCore
         protected abstract System.Threading.Tasks.ValueTask<Microsoft.AspNetCore.Authorization.AuthorizationResult> AuthorizeAsync(string policy);
         public virtual System.Threading.Tasks.ValueTask EnterAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
-        protected System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedUsedFragments(GraphQL.Validation.ValidationContext validationContext) { }
         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<string> roles) { }
         protected abstract bool IsInRole(string role);
         public virtual System.Threading.Tasks.ValueTask LeaveAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
-        protected virtual bool SkipNode(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual System.Threading.Tasks.ValueTask<bool> ValidateAsync(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
         public virtual System.Threading.Tasks.ValueTask<bool> ValidateSchemaAsync(GraphQL.Validation.ValidationContext context) { }
         public readonly struct ValidationInfo : System.IEquatable<GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo>

tests/ApiApprovalTests/netcoreapp21+netstandard20/GraphQL.Server.Transports.AspNetCore.approved.txt

@@ -43,13 +43,11 @@ namespace GraphQL.Server.Transports.AspNetCore
         protected abstract System.Threading.Tasks.ValueTask<Microsoft.AspNetCore.Authorization.AuthorizationResult> AuthorizeAsync(string policy);
         public virtual System.Threading.Tasks.ValueTask EnterAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
-        protected System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedUsedFragments(GraphQL.Validation.ValidationContext validationContext) { }
         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<string> roles) { }
         protected abstract bool IsInRole(string role);
         public virtual System.Threading.Tasks.ValueTask LeaveAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
-        protected virtual bool SkipNode(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual System.Threading.Tasks.ValueTask<bool> ValidateAsync(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
         public virtual System.Threading.Tasks.ValueTask<bool> ValidateSchemaAsync(GraphQL.Validation.ValidationContext context) { }
         public readonly struct ValidationInfo : System.IEquatable<GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo>

tests/ApiApprovalTests/netcoreapp31/GraphQL.Server.Transports.AspNetCore.approved.txt

@@ -36,13 +36,11 @@ namespace GraphQL.Server.Transports.AspNetCore
         protected abstract System.Threading.Tasks.ValueTask<Microsoft.AspNetCore.Authorization.AuthorizationResult> AuthorizeAsync(string policy);
         public virtual System.Threading.Tasks.ValueTask EnterAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual string GenerateResourceDescription(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
-        protected System.Collections.Generic.List<GraphQLParser.AST.GraphQLFragmentDefinition>? GetRecursivelyReferencedUsedFragments(GraphQL.Validation.ValidationContext validationContext) { }
         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<string> roles) { }
         protected abstract bool IsInRole(string role);
         public virtual System.Threading.Tasks.ValueTask LeaveAsync(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
-        protected virtual bool SkipNode(GraphQLParser.AST.ASTNode node, GraphQL.Validation.ValidationContext context) { }
         protected virtual System.Threading.Tasks.ValueTask<bool> ValidateAsync(GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo info) { }
         public virtual System.Threading.Tasks.ValueTask<bool> ValidateSchemaAsync(GraphQL.Validation.ValidationContext context) { }
         public readonly struct ValidationInfo : System.IEquatable<GraphQL.Server.Transports.AspNetCore.AuthorizationVisitorBase.ValidationInfo>

tests/Transports.AspNetCore.Tests/AuthorizationTests.cs

@@ -545,8 +545,9 @@ private void Apply(IMetadataWriter obj, Mode mode)
     public void Constructors()
     {
         Should.Throw<ArgumentNullException>(() => new AuthorizationVisitor(null!, _principal, Mock.Of<IAuthorizationService>()));
-        Should.Throw<ArgumentNullException>(() => new AuthorizationVisitor(new ValidationContext(), null!, Mock.Of<IAuthorizationService>()));
-        Should.Throw<ArgumentNullException>(() => new AuthorizationVisitor(new ValidationContext(), _principal, null!));
+        var context = new ValidationContext() { Operation = new(new([])), Document = new([]) };
+        Should.Throw<ArgumentNullException>(() => new AuthorizationVisitor(context, null!, Mock.Of<IAuthorizationService>()));
+        Should.Throw<ArgumentNullException>(() => new AuthorizationVisitor(context, _principal, null!));
     }
 
     [Theory]