diff --git a/src/Middleware/CORS/samples/SampleDestination/Startup.cs b/src/Middleware/CORS/samples/SampleDestination/Startup.cs index 740346cfdeae..52e19629cb3b 100644 --- a/src/Middleware/CORS/samples/SampleDestination/Startup.cs +++ b/src/Middleware/CORS/samples/SampleDestination/Startup.cs @@ -50,8 +50,7 @@ public void ConfigureServices(IServiceCollection services) options.AddPolicy("AllowAll", policy => policy .AllowAnyOrigin() .AllowAnyMethod() - .AllowAnyHeader() - .AllowCredentials()); + .AllowAnyHeader()); }); services.AddRouting(); } diff --git a/src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs b/src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs index 590d26c0f8da..d4a95f40b4b0 100644 --- a/src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs +++ b/src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs @@ -73,8 +73,7 @@ public void Configure(IApplicationBuilder app) innerBuilder.UseCors(policy => policy .AllowAnyOrigin() .AllowAnyMethod() - .AllowAnyHeader() - .AllowCredentials()); + .AllowAnyHeader()); innerBuilder.UseMiddleware(); }); diff --git a/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs b/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs index 145571ab607c..667c79d733b8 100644 --- a/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs +++ b/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs @@ -224,6 +224,11 @@ public CorsPolicyBuilder SetIsOriginAllowedToAllowWildcardSubdomains() /// The constructed . public CorsPolicy Build() { + if (_policy.AllowAnyOrigin && _policy.SupportsCredentials) + { + throw new InvalidOperationException(Resources.InsecureConfiguration); + } + return _policy; } diff --git a/src/Middleware/CORS/src/Infrastructure/CorsService.cs b/src/Middleware/CORS/src/Infrastructure/CorsService.cs index bf8d03e35937..a240ae22c993 100644 --- a/src/Middleware/CORS/src/Infrastructure/CorsService.cs +++ b/src/Middleware/CORS/src/Infrastructure/CorsService.cs @@ -8,7 +8,6 @@ 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 @@ public CorsResult EvaluatePolicy(HttpContext context, CorsPolicy policy) if (policy.AllowAnyOrigin && policy.SupportsCredentials) { - _logger.InsecureConfiguration(); + throw new ArgumentException(Resources.InsecureConfiguration, nameof(policy)); } var origin = context.Request.Headers[CorsConstants.Origin]; diff --git a/src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs b/src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs index 815bbfc2a2cb..58886c37c98d 100644 --- a/src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs +++ b/src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs @@ -18,7 +18,6 @@ internal static class CORSLoggerExtensions private static readonly Action _requestHeaderNotAllowed; private static readonly Action _failedToSetCorsHeaders; private static readonly Action _noCorsPolicyFound; - private static readonly Action _insecureConfiguration; private static readonly Action _isNotPreflightRequest; static CORSLoggerExtensions() @@ -73,11 +72,6 @@ static CORSLoggerExtensions() 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 @@ public static void NoCorsPolicyFound(this ILogger logger) _noCorsPolicyFound(logger, null); } - public static void InsecureConfiguration(this ILogger logger) - { - _insecureConfiguration(logger, null); - } - public static void IsNotPreflightRequest(this ILogger logger) { _isNotPreflightRequest(logger, null); diff --git a/src/Middleware/CORS/src/Properties/Resources.Designer.cs b/src/Middleware/CORS/src/Properties/Resources.Designer.cs new file mode 100644 index 000000000000..e91bc3b6898b --- /dev/null +++ b/src/Middleware/CORS/src/Properties/Resources.Designer.cs @@ -0,0 +1,58 @@ +// +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); + + /// + /// 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. + /// + internal static string InsecureConfiguration + { + get => GetString("InsecureConfiguration"); + } + + /// + /// 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. + /// + internal static string FormatInsecureConfiguration() + => GetString("InsecureConfiguration"); + + /// + /// PreflightMaxAge must be greater than or equal to 0. + /// + internal static string PreflightMaxAgeOutOfRange + { + get => GetString("PreflightMaxAgeOutOfRange"); + } + + /// + /// PreflightMaxAge must be greater than or equal to 0. + /// + 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; + } + } +} diff --git a/src/Middleware/CORS/src/Resources.Designer.cs b/src/Middleware/CORS/src/Resources.Designer.cs deleted file mode 100644 index 0bc258074c94..000000000000 --- a/src/Middleware/CORS/src/Resources.Designer.cs +++ /dev/null @@ -1,71 +0,0 @@ -//------------------------------------------------------------------------------ -// -// 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. -// -//------------------------------------------------------------------------------ - -namespace Microsoft.AspNetCore.Cors { - using System; - using System.Reflection; - - - /// - /// A strongly-typed resource class, for looking up localized strings, etc. - /// - // 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() { - } - - /// - /// Returns the cached ResourceManager instance used by this class. - /// - [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; - } - } - - /// - /// Overrides the current thread's CurrentUICulture property for all - /// resource lookups using this strongly typed resource class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Globalization.CultureInfo Culture { - get { - return resourceCulture; - } - set { - resourceCulture = value; - } - } - - /// - /// Looks up a localized string similar to PreflightMaxAge must be greater than or equal to 0.. - /// - internal static string PreflightMaxAgeOutOfRange { - get { - return ResourceManager.GetString("PreflightMaxAgeOutOfRange", resourceCulture); - } - } - } -} diff --git a/src/Middleware/CORS/src/Resources.resx b/src/Middleware/CORS/src/Resources.resx index 6b9ebaad31c4..f12e1ddeb141 100644 --- a/src/Middleware/CORS/src/Resources.resx +++ b/src/Middleware/CORS/src/Resources.resx @@ -1,6 +1,6 @@  - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - text/microsoft-resx + text/microsoft-resx - 2.0 + 2.0 - System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + 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. + PreflightMaxAge must be greater than or equal to 0. diff --git a/src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs b/src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs index b7508c4610da..f8d2e22fd444 100644 --- a/src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs +++ b/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 @@ public void AllowCredential_SetsSupportsCredentials_ToTrue() Assert.True(corsPolicy.SupportsCredentials); } - [Fact] public void DisallowCredential_SetsSupportsCredentials_ToFalse() { @@ -300,6 +299,21 @@ public void DisallowCredential_SetsSupportsCredentials_ToFalse() Assert.False(corsPolicy.SupportsCredentials); } + [Fact] + public void Build_ThrowsIfConfiguredToAllowAnyOriginWithCredentials() + { + // Arrange + var builder = new CorsPolicyBuilder() + .AllowAnyOrigin() + .AllowCredentials(); + + // Act + var ex = Assert.Throws(() => builder.Build()); + + // Assert + Assert.Equal(Resources.InsecureConfiguration, ex.Message); + } + [Theory] [InlineData("Some-String", "some-string")] [InlineData("x:\\Test", "x:\\test")] diff --git a/src/Middleware/CORS/test/UnitTests/CorsServiceTests.cs b/src/Middleware/CORS/test/UnitTests/CorsServiceTests.cs index a2ab09d2a034..e9bf95531365 100644 --- a/src/Middleware/CORS/test/UnitTests/CorsServiceTests.cs +++ b/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 @@ public void EvaluatePolicy_AllowAnyOrigin_AddsAnyOrigin() // Arrange var corsService = GetCorsService(); var requestContext = GetHttpContext(origin: "http://example.com"); - var policy = new CorsPolicy - { - SupportsCredentials = true - }; + var policy = new CorsPolicy(); policy.Origins.Add(CorsConstants.AnyOrigin); // Act @@ -145,7 +162,7 @@ public void EvaluatePolicy_SupportsCredentials_AllowCredentialsReturnsTrue() { SupportsCredentials = true }; - policy.Origins.Add(CorsConstants.AnyOrigin); + policy.Origins.Add("http://example.com"); // Act var result = corsService.EvaluatePolicy(requestContext, policy); @@ -171,27 +188,6 @@ public void EvaluatePolicy_AllowAnyOrigin_DoesNotSupportsCredentials_DoesNotVary 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 @@ public void EvaluatePolicy_PreflightRequest_SupportsCredentials_AllowCredentials { SupportsCredentials = true }; - policy.Origins.Add(CorsConstants.AnyOrigin); + policy.Origins.Add("http://example.com"); policy.Methods.Add("*"); // Act @@ -532,7 +528,7 @@ public void EvaluatePolicy_PreflightRequest_WithCredentials_ReflectsHeaders() 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; diff --git a/src/Mvc/test/Mvc.FunctionalTests/CorsTestsBase.cs b/src/Mvc/test/Mvc.FunctionalTests/CorsTestsBase.cs index e04b7cc3e12d..91ccaf578d31 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/CorsTestsBase.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/CorsTestsBase.cs @@ -157,7 +157,7 @@ public async Task SuccessfulCorsRequest_AllowsCredentials_IfThePolicyAllowsCrede 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 @@ public async Task SuccessfulPreflightRequest_AllowsCredentials_IfThePolicyAllows 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 @@ public async Task CorsFilter_RunsBeforeOtherAuthorizationFilters_UsesPolicySpeci 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()); diff --git a/src/Mvc/test/WebSites/CorsWebSite/Controllers/BlogController.cs b/src/Mvc/test/WebSites/CorsWebSite/Controllers/BlogController.cs index cfab955ec1a0..96aa4f77f034 100644 --- a/src/Mvc/test/WebSites/CorsWebSite/Controllers/BlogController.cs +++ b/src/Mvc/test/WebSites/CorsWebSite/Controllers/BlogController.cs @@ -29,7 +29,7 @@ public string GetExclusiveContent() return "exclusive"; } - [EnableCors("WithCredentialsAnyOrigin")] + [EnableCors("WithCredentialsAndOtherSettings")] public string EditUserComment(int id, string userComment) { return userComment; diff --git a/src/Mvc/test/WebSites/CorsWebSite/Startup.cs b/src/Mvc/test/WebSites/CorsWebSite/Startup.cs index 62eca5374f48..6f89c6aa4d43 100644 --- a/src/Mvc/test/WebSites/CorsWebSite/Startup.cs +++ b/src/Mvc/test/WebSites/CorsWebSite/Startup.cs @@ -41,11 +41,11 @@ public void ConfigureServices(IServiceCollection services) }); options.AddPolicy( - "WithCredentialsAnyOrigin", + "WithCredentialsAndOtherSettings", builder => { builder.AllowCredentials() - .AllowAnyOrigin() + .WithOrigins("http://example.com") .AllowAnyHeader() .WithMethods("PUT", "POST") .WithExposedHeaders("exposed1", "exposed2"); @@ -55,8 +55,7 @@ public void ConfigureServices(IServiceCollection services) "AllowAll", builder => { - builder.AllowCredentials() - .AllowAnyMethod() + builder.AllowAnyMethod() .AllowAnyHeader() .AllowAnyOrigin(); });