Fixed authorization type interceptor flow. (#8096)

Author: michaelstaib

src/HotChocolate/Core/src/Authorization/AuthorizationTypeInterceptor.cs

@@ -53,6 +53,9 @@ internal override void OnBeforeCreateSchemaInternal(
         schemaBuilder.SetSchema(d => _schemaContextData = d.Extend().Definition.ContextData);
     }
 
+    private ITypeCompletionContext _tc = default!;
+    private AuthorizeDirectiveType _t = default!;
+
     public override void OnBeforeCompleteName(
         ITypeCompletionContext completionContext,
         DefinitionBase definition)
@@ -69,14 +72,33 @@ public override void OnBeforeCompleteName(
             case UnionType when definition is UnionTypeDefinition unionTypeDef:
                 _unionTypes.Add(new UnionTypeInfo(completionContext, unionTypeDef));
                 break;
+
+            case AuthorizeDirectiveType type:
+                _t = type;
+                _tc = completionContext;
+                break;
         }
 
         // note, we do not need to collect interfaces as the object type has a
         // list implements that links to the interfaces that expose an object type.
     }
 
-    public override void OnBeforeCompleteTypes()
+    public override void OnAfterResolveRootType(
+        ITypeCompletionContext completionContext,
+        ObjectTypeDefinition definition,
+        OperationType operationType)
+    {
+        if (operationType is OperationType.Query)
+        {
+            _queryContext = completionContext;
+        }
+    }
+
+    public override void OnBeforeCompleteMetadata()
     {
+        _t.CompleteMetadata(_tc);
+        ((RegisteredType)_tc).Status = TypeStatus.MetadataCompleted;
+
         // at this stage in the type initialization we will create some state that we
         // will use to transform the schema authorization.
         var state = _state = CreateState();
@@ -99,18 +121,7 @@ public override void OnBeforeCompleteTypes()
         FindFieldsAndApplyAuthMiddleware(state);
     }
 
-    public override void OnAfterResolveRootType(
-        ITypeCompletionContext completionContext,
-        ObjectTypeDefinition definition,
-        OperationType operationType)
-    {
-        if (operationType is OperationType.Query)
-        {
-            _queryContext = completionContext;
-        }
-    }
-
-    public override void OnBeforeCompleteType(
+    public override void OnBeforeCompleteMetadata(
         ITypeCompletionContext completionContext,
         DefinitionBase definition)
     {
@@ -130,101 +141,107 @@ public override void OnAfterMakeExecutable()
         {
             var objectType = (ObjectType)type.TypeReg.Type;
 
-            if (objectType.ContextData.TryGetValue(NodeResolver, out var o) &&
-                o is NodeResolverInfo nodeResolverInfo)
+            if (!objectType.ContextData.TryGetValue(NodeResolver, out var o)
+                || o is not NodeResolverInfo nodeResolverInfo)
             {
-                var pipeline = nodeResolverInfo.Pipeline;
-                var directives = (DirectiveCollection)objectType.Directives;
-                var length = directives.Count;
-                ref var start = ref directives.GetReference();
+                continue;
+            }
 
-                for (var i = length - 1; i >= 0; i--)
-                {
-                    var directive = Unsafe.Add(ref start, i);
+            var pipeline = nodeResolverInfo.Pipeline;
+            var directives = (DirectiveCollection)objectType.Directives;
+            var length = directives.Count;
+            ref var start = ref directives.GetReference();
 
-                    if (directive.Type.Name.EqualsOrdinal(Authorize))
-                    {
-                        var authDir = directive.AsValue<AuthorizeDirective>();
-                        pipeline = CreateAuthMiddleware(authDir).Middleware.Invoke(pipeline);
-                    }
-                }
+            for (var i = length - 1; i >= 0; i--)
+            {
+                var directive = Unsafe.Add(ref start, i);
 
-                type.TypeDef.ContextData[NodeResolver] =
-                    new NodeResolverInfo(nodeResolverInfo.QueryField, pipeline);
+                if (directive.Type.Name.EqualsOrdinal(Authorize))
+                {
+                    var authDir = directive.AsValue<AuthorizeDirective>();
+                    pipeline = CreateAuthMiddleware(authDir).Middleware.Invoke(pipeline);
+                }
             }
+
+            type.TypeDef.ContextData[NodeResolver] =
+                new NodeResolverInfo(nodeResolverInfo.QueryField, pipeline);
         }
     }
 
     private void InspectObjectTypesForAuthDirective(State state)
     {
         foreach (var type in _objectTypes)
         {
-            if (IsAuthorizedType(type.TypeDef))
+            if (!IsAuthorizedType(type.TypeDef))
             {
-                var registration = type.TypeReg;
-                var mainTypeRef = registration.TypeReference;
+                continue;
+            }
+
+            var registration = type.TypeReg;
+            var mainTypeRef = registration.TypeReference;
 
-                // if this type is a root type we will copy type level auth down to the field.
-                if (registration.IsQueryType == true ||
-                    registration.IsMutationType == true ||
-                    registration.IsSubscriptionType == true)
+            // if this type is a root type we will copy type level auth down to the field.
+            if (registration.IsQueryType == true ||
+                registration.IsMutationType == true ||
+                registration.IsSubscriptionType == true)
+            {
+                foreach (var fieldDef in type.TypeDef.Fields)
                 {
-                    foreach (var fieldDef in type.TypeDef.Fields)
+                    // we are not interested in introspection fields or the node fields.
+                    if (fieldDef.IsIntrospectionField || fieldDef.IsNodeField())
                     {
-                        // we are not interested in introspection fields or the node fields.
-                        if (fieldDef.IsIntrospectionField || fieldDef.IsNodeField())
-                        {
-                            continue;
-                        }
-
-                        // if the field contains the AnonymousAllowed flag we will not
-                        // apply authorization on it.
-                        if(fieldDef.GetContextData().ContainsKey(AllowAnonymous))
-                        {
-                            continue;
-                        }
+                        continue;
+                    }
 
-                        ApplyAuthMiddleware(fieldDef, registration, false);
+                    // if the field contains the AnonymousAllowed flag we will not
+                    // apply authorization on it.
+                    if(fieldDef.GetContextData().ContainsKey(AllowAnonymous))
+                    {
+                        continue;
                     }
-                }
 
-                foreach (var reference in registration.References)
-                {
-                    state.AuthTypes.Add(reference);
-                    state.NeedsAuth.Add(reference);
+                    ApplyAuthMiddleware(fieldDef, registration, false);
                 }
+            }
+
+            foreach (var reference in registration.References)
+            {
+                state.AuthTypes.Add(reference);
+                state.NeedsAuth.Add(reference);
+            }
+
+            if (!type.TypeDef.HasInterfaces)
+            {
+                continue;
+            }
 
-                if (type.TypeDef.HasInterfaces)
+            CollectInterfaces(
+                type.TypeDef.GetInterfaces(),
+                interfaceTypeRef =>
                 {
-                    CollectInterfaces(
-                        type.TypeDef.GetInterfaces(),
-                        interfaceTypeRef =>
+                    if (_typeRegistry.TryGetType(
+                        interfaceTypeRef,
+                        out var interfaceTypeReg))
+                    {
+                        foreach (var typeRef in interfaceTypeReg.References)
                         {
-                            if (_typeRegistry.TryGetType(
-                                interfaceTypeRef,
-                                out var interfaceTypeReg))
+                            state.NeedsAuth.Add(typeRef);
+
+                            if (!state.AbstractToConcrete.TryGetValue(
+                                typeRef,
+                                out var authTypeRefs))
                             {
-                                foreach (var typeRef in interfaceTypeReg.References)
-                                {
-                                    state.NeedsAuth.Add(typeRef);
-
-                                    if (!state.AbstractToConcrete.TryGetValue(
-                                        typeRef,
-                                        out var authTypeRefs))
-                                    {
-                                        authTypeRefs = [];
-                                        state.AbstractToConcrete.Add(typeRef, authTypeRefs);
-                                    }
-
-                                    authTypeRefs.Add(mainTypeRef);
-                                }
+                                authTypeRefs = [];
+                                state.AbstractToConcrete.Add(typeRef, authTypeRefs);
                             }
-                        },
-                        state);
 
-                    state.Completed.Clear();
-                }
-            }
+                            authTypeRefs.Add(mainTypeRef);
+                        }
+                    }
+                },
+                state);
+
+            state.Completed.Clear();
         }
     }
 
@@ -623,7 +640,7 @@ private State CreateState()
     }
 }
 
-static file class AuthorizationTypeInterceptorExtensions
+file static class AuthorizationTypeInterceptorExtensions
 {
     public static bool IsNodeField(this ObjectFieldDefinition fieldDef)
     {

src/HotChocolate/Core/src/Types/Configuration/TypeInitializer.cs

@@ -595,7 +595,7 @@ private void CompleteTypes()
 
     internal bool CompleteType(RegisteredType registeredType)
     {
-        if (registeredType.Status is TypeStatus.Completed)
+        if (registeredType.Type.IsCompleted)
         {
             return true;
         }
@@ -615,9 +615,10 @@ private void CompleteMetadata()
 
         foreach (var registeredType in _typeRegistry.Types)
         {
-            if (!registeredType.IsExtension)
+            if (registeredType is { IsExtension: false, Status: TypeStatus.Completed })
             {
                 registeredType.Type.CompleteMetadata(registeredType);
+                registeredType.Status = TypeStatus.MetadataCompleted;
             }
         }
 
@@ -635,6 +636,7 @@ private void MakeExecutable()
             if (!registeredType.IsExtension)
             {
                 registeredType.Type.MakeExecutable(registeredType);
+                registeredType.Status = TypeStatus.Executable;
             }
         }
 
@@ -650,6 +652,7 @@ private void FinalizeTypes()
             if (!registeredType.IsExtension)
             {
                 registeredType.Type.FinalizeType(registeredType);
+                registeredType.Status = TypeStatus.Finalized;
             }
         }
 

src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs

@@ -499,6 +499,30 @@ public async Task Authorize_Type_Field()
         Assert.Equal(401, value);
     }
 
+    [Fact]
+    public async Task Authorize_Node_Field_Schema()
+    {
+        // arrange
+        var handler = new AuthHandler(
+            resolver: (_, _) => AuthorizeResult.Allowed,
+            validation: (_, d) => d.Policy.EqualsOrdinal("READ_NODE")
+                ? AuthorizeResult.NotAllowed
+                : AuthorizeResult.Allowed);
+
+        // act
+        var services = CreateServices(
+            handler,
+            options =>
+            {
+                options.ConfigureNodeFields =
+                    descriptor => descriptor.Authorize("READ_NODE", ApplyPolicy.Validation);
+            });
+
+        // assert
+        var executor = await services.GetRequestExecutorAsync();
+        executor.Schema.MatchSnapshot();
+    }
+
     [Fact]
     public async Task Authorize_Node_Field()
     {
@@ -513,7 +537,7 @@ public async Task Authorize_Node_Field()
             options =>
             {
                 options.ConfigureNodeFields =
-                    descriptor => { descriptor.Authorize("READ_NODE", ApplyPolicy.Validation); };
+                    descriptor => descriptor.Authorize("READ_NODE", ApplyPolicy.Validation);
             });
         var executor = await services.GetRequestExecutorAsync();
 

src/HotChocolate/Core/test/Authorization.Tests/SchemaFirstAuthorizationTests.cs

@@ -0,0 +1,41 @@
+using HotChocolate.Execution;
+using HotChocolate.Resolvers;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace HotChocolate.Authorization;
+
+public class SchemaFirstAuthorizationTests
+{
+    [Fact]
+    public async Task Authorize_Apply_Can_Be_Omitted()
+    {
+        var schema = await new ServiceCollection()
+            .AddGraphQLServer()
+            .AddDocumentFromString(
+                """
+                type Query @authorize(roles: [ "policy_tester_noupdate", "policy_tester_update_noread", "authorizationHandlerTester" ])  {
+                    hello: String @authorize(roles: ["admin"])
+                }
+                """)
+            .AddResolver("Query", "hello", "world")
+            .AddAuthorizationHandler<MockAuth>()
+            .BuildSchemaAsync();
+
+        schema.MatchSnapshot();
+    }
+
+    private sealed class MockAuth : IAuthorizationHandler
+    {
+        public ValueTask<AuthorizeResult> AuthorizeAsync(
+            IMiddlewareContext context,
+            AuthorizeDirective directive,
+            CancellationToken cancellationToken = default)
+            => new(AuthorizeResult.NotAllowed);
+
+        public ValueTask<AuthorizeResult> AuthorizeAsync(
+            AuthorizationContext context,
+            IReadOnlyList<AuthorizeDirective> directives,
+            CancellationToken cancellationToken = default)
+            => new(AuthorizeResult.NotAllowed);
+    }
+}