EnableCors attribute does not work on OData controllers

[giulianob]

Assemblies affected
ASP.NET Core OData 8.0.6

Describe the bug
CORS is not correctly registered for OData routes when using [EnableCors].

Reproduce steps

1. Enable CORS on an OData controller
   sample/ODataCustomizedSample/Controllers/EnumsController.cs:21

+   [EnableCors("MyPolicy")]
    public class EmployeesController : ODataController

2. Enable CORS for the app:
   sample/ODataCustomizedSample/Startup.cs:36

+ services.AddCors(
+ options => options.AddPolicy("MyPolicy", builder => builder.WithOrigins("http://foo.com").AllowAnyMethod()));

sample/ODataCustomizedSample/Startup.cs:70

  app.UseRouting();            
+ app.UseCors();

Request/Response
Run the sample and send a CORS request:

OPTIONS http://localhost:5000/convention/Employees
Origin: http://foo.com
Accept: application/json
Access-Control-Request-Method: GET
Access-Control-Request-Headers: Content-Type

Response:

HTTP/1.1 405 Method Not Allowed
Connection: close
Date: Tue, 22 Mar 2022 17:34:21 GMT
Server: Kestrel
Content-Length: 0
Allow: GET, PATCH, POST

Expected behavior
CORS request should succeed. If you add the same attribute to the WeatherForecastController which is not an OData controller the request to OPTIONS http://localhost:5000/WeatherForecast will succeed.

Additional context
This happens because the OData ODataRoutingApplicationModelProvider runs after the built in CorsApplicationModelProvider. The CorsApplicationModelProvider is responsible for setting acceptCorsPreflight to true for the HttpMethodMetadata of every selector which has a CORS attribute. There are a couple of ways I know to fix this:

1. The ODataRoutingApplicationModelProvider needs to run before the CorsApplicationModelProvider so the selectors added by OData exist when CorsApplicationModelProvider runs. The CorsApplicationModelProvider uses a negative order so this may not be recommended.
2. The OData AddSelector method should look if the EnableCors attribute is defined and create the HttpMethodMetadata accordingly. For example:
   src/Microsoft.AspNetCore.OData/Extensions/ActionModelExtensions.cs:178

+           var acceptPreflight = action.Controller.Attributes.OfType<IDisableCorsAttribute>().Any() ||
+                                 action.Controller.Attributes.OfType<IEnableCorsAttribute>().Any() ||
+                                 action.Attributes.OfType<IDisableCorsAttribute>().Any() ||
+                                 action.Attributes.OfType<IEnableCorsAttribute>().Any();
                                  

            // If the methods have different case sensitive, for example, "get", "Get", in the ASP.NET Core 3.1,
            // It will throw "An item with the same key has already been added. Key: GET", in
            // HttpMethodMatcherPolicy.BuildJumpTable(Int32 exitDestination, IReadOnlyList`1 edges)
            // Another root cause is that in attribute routing, we reuse the HttpMethodMetadata, the method name is always "upper" case.
            // Therefore, we upper the http method name always.
            string[] methods = httpMethods.ToUpperInvariant().Split(',');
            foreach (string template in path.GetTemplates(options))
            {
                // Be noted: https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/ApplicationModels/ActionAttributeRouteModel.cs#L74-L75
                // No matter whether the action selector model is absolute route template, the controller's attribute will apply automatically
                // So, let's only create/update the action selector model
                SelectorModel selectorModel = action.Selectors.FirstOrDefault(s => s.AttributeRouteModel == null);
                if (hasAttributeRouteOnController || selectorModel == null)
                {
                    // Create a new selector model.
-                    selectorModel = CreateSelectorModel(action, methods);
+                    selectorModel = CreateSelectorModel(action, methods, acceptPreflight);
                    action.Selectors.Add(selectorModel);
                }
                else
                {
                    // Update the existing non attribute routing selector model.
-                    selectorModel = UpdateSelectorModel(selectorModel, methods);
+                    selectorModel = UpdateSelectorModel(selectorModel, methods, acceptPreflight);
                }

then pass the new argument in Create/UpdateSelectorModel: selectorModel.EndpointMetadata.Add(new HttpMethodMetadata(httpMethods, acceptPreflight));

[habbes]

@giulianob I was able to reproduce this issue. Would you be willing to contribute a pull request for this? We would be happy to review it,

PS: Using app.UseCors("MyPolicy") instead of [EnableCors] attribute seems to work for both cases.

[giulianob]

@habbes PR created 👌