Endpoint routing ignores [Authorize] attribute

[nikolai-mb]

Hello

I'm experiencing a strange issue where the [Authorize] attribute is not honored if endpoint routing is enabled togheter with odata api versioning. If endpoint routing is enabled, odata routes can be queried regardless if the request is authenticated or not.

I'm currently testing in .NET 5, but issue is also reproducible in .NET Core 3.1.

.NET 5 reproduction is done with the following dependencies:

• Microsoft.AspNetCore.Authentication.JwtBearer - 5.0.0
• Microsoft.AspNetCore.OData - 7.5.1
• Microsoft.AspNetCore.OData.Versioning.ApiExplorer - 5.0.0
• Swashbuckle.AspNetCore - 5.6.3

Authentication configuration is as follows in Startup.cs where "fakeauthority" is provided to force a 401 response.

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme).AddJwtBearer(o =>
{
    o.Authority = "https://fakeauthority.com";
});

Expected behaviour
curl http://localhost:51389/api/v1/users
curl : The remote server returned an error: (401) Unauthorized.

This works as expected with endpoint routing disabled

services.AddMvc(options => options.EnableEndpointRouting = false);

Actual behaviour with endpoint routing enabled
curl http://localhost:51389/api/v1/users
StatusCode : 200 StatusDescription : OK Content : {"@odata.context":"http://localhost:51389/api/v1/$metadata#Users","value":[{"id":1,"firstname":"Bob ","lastname":"Marley"},{"id":2,"firstname":"Mahatma","lastname":"Gandhi"}]} RawContent : HTTP/1.1 200 OK

I have uploaded a tiny project where issue can be reproduced:
https://github.com/nikolai-mb/ODataBugRepro

In the Startup.cs file of the repo above, the following variable exists:

private static readonly bool _useEndpointRouting = false;

When value is set to false, controller returns 401 as expected, but change variable to true and the issue appears.

I have not been able to reproduce with OData alone. Only with OData together with api-versioning and creating the OData route with MapVersionedODataRoute

Any help would be much appricated

[commonsensesoftware]

Strange. I'm kind of curious how this even compiles. The package doesn't support the net5.0 TFM. In fact, neither does the OData package. They are revamping the entire library - again, for .NET 5.0 in OData 8.0. There's a preview package available for OData, but not API Versioning - yet. You provided a repro and said you got the same results in netcoreapp3.1 so I'll give that a whirl.

[nikolai-mb]

Thanks for the quick reply, i suppose you raise a good question as to why it compiles if the packages do not support net5.0.
I'm able to reproduce the issue as mentioned in netcoreapp3.1

The repro i uploaded can be tested by changing targetframework to netcoreapp3.1 and downgrading the package
Microsoft.AspNetCore.Authentication.JwtBearer to version 3.1.10. Changing _useEndpointRouting = true in Startup.cs reveals the same issue for me in 3.1

Let me know if you are unable to reproduce and i'll try and provide more details

[anranruye]

I met the same problem in my project, targeting. net core 3.1. [Authorize] works correctly with none-odata endpoints in the same project. so it can't be a configuration mistake.

[anranruye]

@commonsensesoftware @nikolai-mb

It seems that this issue is casused by EndpointMetadata missing.

It happens in this method when rebuilding ActionDescriptor..

https://github.com/microsoft/aspnet-api-versioning/blob/beed143f753dc0e5d4b6a1abcd27ff9873e9d0c6/src/Microsoft.AspNetCore.OData.Versioning/AspNet.OData/Routing/ODataRouteBindingInfoConvention.cs#L61

[anranruye]

@nikolai-mb

Using an attribute class inherited from AuthorizeAttribute and IFilterMetadata instead of [Authorize] attribute can be a workaround.

public class MyAuthorizeAttribute : AuthorizeAttribute, IFilterMetadata
    {
        public MyAuthorizeAttribute()
        {
        }

        public MyAuthorizeAttribute(string policy) : base(policy)
        {
        }
    }

[strandman]

Bumping this,
I experience the the same issue, [AuthorizeAttribute] not working on OData Endpoint routing using .NET 5.
As @anranruye stated, [ODataRouteBindingInfoConvention.Clone] is missing [EndpointMetadata]

Thanks for the workaround @anranruye!

[manuel-fernandez-rodriguez]

@anranruye @nikolai-mb
If modifying the attribute is not an option, there's another possibility: manually adding the missing attribute to the endpoint metadata:
In Startup.Configure:

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers()
                    .AddTokenValidationToODataActions();
                endpoints.Select().Filter().Expand().OrderBy().Count().MaxTop(10);
                endpoints.MapVersionedODataRoute("odata", "v{version:apiVersion}/odata", modelBuilder.GetEdmModels());
            });

Then, in the extension method AddTokenValidationToODataActions , we add the attribute as needed. This particular example adds a custom TokenAuthentication attribute to every action explicitly decorated with it, and to all actions of every controller which has been decorated with it:

        public static ControllerActionEndpointConventionBuilder AddTokenValidationToODataActions(this ControllerActionEndpointConventionBuilder builder)
        {
            builder.Add(x =>
            {
                var controllerActionDescription = x.Metadata.OfType<ControllerActionDescriptor>().FirstOrDefault();
                if (controllerActionDescription != null)
                {
                    var actionMethod = controllerActionDescription.MethodInfo;
                    var attr = actionMethod.GetCustomAttributes(typeof(TokenAuthentication), true).FirstOrDefault();
                    if (attr == null)
                    {
                        var controllerType = controllerActionDescription.ControllerTypeInfo;
                        attr = controllerType.GetCustomAttributes(typeof(TokenAuthentication), true).FirstOrDefault();
                    }

                    if (attr != null)
                    {
                        x.Metadata.Add(attr);
                    }
                }
            });

            return builder;
        }

It might make sense, as well, to make the extension method generic, taking the attribute type as a type parameter, so this same workaround will help with multiple missing attribute types.

Hope it helps.

[adrianaxente]

I made @manuel-fernandez-rodriguez method to accept a list of attribute types and put into EndpointBuilder metadata all attribute instances.
I tested it and it seems to work:

        app.UseEndpoints(endpoints =>
        {
            endpoints.MapControllers()
                .AddAttributesToEndpointBuilderMetadata(typeof(AuthorizeAttribute));
            endpoints.Select().Filter().Expand().OrderBy().Count().MaxTop(10);
            endpoints.MapVersionedODataRoute("odata", "v{version:apiVersion}/odata", modelBuilder.GetEdmModels());
        });

And the extension method and the class having it:

        public static class ControllerActionEndpointConventionBuilderExtensions
        {
            /// <summary>
            /// Adds action or controller attributes to <see cref="EndpointBuilder" Metadata/>
            /// </summary>
            /// <param name="builder">
            /// The <see cref="ControllerActionEndpointConventionBuilder"/>
            /// </param>
            /// <param name="attributeTypes">
            /// Array of attributes types <see cref="Type"/> to add
            /// </param>
            /// <returns>
            /// <see cref="ControllerActionEndpointConventionBuilder"/>
            /// </returns>
            /// <remarks>
            /// This method is used to fix <see href="https://github.com/dotnet/aspnet-api-versioning/issues/695"/>
            /// Endpoint routing ignores <see cref="AuthorizeAttribute"/> attribute when using OData Api versioning
            /// </remarks>
            public static ControllerActionEndpointConventionBuilder AddAttributesToEndpointBuilderMetadata(
                this ControllerActionEndpointConventionBuilder builder,
                params Type[] attributeTypes)
            {
                if (builder is null)
                {
                    throw new ArgumentNullException(nameof(builder));
                }
        
                if (attributeTypes?.Length == 0)
                {
                    return builder;
                }
        
                builder.Add(eb =>
                {
                    foreach(var attributeType in attributeTypes)
                    {
                        eb.AddAttributeToMetadata(attributeType);
                    }
                });
        
                return builder;
            }
        
            private static EndpointBuilder AddAttributeToMetadata(this EndpointBuilder builder, Type attributeType)
            {
                var controllerActionDescription = builder
                    .Metadata
                    .OfType<ControllerActionDescriptor>()
                    .FirstOrDefault();
        
                if (controllerActionDescription is null)
                {
                    return builder;
                }
        
                var actionMethod = controllerActionDescription.MethodInfo;
                var attributes = actionMethod.GetCustomAttributes(attributeType, true)?.ToList();
        
                if (attributes?.Count == 0)
                {
                    var controllerType = controllerActionDescription.ControllerTypeInfo;
                    attributes = controllerType.GetCustomAttributes(attributeType, true)?.ToList();
                }
        
                if (attributes?.Count == 0)
                {
                    return builder;
                }
        
                foreach(var attribute in attributes)
                {
                    builder.Metadata.Add(attribute);
                }
        
                return builder;
            }
        }

[mcmastercloud]

@adrianaxente - You are A M A Z I N G. Thanks for this; it worked perfectly well for me too. Relieve to get a fix for this.

[vikneshrajspui]

I am on dotnet-6 using Microsoft.AspNetCore.OData (7.5.12) and tried all the above approach and it is not working for me.

Attribute wrapping approach worked for me with dotnet-5 though.

Appreciate your suggestions

[commonsensesoftware]

@vikneshrajspui OData is not officially supported on .NET 5.0 or 6.0 - yet. OData 8.0 brought significant breaking changes to the infrastructure that is essentially a rewrite. I've sketched out a rough implementation and finally ready to start some active development. If you had things working in .NET 5.0 or earlier, my recommendation is to stay there - for now.