Throw if CORS policy is configured to allow credentials and any origin (#7751)

* Throw if CORS policy is configured to allow credentials and any origin

Fixes https://github.com/aspnet/AspNetCore/issues/3106

src/Middleware/CORS/samples/SampleDestination/Startup.cs

@@ -50,8 +50,7 @@ namespace SampleDestination
                 options.AddPolicy("AllowAll", policy => policy
                     .AllowAnyOrigin()
                     .AllowAnyMethod()
-                    .AllowAnyHeader()
+                    .AllowAnyHeader());
-                    .AllowCredentials());
             });
             services.AddRouting();
         }

src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs

@@ -73,8 +73,7 @@ namespace SampleDestination
                 innerBuilder.UseCors(policy => policy
                     .AllowAnyOrigin()
                     .AllowAnyMethod()
-                    .AllowAnyHeader()
+                    .AllowAnyHeader());
-                    .AllowCredentials());
 
                 innerBuilder.UseMiddleware<SampleMiddleware>();
             });

src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs

@@ -224,6 +224,11 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
         /// <returns>The constructed <see cref="CorsPolicy"/>.</returns>
         public CorsPolicy Build()
         {
+            if (_policy.AllowAnyOrigin && _policy.SupportsCredentials)
+            {
+                throw new InvalidOperationException(Resources.InsecureConfiguration);
+            }
+
             return _policy;
         }
 

src/Middleware/CORS/src/Infrastructure/CorsService.cs

@@ -8,7 +8,6 @@ using System.Linq;
 using Microsoft.AspNetCore.Cors.Internal;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
-using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using Microsoft.Extensions.Primitives;
 
@@ -77,7 +76,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
 
             if (policy.AllowAnyOrigin && policy.SupportsCredentials)
             {
-                _logger.InsecureConfiguration();
+                throw new ArgumentException(Resources.InsecureConfiguration, nameof(policy));
             }
 
             var origin = context.Request.Headers[CorsConstants.Origin];

src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs

@@ -18,7 +18,6 @@ namespace Microsoft.AspNetCore.Cors.Internal
         private static readonly Action<ILogger, string, Exception> _requestHeaderNotAllowed;
         private static readonly Action<ILogger, Exception> _failedToSetCorsHeaders;
         private static readonly Action<ILogger, Exception> _noCorsPolicyFound;
-        private static readonly Action<ILogger, Exception> _insecureConfiguration;
         private static readonly Action<ILogger, Exception> _isNotPreflightRequest;
 
         static CORSLoggerExtensions()
@@ -73,11 +72,6 @@ namespace Microsoft.AspNetCore.Cors.Internal
                 new EventId(10, "NoCorsPolicyFound"),
                 "No CORS policy found for the specified request.");
 
-            _insecureConfiguration = LoggerMessage.Define(
-                LogLevel.Warning,
-                new EventId(11, "InsecureConfiguration"),
-                "The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the policy by listing individual origins if credentials needs to be supported.");
-
             _isNotPreflightRequest = LoggerMessage.Define(
                 LogLevel.Debug,
                 new EventId(12, "IsNotPreflightRequest"),
@@ -134,11 +128,6 @@ namespace Microsoft.AspNetCore.Cors.Internal
             _noCorsPolicyFound(logger, null);
         }
 
-        public static void InsecureConfiguration(this ILogger logger)
-        {
-            _insecureConfiguration(logger, null);
-        }
-
         public static void IsNotPreflightRequest(this ILogger logger)
         {
             _isNotPreflightRequest(logger, null);

src/Middleware/CORS/src/Properties/Resources.Designer.cs

@@ -0,0 +1,58 @@
+// <auto-generated />
+namespace Microsoft.AspNetCore.Cors
+{
+    using System.Globalization;
+    using System.Reflection;
+    using System.Resources;
+
+    internal static class Resources
+    {
+        private static readonly ResourceManager _resourceManager
+            = new ResourceManager("Microsoft.AspNetCore.Cors.Resources", typeof(Resources).GetTypeInfo().Assembly);
+
+        /// <summary>
+        /// The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the CORS policy by listing individual origins if credentials needs to be supported.
+        /// </summary>
+        internal static string InsecureConfiguration
+        {
+            get => GetString("InsecureConfiguration");
+        }
+
+        /// <summary>
+        /// The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the CORS policy by listing individual origins if credentials needs to be supported.
+        /// </summary>
+        internal static string FormatInsecureConfiguration()
+            => GetString("InsecureConfiguration");
+
+        /// <summary>
+        /// PreflightMaxAge must be greater than or equal to 0.
+        /// </summary>
+        internal static string PreflightMaxAgeOutOfRange
+        {
+            get => GetString("PreflightMaxAgeOutOfRange");
+        }
+
+        /// <summary>
+        /// PreflightMaxAge must be greater than or equal to 0.
+        /// </summary>
+        internal static string FormatPreflightMaxAgeOutOfRange()
+            => GetString("PreflightMaxAgeOutOfRange");
+
+        private static string GetString(string name, params string[] formatterNames)
+        {
+            var value = _resourceManager.GetString(name);
+
+            System.Diagnostics.Debug.Assert(value != null);
+
+            if (formatterNames != null)
+            {
+                for (var i = 0; i < formatterNames.Length; i++)
+                {
+                    value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}");
+                }
+            }
+
+            return value;
+        }
+    }
+}

src/Middleware/CORS/src/Resources.Designer.cs

@@ -1,71 +0,0 @@
-//------------------------------------------------------------------------------
-// <auto-generated>
-//     This code was generated by a tool.
-//     Runtime Version:4.0.30319.42000
-//
-//     Changes to this file may cause incorrect behavior and will be lost if
-//     the code is regenerated.
-// </auto-generated>
-//------------------------------------------------------------------------------
-
-namespace Microsoft.AspNetCore.Cors {
-    using System;
-    using System.Reflection;
-    
-    
-    /// <summary>
-    ///    A strongly-typed resource class, for looking up localized strings, etc.
-    /// </summary>
-    // This class was auto-generated by the StronglyTypedResourceBuilder
-    // class via a tool like ResGen or Visual Studio.
-    // To add or remove a member, edit your .ResX file then rerun ResGen
-    // with the /str option, or rebuild your VS project.
-    [global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
-    [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()]
-    internal class Resources {
-        
-        private static global::System.Resources.ResourceManager resourceMan;
-        
-        private static global::System.Globalization.CultureInfo resourceCulture;
-        
-        internal Resources() {
-        }
-        
-        /// <summary>
-        ///    Returns the cached ResourceManager instance used by this class.
-        /// </summary>
-        [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)]
-        internal static global::System.Resources.ResourceManager ResourceManager {
-            get {
-                if (object.ReferenceEquals(resourceMan, null)) {
-                    global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.AspNetCore.Cors.Resources", typeof(Resources).GetTypeInfo().Assembly);
-                    resourceMan = temp;
-                }
-                return resourceMan;
-            }
-        }
-        
-        /// <summary>
-        ///    Overrides the current thread's CurrentUICulture property for all
-        ///    resource lookups using this strongly typed resource class.
-        /// </summary>
-        [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)]
-        internal static global::System.Globalization.CultureInfo Culture {
-            get {
-                return resourceCulture;
-            }
-            set {
-                resourceCulture = value;
-            }
-        }
-        
-        /// <summary>
-        ///    Looks up a localized string similar to PreflightMaxAge must be greater than or equal to 0..
-        /// </summary>
-        internal static string PreflightMaxAgeOutOfRange {
-            get {
-                return ResourceManager.GetString("PreflightMaxAgeOutOfRange", resourceCulture);
-            }
-        }
-    }
-}

src/Middleware/CORS/src/Resources.resx

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-    <!-- 
+  <!-- 
     Microsoft ResX Schema 
     
     Version 2.0
@@ -60,63 +60,66 @@
             : and then encoded with base64 encoding.
     -->
   <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
-      <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
+    <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
-      <xsd:element name="root" msdata:IsDataSet="true">
+    <xsd:element name="root" msdata:IsDataSet="true">
-          <xsd:complexType>
+      <xsd:complexType>
-              <xsd:choice maxOccurs="unbounded">
+        <xsd:choice maxOccurs="unbounded">
-                  <xsd:element name="metadata">
+          <xsd:element name="metadata">
-                      <xsd:complexType>
+            <xsd:complexType>
-                          <xsd:sequence>
+              <xsd:sequence>
-                              <xsd:element name="value" type="xsd:string" minOccurs="0" />
+                <xsd:element name="value" type="xsd:string" minOccurs="0" />
-                          </xsd:sequence>
+              </xsd:sequence>
-                          <xsd:attribute name="name" use="required" type="xsd:string" />
+              <xsd:attribute name="name" use="required" type="xsd:string" />
-                          <xsd:attribute name="type" type="xsd:string" />
+              <xsd:attribute name="type" type="xsd:string" />
-                          <xsd:attribute name="mimetype" type="xsd:string" />
+              <xsd:attribute name="mimetype" type="xsd:string" />
-                          <xsd:attribute ref="xml:space" />
+              <xsd:attribute ref="xml:space" />
-                      </xsd:complexType>
+            </xsd:complexType>
-                  </xsd:element>
+          </xsd:element>
-                  <xsd:element name="assembly">
+          <xsd:element name="assembly">
-                      <xsd:complexType>
+            <xsd:complexType>
-                          <xsd:attribute name="alias" type="xsd:string" />
+              <xsd:attribute name="alias" type="xsd:string" />
-                          <xsd:attribute name="name" type="xsd:string" />
+              <xsd:attribute name="name" type="xsd:string" />
-                      </xsd:complexType>
+            </xsd:complexType>
-                  </xsd:element>
+          </xsd:element>
-                  <xsd:element name="data">
+          <xsd:element name="data">
-                      <xsd:complexType>
+            <xsd:complexType>
-                          <xsd:sequence>
+              <xsd:sequence>
-                              <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
-                              <xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
+                <xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
-                          </xsd:sequence>
+              </xsd:sequence>
-                          <xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
+              <xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
-                          <xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
+              <xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
-                          <xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
+              <xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
-                          <xsd:attribute ref="xml:space" />
+              <xsd:attribute ref="xml:space" />
-                      </xsd:complexType>
+            </xsd:complexType>
-                  </xsd:element>
+          </xsd:element>
-                  <xsd:element name="resheader">
+          <xsd:element name="resheader">
-                      <xsd:complexType>
+            <xsd:complexType>
-                          <xsd:sequence>
+              <xsd:sequence>
-                              <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
-                          </xsd:sequence>
+              </xsd:sequence>
-                          <xsd:attribute name="name" type="xsd:string" use="required" />
+              <xsd:attribute name="name" type="xsd:string" use="required" />
-                      </xsd:complexType>
+            </xsd:complexType>
-                  </xsd:element>
+          </xsd:element>
-              </xsd:choice>
+        </xsd:choice>
-          </xsd:complexType>
+      </xsd:complexType>
-      </xsd:element>
+    </xsd:element>
   </xsd:schema>
   <resheader name="resmimetype">
-      <value>text/microsoft-resx</value>
+    <value>text/microsoft-resx</value>
   </resheader>
   <resheader name="version">
-      <value>2.0</value>
+    <value>2.0</value>
   </resheader>
   <resheader name="reader">
-      <value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+    <value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
   <resheader name="writer">
-      <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+    <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="InsecureConfiguration" xml:space="preserve">
+    <value>The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the CORS policy by listing individual origins if credentials needs to be supported.</value>
+  </data>
   <data name="PreflightMaxAgeOutOfRange" xml:space="preserve">
     <value>PreflightMaxAge must be greater than or equal to 0.</value>
   </data>

src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -285,7 +285,6 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             Assert.True(corsPolicy.SupportsCredentials);
         }
 
-
         [Fact]
         public void DisallowCredential_SetsSupportsCredentials_ToFalse()
         {
@@ -300,6 +299,21 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             Assert.False(corsPolicy.SupportsCredentials);
         }
 
+        [Fact]
+        public void Build_ThrowsIfConfiguredToAllowAnyOriginWithCredentials()
+        {
+            // Arrange
+            var builder = new CorsPolicyBuilder()
+                .AllowAnyOrigin()
+                .AllowCredentials();
+
+            // Act
+            var ex = Assert.Throws<InvalidOperationException>(() => builder.Build());
+
+            // Assert
+            Assert.Equal(Resources.InsecureConfiguration, ex.Message);
+        }
+
         [Theory]
         [InlineData("Some-String", "some-string")]
         [InlineData("x:\\Test", "x:\\test")]

src/Middleware/CORS/test/UnitTests/CorsServiceTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using Xunit;
@@ -11,6 +12,25 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
 {
     public class CorsServiceTests
     {
+        [Fact]
+        public void EvaluatePolicy_Throws_IfPolicyIsIncorrectlyConfigured()
+        {
+            // Arrange
+            var corsService = GetCorsService();
+            var requestContext = GetHttpContext("POST", origin: null);
+            var policy = new CorsPolicy
+            {
+                Origins = { "*" },
+                SupportsCredentials = true,
+            };
+
+            // Act & Assert
+            ExceptionAssert.ThrowsArgument(
+                () => corsService.EvaluatePolicy(requestContext, policy),
+                "policy",
+                Resources.InsecureConfiguration);
+        }
+
         [Fact]
         public void EvaluatePolicy_NoOrigin_ReturnsInvalidResult()
         {
@@ -103,10 +123,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             // Arrange
             var corsService = GetCorsService();
             var requestContext = GetHttpContext(origin: "http://example.com");
-            var policy = new CorsPolicy
+            var policy = new CorsPolicy();
-            {
-                SupportsCredentials = true
-            };
             policy.Origins.Add(CorsConstants.AnyOrigin);
 
             // Act
@@ -145,7 +162,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             {
                 SupportsCredentials = true
             };
-            policy.Origins.Add(CorsConstants.AnyOrigin);
+            policy.Origins.Add("http://example.com");
 
             // Act
             var result = corsService.EvaluatePolicy(requestContext, policy);
@@ -171,27 +188,6 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             Assert.False(result.VaryByOrigin);
         }
 
-        [Fact]
-        public void EvaluatePolicy_AllowAnyOrigin_SupportsCredentials_DoesNotVaryByOrigin()
-        {
-            // Arrange
-            var corsService = GetCorsService();
-            var requestContext = GetHttpContext(origin: "http://example.com");
-            var policy = new CorsPolicy
-            {
-                SupportsCredentials = true
-            };
-            policy.Origins.Add(CorsConstants.AnyOrigin);
-
-            // Act
-            var result = corsService.EvaluatePolicy(requestContext, policy);
-
-            // Assert
-            Assert.Equal("*", result.AllowedOrigin);
-            Assert.True(result.SupportsCredentials);
-            Assert.True(result.VaryByOrigin);
-        }
-
         [Fact]
         public void EvaluatePolicy_AllowOneOrigin_DoesNotVaryByOrigin()
         {
@@ -369,7 +365,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             {
                 SupportsCredentials = true
             };
-            policy.Origins.Add(CorsConstants.AnyOrigin);
+            policy.Origins.Add("http://example.com");
             policy.Methods.Add("*");
 
             // Act
@@ -532,7 +528,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure
             var corsService = GetCorsService();
             var httpContext = GetHttpContext(method: "OPTIONS", origin: "http://example.com", accessControlRequestMethod: "PUT");
             var policy = new CorsPolicy();
-            policy.Origins.Add(CorsConstants.AnyOrigin);
+            policy.Origins.Add("http://example.com");
             policy.Methods.Add("*");
             policy.Headers.Add("*");
             policy.SupportsCredentials = true;

src/Mvc/test/Mvc.FunctionalTests/CorsTestsBase.cs

@@ -157,7 +157,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
             var responseHeaders = response.Headers;
             Assert.Equal(
-                new[] { "*" },
+                new[] { "http://example.com" },
                 responseHeaders.GetValues(CorsConstants.AccessControlAllowOrigin).ToArray());
             Assert.Equal(
                new[] { "true" },
@@ -190,7 +190,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
             var responseHeaders = response.Headers;
             Assert.Equal(
-                new[] { "*" },
+                new[] { "http://example.com" },
                 responseHeaders.GetValues(CorsConstants.AccessControlAllowOrigin).ToArray());
             Assert.Equal(
                new[] { "true" },
@@ -302,9 +302,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
             Assert.Equal(
                 new[] { "*" },
                 responseHeaders.GetValues(CorsConstants.AccessControlAllowOrigin).ToArray());
-            Assert.Equal(
-               new[] { "true" },
-               responseHeaders.GetValues(CorsConstants.AccessControlAllowCredentials).ToArray());
             Assert.Equal(
                new[] { "Custom" },
                responseHeaders.GetValues(CorsConstants.AccessControlAllowHeaders).ToArray());

src/Mvc/test/WebSites/CorsWebSite/Controllers/BlogController.cs

@@ -29,7 +29,7 @@ namespace CorsWebSite
             return "exclusive";
         }
 
-        [EnableCors("WithCredentialsAnyOrigin")]
+        [EnableCors("WithCredentialsAndOtherSettings")]
         public string EditUserComment(int id, string userComment)
         {
             return userComment;

src/Mvc/test/WebSites/CorsWebSite/Startup.cs

@@ -41,11 +41,11 @@ namespace CorsWebSite
                     });
 
                 options.AddPolicy(
-                    "WithCredentialsAnyOrigin",
+                    "WithCredentialsAndOtherSettings",
                     builder =>
                     {
                         builder.AllowCredentials()
-                               .AllowAnyOrigin()
+                               .WithOrigins("http://example.com")
                                .AllowAnyHeader()
                                .WithMethods("PUT", "POST")
                                .WithExposedHeaders("exposed1", "exposed2");
@@ -55,8 +55,7 @@ namespace CorsWebSite
                     "AllowAll",
                     builder =>
                     {
-                        builder.AllowCredentials()
+                        builder.AllowAnyMethod()
-                               .AllowAnyMethod()
                                .AllowAnyHeader()
                                .AllowAnyOrigin();
                     });