diff --git a/src/HotChocolate/Core/src/Authorization/AuthorizationTypeInterceptor.cs b/src/HotChocolate/Core/src/Authorization/AuthorizationTypeInterceptor.cs index 869fe147bf1..2ad37b1cdca 100644 --- a/src/HotChocolate/Core/src/Authorization/AuthorizationTypeInterceptor.cs +++ b/src/HotChocolate/Core/src/Authorization/AuthorizationTypeInterceptor.cs @@ -1,3 +1,4 @@ +using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using HotChocolate.Configuration; @@ -8,11 +9,19 @@ using HotChocolate.Types.Relay; using HotChocolate.Utilities; using static HotChocolate.Authorization.AuthorizeDirectiveType.Names; +using static HotChocolate.Authorization.Properties.AuthCoreResources; namespace HotChocolate.Authorization; internal sealed partial class AuthorizationTypeInterceptor : TypeInterceptor { + private const string AspNetCoreAuthorizeAttributeName = "Microsoft.AspNetCore.Authorization.AuthorizeAttribute"; + private const string AspNetCoreAllowAnonymousAttributeName = + "Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute"; + + private static readonly string s_authorizeAttributeName = typeof(AuthorizeAttribute).FullName!; + private static readonly string s_allowAnonymousAttributeName = typeof(AllowAnonymousAttribute).FullName!; + private readonly List _objectTypes = []; private readonly List _unionTypes = []; private readonly Dictionary _directives = []; @@ -121,14 +130,79 @@ public override void OnBeforeCompleteMetadata( ITypeCompletionContext completionContext, TypeSystemConfiguration configuration) { + if (configuration is not ObjectTypeConfiguration typeDef) + { + return; + } + // last in the initialization we need to intercept the query type and ensure that // authorization configuration is applied to the special introspection and node fields. - if (ReferenceEquals(_queryContext, completionContext) && - configuration is ObjectTypeConfiguration typeDef) + if (ReferenceEquals(_queryContext, completionContext)) { var state = _state ?? throw ThrowHelper.StateNotInitialized(); HandleSpecialQueryFields(new ObjectTypeInfo(completionContext, typeDef), state); } + + if (_context.Options.ErrorOnAspNetCoreAuthorizationAttributes && !completionContext.IsIntrospectionType) + { + var runtimeType = typeDef.RuntimeType; + var attributesOnType = runtimeType.GetCustomAttributes().ToArray(); + + if (ContainsNamedAttribute(attributesOnType, AspNetCoreAuthorizeAttributeName)) + { + completionContext.ReportError( + UnsupportedAspNetCoreAttributeError( + AspNetCoreAuthorizeAttributeName, + s_authorizeAttributeName, + runtimeType)); + return; + } + + if (ContainsNamedAttribute(attributesOnType, AspNetCoreAllowAnonymousAttributeName)) + { + completionContext.ReportError( + UnsupportedAspNetCoreAttributeError( + AspNetCoreAllowAnonymousAttributeName, + s_allowAnonymousAttributeName, + runtimeType)); + return; + } + + foreach (var field in typeDef.Fields) + { + if (field.IsIntrospectionField) + { + continue; + } + + var fieldMember = field.ResolverMember ?? field.Member; + + if (fieldMember is not null) + { + var attributesOnResolver = fieldMember.GetCustomAttributes().ToArray(); + + if (ContainsNamedAttribute(attributesOnResolver, AspNetCoreAuthorizeAttributeName)) + { + completionContext.ReportError( + UnsupportedAspNetCoreAttributeError( + AspNetCoreAuthorizeAttributeName, + s_authorizeAttributeName, + fieldMember)); + return; + } + + if (ContainsNamedAttribute(attributesOnResolver, AspNetCoreAllowAnonymousAttributeName)) + { + completionContext.ReportError( + UnsupportedAspNetCoreAttributeError( + AspNetCoreAllowAnonymousAttributeName, + s_allowAnonymousAttributeName, + fieldMember)); + return; + } + } + } + } } public override void OnAfterMakeExecutable() @@ -620,6 +694,36 @@ private static bool IsAuthorizedType(T definition) private State CreateState() => new(_context.GetAuthorizationOptions()); + + private static bool ContainsNamedAttribute(Attribute[] attributes, string nameOfAttribute) + => attributes.Any(a => a.GetType().FullName == nameOfAttribute); + + private static ISchemaError UnsupportedAspNetCoreAttributeError( + string aspNetCoreAttributeName, + string properAttributeName, + Type runtimeType) + { + return SchemaErrorBuilder.New() + .SetMessage(string.Format(AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnType, + aspNetCoreAttributeName, runtimeType.FullName, properAttributeName)) + .SetCode(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute) + .Build(); + } + + private static ISchemaError UnsupportedAspNetCoreAttributeError( + string aspNetCoreAttributeName, + string properAttributeName, + MemberInfo member) + { + var nameOfDeclaringType = member.DeclaringType?.FullName; + var nameOfMember = member.Name; + + return SchemaErrorBuilder.New() + .SetMessage(string.Format(AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnMember, + aspNetCoreAttributeName, nameOfDeclaringType, nameOfMember, properAttributeName)) + .SetCode(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute) + .Build(); + } } file static class AuthorizationTypeInterceptorExtensions diff --git a/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.Designer.cs b/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.Designer.cs index 519551433f2..90610d65f1b 100644 --- a/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.Designer.cs +++ b/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.Designer.cs @@ -86,5 +86,17 @@ internal static string ThrowHelper_UnableToResolveTypeReg { return ResourceManager.GetString("ThrowHelper_UnableToResolveTypeReg", resourceCulture); } } + + internal static string AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnType { + get { + return ResourceManager.GetString("AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnType", resourceCulture); + } + } + + internal static string AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnMember { + get { + return ResourceManager.GetString("AuthorizationTypeInterceptor_UnsupportedAspNetCoreAttributeOnMember", resourceCulture); + } + } } } diff --git a/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.resx b/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.resx index d6642c013c7..cae96320485 100644 --- a/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.resx +++ b/src/HotChocolate/Core/src/Authorization/Properties/AuthCoreResources.resx @@ -39,4 +39,10 @@ Unable to resolve a type registration. + + Found unsupported `{0}` on `{1}`. Use `{2}` instead. + + + Found unsupported `{0}` on `{1}.{2}`. Use `{3}` instead. + diff --git a/src/HotChocolate/Core/src/Types/IReadOnlySchemaOptions.cs b/src/HotChocolate/Core/src/Types/IReadOnlySchemaOptions.cs index 365414ac29f..81b40655118 100644 --- a/src/HotChocolate/Core/src/Types/IReadOnlySchemaOptions.cs +++ b/src/HotChocolate/Core/src/Types/IReadOnlySchemaOptions.cs @@ -207,4 +207,10 @@ public interface IReadOnlySchemaOptions /// to the DataLoader promise cache. /// bool PublishRootFieldPagesToPromiseCache { get; } + + /// + /// Errors if either an ASP.NET Core [Authorize] or [AllowAnonymous] attribute + /// is used on a Hot Chocolate resolver or type definition. + /// + bool ErrorOnAspNetCoreAuthorizationAttributes { get; } } diff --git a/src/HotChocolate/Core/src/Types/SchemaOptions.cs b/src/HotChocolate/Core/src/Types/SchemaOptions.cs index adfcdb53cd8..e2a67ca4c9e 100644 --- a/src/HotChocolate/Core/src/Types/SchemaOptions.cs +++ b/src/HotChocolate/Core/src/Types/SchemaOptions.cs @@ -137,6 +137,9 @@ public FieldBindingFlags DefaultFieldBindingFlags /// public bool PublishRootFieldPagesToPromiseCache { get; set; } = true; + /// + public bool ErrorOnAspNetCoreAuthorizationAttributes { get; set; } = true; + /// /// Creates a mutable options object from a read-only options object. /// @@ -175,7 +178,8 @@ public static SchemaOptions FromOptions(IReadOnlySchemaOptions options) StripLeadingIFromInterface = options.StripLeadingIFromInterface, EnableTag = options.EnableTag, DefaultQueryDependencyInjectionScope = options.DefaultQueryDependencyInjectionScope, - DefaultMutationDependencyInjectionScope = options.DefaultMutationDependencyInjectionScope + DefaultMutationDependencyInjectionScope = options.DefaultMutationDependencyInjectionScope, + ErrorOnAspNetCoreAuthorizationAttributes = options.ErrorOnAspNetCoreAuthorizationAttributes }; } } diff --git a/src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs b/src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs index 079fb0def45..1e3ddff579a 100644 --- a/src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs +++ b/src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs @@ -955,6 +955,78 @@ public async Task Skip_After_Validation_For_Null() """); } + [Fact] + public async Task Microsoft_AuthorizeAttribute_On_Method_Produces_Error() + { + var builder = new ServiceCollection() + .AddGraphQL() + .AddQueryType() + .AddAuthorizationCore(); + + var act = async () => await builder.BuildSchemaAsync(); + + var exception = await Assert.ThrowsAsync(act); + var error = exception.Errors.First(); + Assert.Equal(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute, error.Code); + Assert.Equal( + "Found unsupported `Microsoft.AspNetCore.Authorization.AuthorizeAttribute` on `HotChocolate.Authorization.AnnotationBasedAuthorizationTests+QueryWithMicrosoftAuthorizeAttributeOnMethod.Field`. Use `HotChocolate.Authorization.AuthorizeAttribute` instead.", + error.Message); + } + + [Fact] + public async Task Microsoft_AllowAnonymousAttribute_On_Method_Produces_Error() + { + var builder = new ServiceCollection() + .AddGraphQL() + .AddQueryType() + .AddAuthorizationCore(); + + var act = async () => await builder.BuildSchemaAsync(); + + var exception = await Assert.ThrowsAsync(act); + var error = exception.Errors.First(); + Assert.Equal(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute, error.Code); + Assert.Equal( + "Found unsupported `Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute` on `HotChocolate.Authorization.AnnotationBasedAuthorizationTests+QueryWithMicrosoftAllowAnonymousAttributeOnMethod.Field`. Use `HotChocolate.Authorization.AllowAnonymousAttribute` instead.", + error.Message); + } + + [Fact] + public async Task Microsoft_AuthorizeAttribute_On_Type_Produces_Error() + { + var builder = new ServiceCollection() + .AddGraphQL() + .AddQueryType() + .AddAuthorizationCore(); + + var act = async () => await builder.BuildSchemaAsync(); + + var exception = await Assert.ThrowsAsync(act); + var error = exception.Errors.First(); + Assert.Equal(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute, error.Code); + Assert.Equal( + "Found unsupported `Microsoft.AspNetCore.Authorization.AuthorizeAttribute` on `HotChocolate.Authorization.AnnotationBasedAuthorizationTests+QueryWithMicrosoftAuthorizeAttribute`. Use `HotChocolate.Authorization.AuthorizeAttribute` instead.", + error.Message); + } + + [Fact] + public async Task Microsoft_AllowAnonymousAttribute_On_Type_Produces_Error() + { + var builder = new ServiceCollection() + .AddGraphQL() + .AddQueryType() + .AddAuthorizationCore(); + + var act = async () => await builder.BuildSchemaAsync(); + + var exception = await Assert.ThrowsAsync(act); + var error = exception.Errors.First(); + Assert.Equal(ErrorCodes.Schema.UnsupportedAspNetCoreAuthorizationAttribute, error.Code); + Assert.Equal( + "Found unsupported `Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute` on `HotChocolate.Authorization.AnnotationBasedAuthorizationTests+QueryWithMicrosoftAllowAnonymousAttribute`. Use `HotChocolate.Authorization.AllowAnonymousAttribute` instead.", + error.Message); + } + private static IServiceProvider CreateServices( IAuthorizationHandler handler, Action? configure = null) @@ -971,6 +1043,30 @@ private static IServiceProvider CreateServices( .Services .BuildServiceProvider(); + public class QueryWithMicrosoftAuthorizeAttributeOnMethod + { + [Microsoft.AspNetCore.Authorization.Authorize] + public string Field() => "foo"; + } + + public class QueryWithMicrosoftAllowAnonymousAttributeOnMethod + { + [Microsoft.AspNetCore.Authorization.AllowAnonymous] + public string Field() => "foo"; + } + + [Microsoft.AspNetCore.Authorization.Authorize] + public class QueryWithMicrosoftAuthorizeAttribute + { + public string Field() => "foo"; + } + + [Microsoft.AspNetCore.Authorization.AllowAnonymous] + public class QueryWithMicrosoftAllowAnonymousAttribute + { + public string Field() => "foo"; + } + [FooDirective] [Authorize("QUERY", ApplyPolicy.Validation)] [Authorize("QUERY2", ApplyPolicy.BeforeResolver)] diff --git a/src/HotChocolate/Primitives/src/Primitives/ErrorCodes.cs b/src/HotChocolate/Primitives/src/Primitives/ErrorCodes.cs index 089b520795f..b81ef509123 100644 --- a/src/HotChocolate/Primitives/src/Primitives/ErrorCodes.cs +++ b/src/HotChocolate/Primitives/src/Primitives/ErrorCodes.cs @@ -253,6 +253,12 @@ public static class Schema /// The specified directive argument does not exist. /// public const string UnknownDirectiveArgument = "HC0072"; + + /// + /// An underlying schema runtime type / member is annotated with a + /// Microsoft.AspNetCore.Authorization.* attribute that is not supported by Hot Chocolate. + /// + public const string UnsupportedAspNetCoreAuthorizationAttribute = "HC0090"; } public static class Scalars @@ -264,7 +270,7 @@ public static class Scalars /// /// Either the syntax node is invalid when parsing the literal or the syntax - /// node value has an invalid format. + /// node value has an invalid format.` /// public const string InvalidSyntaxFormat = "HC0002"; } diff --git a/website/src/docs/hotchocolate/v16/migrating/migrate-from-15-to-16.md b/website/src/docs/hotchocolate/v16/migrating/migrate-from-15-to-16.md index ced1f7b660f..8f8de20b0ca 100644 --- a/website/src/docs/hotchocolate/v16/migrating/migrate-from-15-to-16.md +++ b/website/src/docs/hotchocolate/v16/migrating/migrate-from-15-to-16.md @@ -20,6 +20,24 @@ The `@skip` and `@include` directives are now disallowed on root subscription fi Deprecating a field now requires the implemented field in the interface to also be deprecated, as specified in the [draft specification](https://spec.graphql.org/draft/#sec-Objects.Type-Validation). +## Accidental use of `Microsoft.AspNetCore.Authorization.*` attributes throws an error + +Since our authorization attributes (`[Authorize]` and `[AllowAnonymous]`) share the same names as the regular ASP.NET attributes, it's easy to accidentally use the wrong ones. +In the worst-case scenario, this could result in your field or type ending up without any authorization being applied! + +To prevent this, we've added a check that throws an error during schema generation if it detects `Microsoft.AspNetCore.Authorization.*` attributes being applied to a GraphQL resolver. + +> Note: Keep in mind that your clients might currently rely on the absence of authorization. + +You can disable this new validation by setting the `ErrorOnAspNetCoreAuthorizationAttributes` option to `false`: + +```csharp +builder.Services.AddGraphQLServer() + .ModifyOptions(options => { + options.ErrorOnAspNetCoreAuthorizationAttributes = false; + }) +``` + # Deprecations Things that will continue to function this release, but we encourage you to move away from.