Fixing CORS attribute (#544)

Registering selectors with CORS when controller/action use CORS attributes. Fixes #534. Co-authored-by: Giuliano Barberi <gbarberi@microsoft.com>
OData · Apr 4, 2022 · e0eb6be · e0eb6be
1 parent a10e8e5
commit e0eb6be
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 9 deletions.
diff --git a/src/Microsoft.AspNetCore.OData/Extensions/ActionModelExtensions.cs b/src/Microsoft.AspNetCore.OData/Extensions/ActionModelExtensions.cs
@@ -9,6 +9,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.Contracts;
 using System.Linq;
+using Microsoft.AspNetCore.Cors.Infrastructure;
 using Microsoft.AspNetCore.Mvc.ActionConstraints;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Mvc.Routing;
@@ -173,7 +174,13 @@ public static void AddSelector(this ActionModel action, string httpMethods, stri
             // let's always create new selector model for action.
             // Since the new created selector model is absolute attribute route, the controller attribute route doesn't apply to this selector model.
             bool hasAttributeRouteOnController = action.Controller.Selectors.Any(s => s.AttributeRouteModel != null);
-
+
+            // Check if CORS attribute is specified on action. New selectors need to be registered with CORS support.
+            bool 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)
@@ -189,13 +196,13 @@ public static void AddSelector(this ActionModel action, string httpMethods, stri
                 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);
                 }
 
                 ODataRoutingMetadata odataMetadata = new ODataRoutingMetadata(prefix, model, path);
@@ -216,7 +223,7 @@ public static void AddSelector(this ActionModel action, string httpMethods, stri
             }
         }
 
-        internal static SelectorModel UpdateSelectorModel(SelectorModel selectorModel, string[] httpMethods)
+        internal static SelectorModel UpdateSelectorModel(SelectorModel selectorModel, string[] httpMethods, bool acceptPreflight)
         {
             Contract.Assert(selectorModel != null);
 
@@ -257,15 +264,15 @@ internal static SelectorModel UpdateSelectorModel(SelectorModel selectorModel, s
             // append the http method metadata.
             Contract.Assert(httpMethods.Length >= 1);
             selectorModel.ActionConstraints.Add(new HttpMethodActionConstraint(httpMethods));
-            selectorModel.EndpointMetadata.Add(new HttpMethodMetadata(httpMethods));
+            selectorModel.EndpointMetadata.Add(new HttpMethodMetadata(httpMethods, acceptPreflight));
 
             // append controller attributes to action selector model? -- NO
             // Be noted: https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/ApplicationModels/ActionAttributeRouteModel.cs#L74-L75
 
             return selectorModel;
         }
 
-        internal static SelectorModel CreateSelectorModel(ActionModel actionModel, string[] httpMethods)
+        internal static SelectorModel CreateSelectorModel(ActionModel actionModel, string[] httpMethods, bool acceptPreflight)
         {
             Contract.Assert(actionModel != null);
 
@@ -309,7 +316,7 @@ internal static SelectorModel CreateSelectorModel(ActionModel actionModel, strin
 
             Contract.Assert(httpMethods.Length >= 1);
             selectorModel.ActionConstraints.Add(new HttpMethodActionConstraint(httpMethods));
-            selectorModel.EndpointMetadata.Add(new HttpMethodMetadata(httpMethods));
+            selectorModel.EndpointMetadata.Add(new HttpMethodMetadata(httpMethods, acceptPreflight));
 
             // append controller attributes to action selector model? -- NO
             // Be noted: https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/src/ApplicationModels/ActionAttributeRouteModel.cs#L74-L75

diff --git a/test/Microsoft.AspNetCore.OData.Tests/Extensions/ActionModelExtensionsTests.cs b/test/Microsoft.AspNetCore.OData.Tests/Extensions/ActionModelExtensionsTests.cs
@@ -5,12 +5,17 @@
 // </copyright>
 //------------------------------------------------------------------------------
 
+using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
+using Microsoft.AspNetCore.Cors;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.OData.Extensions;
 using Microsoft.AspNetCore.OData.Routing.Attributes;
+using Microsoft.AspNetCore.OData.Routing.Template;
 using Microsoft.AspNetCore.OData.Tests.Commons;
+using Microsoft.AspNetCore.Routing;
 using Microsoft.OData.Edm;
 using Moq;
 using Xunit;
@@ -113,15 +118,50 @@ public void AddSelector_ThrowsArgumentNull_ForInputParameter()
             IEdmModel model = new Mock<IEdmModel>().Object;
             ExceptionAssert.ThrowsArgumentNull(() => action.AddSelector(httpMethods, null, model, null), "path");
         }
+
+        [Theory]
+        [InlineData(typeof(TestController), "Get", true)]
+        [InlineData(typeof(TestController), "Create", true)]
+        [InlineData(typeof(TestController), "Index", false)]
+        [InlineData(typeof(CorsTestController), "Index", true)]
+        public void AddSelector_AddsCors_ForActionsWithCorsAttribute(Type controllerType, string actionName, bool expectedCorsSetting)
+        {
+            // Arrange
+            IEdmModel model = new Mock<IEdmModel>().Object;
+            MethodInfo methodInfo = controllerType.GetMethod(actionName);
+            ActionModel action = methodInfo.BuildActionModel();
+            action.Controller = ControllerModelHelpers.BuildControllerModel(controllerType);
+
+            // Act
+            action.AddSelector("Get", string.Empty, model, new ODataPathTemplate(CountSegmentTemplate.Instance));
+
+            // Assert
+            SelectorModel newSelector = action.Selectors.FirstOrDefault();
+            Assert.NotNull(newSelector);
+            HttpMethodMetadata httpMethodMetadata = newSelector.EndpointMetadata.OfType<HttpMethodMetadata>().FirstOrDefault();
+            Assert.NotNull(httpMethodMetadata);
+            Assert.Equal(httpMethodMetadata.AcceptCorsPreflight, expectedCorsSetting);
+        }
     }
 
     internal class TestController
     {
         public void Index(int id)
-        {
-        }
+        { }
 
+        [EnableCors]
         public void Get(int key)
         { }
+
+        [DisableCors]
+        public void Create()
+        { }
+    }
+
+    [EnableCors]
+    internal class CorsTestController
+    {
+        public void Index(int id)
+        { }
     }
 }