From b3bdba15591145856925a19a7f2d9c6c57891acf Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Mon, 21 Nov 2016 15:31:14 -0800 Subject: [PATCH] PR comments incorporated. Working on making SampleDestination dynamic- define the policy within the sample through a UI. --- CORS.sln | 4 +- samples/README.md | 8 +- samples/SampleDestination/Program.cs | 1 - samples/SampleDestination/Startup.cs | 16 +- samples/SampleDestination/project.json | 12 +- samples/SampleDestination/web.config | 14 -- samples/SampleOrigin/Program.cs | 3 +- samples/SampleOrigin/project.json | 12 +- samples/SampleOrigin/web.config | 11 +- samples/SampleOrigin/wwwroot/Index.html | 94 +++++++++--- .../Infrastructure/CorsService.cs | 35 +++-- .../Internal/CORSLoggerExtensions.cs | 55 +++++-- .../CorsServiceTests.cs | 143 +++++++++++------- 13 files changed, 253 insertions(+), 155 deletions(-) delete mode 100644 samples/SampleDestination/web.config diff --git a/CORS.sln b/CORS.sln index 87e766660f..c471720431 100644 --- a/CORS.sln +++ b/CORS.sln @@ -21,9 +21,9 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "CorsMiddlewareWebSite", "te EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{960E0703-A8A5-44DF-AA87-B7C614683B3C}" EndProject -Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SampleDestination", "SampleDestination\SampleDestination.xproj", "{F6675DC1-AA21-453B-89B6-DA425FB9C3A5}" +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SampleDestination", "samples\SampleDestination\SampleDestination.xproj", "{F6675DC1-AA21-453B-89B6-DA425FB9C3A5}" EndProject -Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SampleOrigin", "SampleOrigin\SampleOrigin.xproj", "{99460370-AE5D-4DC9-8DBF-04DF66D6B21D}" +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SampleOrigin", "samples\SampleOrigin\SampleOrigin.xproj", "{99460370-AE5D-4DC9-8DBF-04DF66D6B21D}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/samples/README.md b/samples/README.md index c0b4a4414e..ce871c4940 100644 --- a/samples/README.md +++ b/samples/README.md @@ -19,9 +19,15 @@ Save the file and close it. Then clear your browser history. Run the sample *In a command prompt window, open the directory where you cloned the repository, and open the SampleDestination directory. Run the command: dotnet run *Repeat the above step in the SampleOrigin directory. -*Open a browser window and go to http://origin.example.com +*Open a browser window and go to http://origin.example.com:5001 *Click the button to see CORS in action. +The SampleOrigin application will use port 5001, and SampleDestination will use 5000. Please ensure there are no other processes using those ports before running the CORS sample. + +As an example, apart from GET, HEAD and POST requests, PUT requests are allowed in the CORS policy on SampleDestination. Any others, like DELETE, OPTIONS etc. are not allowed and throw an error. +Content-Length has been added as an allowed header to the sample. Any other headers are not allowed and throw an error. +To edit the policy, please see app.UseCors() method in the Startup.cs file of SampleDestination. + If using Visual Studio to launch the request origin: Open Visual Studio and in the launchSettings.json file for the SampleOrigin project, change the launchUrl under SampleOrigin to http://origin.example.com:8080. Using the dropdown near the Start button, choose SampleOrigin before pressing Start to ensure that it uses Kestrel diff --git a/samples/SampleDestination/Program.cs b/samples/SampleDestination/Program.cs index d964f308f5..c196d5b838 100644 --- a/samples/SampleDestination/Program.cs +++ b/samples/SampleDestination/Program.cs @@ -14,7 +14,6 @@ namespace SampleDestination .UseKestrel() .UseUrls("http://*:5000") .UseContentRoot(Directory.GetCurrentDirectory()) - .UseIISIntegration() .UseStartup() .Build(); diff --git a/samples/SampleDestination/Startup.cs b/samples/SampleDestination/Startup.cs index 1055b9f224..199bb16795 100644 --- a/samples/SampleDestination/Startup.cs +++ b/samples/SampleDestination/Startup.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -19,10 +20,21 @@ namespace SampleDestination public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory) { loggerFactory.AddConsole(); - app.UseCors(policy => policy.WithOrigins("http://origin.example.com:8080")); + + app.UseCors(policy => policy + .WithOrigins("http://origin.example.com:5001") + .WithMethods("PUT") + .WithHeaders("Content-Length")); + app.Run(async context => { - await context.Response.WriteAsync("Status code of your request: " + context.Response.StatusCode.ToString()); + var responseHeaders = context.Response.Headers; + foreach (var responseHeader in responseHeaders) + { + await context.Response.WriteAsync("\n"+responseHeader.Key+": "+responseHeader.Value); + } + + await context.Response.WriteAsync("\nStatus code of your request: " + context.Response.StatusCode.ToString()); }); } } diff --git a/samples/SampleDestination/project.json b/samples/SampleDestination/project.json index 4e96e2c773..3c86825eb6 100644 --- a/samples/SampleDestination/project.json +++ b/samples/SampleDestination/project.json @@ -4,16 +4,11 @@ "version": "1.1.0-*", "type": "platform" }, - "Microsoft.AspNetCore.Server.IISIntegration": "1.2.0-*", "Microsoft.AspNetCore.Server.Kestrel": "1.2.0-*", "Microsoft.Extensions.Logging.Console": "1.2.0-*", "Microsoft.AspNetCore.Cors": "1.2.0-*" }, - "tools": { - "Microsoft.AspNetCore.Server.IISIntegration.Tools": "1.0.0-*" - }, - "frameworks": { "netcoreapp1.0": { "imports": [ @@ -36,12 +31,7 @@ "publishOptions": { "include": [ - "wwwroot", - "web.config" + "wwwroot" ] - }, - - "scripts": { - "postpublish": [ "dotnet publish-iis --publish-folder %publish:OutputPath% --framework %publish:FullTargetFramework%" ] } } diff --git a/samples/SampleDestination/web.config b/samples/SampleDestination/web.config deleted file mode 100644 index dc0514fca5..0000000000 --- a/samples/SampleDestination/web.config +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - - - diff --git a/samples/SampleOrigin/Program.cs b/samples/SampleOrigin/Program.cs index 3a51bfdf8b..1a6960a91f 100644 --- a/samples/SampleOrigin/Program.cs +++ b/samples/SampleOrigin/Program.cs @@ -12,9 +12,8 @@ namespace SampleOrigin { var host = new WebHostBuilder() .UseKestrel() - .UseUrls("http://*:8080") + .UseUrls("http://*:5001") .UseContentRoot(Directory.GetCurrentDirectory()) - .UseIISIntegration() .UseStartup() .Build(); diff --git a/samples/SampleOrigin/project.json b/samples/SampleOrigin/project.json index b4165727fd..259fb617c3 100644 --- a/samples/SampleOrigin/project.json +++ b/samples/SampleOrigin/project.json @@ -4,15 +4,10 @@ "version": "1.1.0-*", "type": "platform" }, - "Microsoft.AspNetCore.Server.IISIntegration": "1.2.0-*", "Microsoft.AspNetCore.Server.Kestrel": "1.2.0-*", "Microsoft.Extensions.Logging.Console": "1.2.0-*" }, - "tools": { - "Microsoft.AspNetCore.Server.IISIntegration.Tools": "1.0.0-*" - }, - "frameworks": { "netcoreapp1.0": { "imports": [ @@ -35,12 +30,7 @@ "publishOptions": { "include": [ - "wwwroot", - "web.config" + "wwwroot" ] - }, - - "scripts": { - "postpublish": [ "dotnet publish-iis --publish-folder %publish:OutputPath% --framework %publish:FullTargetFramework%" ] } } diff --git a/samples/SampleOrigin/web.config b/samples/SampleOrigin/web.config index dc0514fca5..e04a0397bf 100644 --- a/samples/SampleOrigin/web.config +++ b/samples/SampleOrigin/web.config @@ -1,14 +1,9 @@  - - - - + - + - + \ No newline at end of file diff --git a/samples/SampleOrigin/wwwroot/Index.html b/samples/SampleOrigin/wwwroot/Index.html index d9d8b74672..dd2c4f0ae9 100644 --- a/samples/SampleOrigin/wwwroot/Index.html +++ b/samples/SampleOrigin/wwwroot/Index.html @@ -2,30 +2,56 @@ - - - - -
- -
-
- -
+

CORS Sample

+ Method:

+ Header Name: Header Value:

+ + +



+ + Method DELETE is not allowed: + Method PUT is allowed:

+ + Header 'Max-Forwards' not supported: + Header 'Content-Length' is supported:

\ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsService.cs b/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsService.cs index c8a8e54d4e..32aea828c5 100644 --- a/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsService.cs +++ b/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsService.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure /// /// The option model representing . public CorsService(IOptions options) - :this(options, loggerFactory: null) + : this(options, loggerFactory: null) { } @@ -103,14 +103,14 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure return; } + _logger?.RequestHasOriginHeader(origin); if (!policy.AllowAnyOrigin && !policy.Origins.Contains(origin)) { - _logger?.RequestHasOriginHeader(); - _logger?.PolicyFailure($"Request origin {origin} does not have permission to access the resource."); + _logger?.PolicyFailure(); + _logger?.OriginNotAllowed(origin); return; } - _logger?.RequestHasOriginHeader(); AddOriginToResult(origin, policy, result); result.SupportsCredentials = policy.SupportsCredentials; AddHeaderValues(result.AllowedExposedHeaders, policy.ExposedHeaders); @@ -126,14 +126,14 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure return; } + _logger?.RequestHasOriginHeader(origin); if (!policy.AllowAnyOrigin && !policy.Origins.Contains(origin)) { - _logger?.RequestHasOriginHeader(); - _logger?.PolicyFailure($"Request origin {origin} does not have permission to access the resource."); + _logger?.PolicyFailure(); + _logger?.OriginNotAllowed(origin); return; } - _logger?.RequestHasOriginHeader(); var accessControlRequestMethod = context.Request.Headers[CorsConstants.AccessControlRequestMethod]; if (StringValues.IsNullOrEmpty(accessControlRequestMethod)) { @@ -158,18 +158,25 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure if (!found) { - _logger?.PolicyFailure($"Request method {accessControlRequestMethod} not allowed in CORS policy."); - return; + _logger?.PolicyFailure(); + _logger?.AccessControlMethodNotAllowed(accessControlRequestMethod); + return; } } if (!policy.AllowAnyHeader && - requestHeaders != null && - !requestHeaders.All(header => CorsConstants.SimpleRequestHeaders.Contains(header, StringComparer.OrdinalIgnoreCase) || - policy.Headers.Contains(header, StringComparer.OrdinalIgnoreCase))) + requestHeaders != null) { - _logger?.PolicyFailure($"One or more request header(s) not allowed in CORS policy."); - return; + foreach (var requestHeader in requestHeaders) + { + if (!CorsConstants.SimpleRequestHeaders.Contains(requestHeader, StringComparer.OrdinalIgnoreCase) && + !policy.Headers.Contains(requestHeader, StringComparer.OrdinalIgnoreCase)) + { + _logger?.PolicyFailure(); + _logger?.RequestHeaderNotAllowed(requestHeader); + return; + } + } } AddOriginToResult(origin, policy, result); diff --git a/src/Microsoft.AspNetCore.Cors/Internal/CORSLoggerExtensions.cs b/src/Microsoft.AspNetCore.Cors/Internal/CORSLoggerExtensions.cs index e744e95565..727d19a4ea 100644 --- a/src/Microsoft.AspNetCore.Cors/Internal/CORSLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Cors/Internal/CORSLoggerExtensions.cs @@ -9,22 +9,25 @@ namespace Microsoft.AspNetCore.Cors.Internal internal static class CORSLoggerExtensions { private static readonly Action _isPreflightRequest; - private static readonly Action _requestHasOriginHeader; + private static readonly Action _requestHasOriginHeader; private static readonly Action _requestDoesNotHaveOriginHeader; private static readonly Action _policySuccess; - private static readonly Action _policyFailure; + private static readonly Action _policyFailure; + private static readonly Action _originNotAllowed; + private static readonly Action _accessControlMethodNotAllowed; + private static readonly Action _requestHeaderNotAllowed; static CORSLoggerExtensions() { _isPreflightRequest = LoggerMessage.Define( LogLevel.Debug, 1, - "This is a preflight request."); + "The request is a preflight request."); - _requestHasOriginHeader = LoggerMessage.Define( + _requestHasOriginHeader = LoggerMessage.Define( LogLevel.Debug, 2, - "The request has an origin header."); + "The request has an origin header: '{origin}'."); _requestDoesNotHaveOriginHeader = LoggerMessage.Define( LogLevel.Debug, @@ -36,10 +39,25 @@ namespace Microsoft.AspNetCore.Cors.Internal 4, "Policy execution successful."); - _policyFailure = LoggerMessage.Define( + _policyFailure = LoggerMessage.Define( LogLevel.Information, 5, - "Policy execution failed. {FailureReason}"); + "Policy execution failed."); + + _originNotAllowed = LoggerMessage.Define( + LogLevel.Information, + 6, + "Request origin {origin} does not have permission to access the resource."); + + _accessControlMethodNotAllowed = LoggerMessage.Define( + LogLevel.Information, + 7, + "Request method {accessControlRequestMethod} not allowed in CORS policy."); + + _requestHeaderNotAllowed = LoggerMessage.Define( + LogLevel.Information, + 8, + "Request header '{requestHeader}' not allowed in CORS policy."); } public static void IsPreflightRequest(this ILogger logger) @@ -47,9 +65,9 @@ namespace Microsoft.AspNetCore.Cors.Internal _isPreflightRequest(logger, null); } - public static void RequestHasOriginHeader(this ILogger logger) + public static void RequestHasOriginHeader(this ILogger logger, string origin) { - _requestHasOriginHeader(logger, null); + _requestHasOriginHeader(logger, origin, null); } public static void RequestDoesNotHaveOriginHeader(this ILogger logger) @@ -62,9 +80,24 @@ namespace Microsoft.AspNetCore.Cors.Internal _policySuccess(logger, null); } - public static void PolicyFailure(this ILogger logger, string failureReason) + public static void PolicyFailure(this ILogger logger) { - _policyFailure(logger, failureReason, null); + _policyFailure(logger, null); + } + + public static void OriginNotAllowed(this ILogger logger, string origin) + { + _originNotAllowed(logger, origin, null); + } + + public static void AccessControlMethodNotAllowed(this ILogger logger, string accessControlMethod) + { + _accessControlMethodNotAllowed(logger, accessControlMethod, null); + } + + public static void RequestHeaderNotAllowed(this ILogger logger, string requestHeader) + { + _requestHeaderNotAllowed(logger, requestHeader, null); } } } diff --git a/test/Microsoft.AspNetCore.Cors.Test/CorsServiceTests.cs b/test/Microsoft.AspNetCore.Cors.Test/CorsServiceTests.cs index d20adfaa4b..b3ce5927d8 100644 --- a/test/Microsoft.AspNetCore.Cors.Test/CorsServiceTests.cs +++ b/test/Microsoft.AspNetCore.Cors.Test/CorsServiceTests.cs @@ -228,39 +228,41 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure Assert.Contains("PUT", result.AllowedMethods); } - public static TheoryData PreflightRequests_LoggingData + public static TheoryData PreflightRequests_LoggingData { get { - return new TheoryData + return new TheoryData { { - "http://example.com", - "PUT", - null, - "The request has an origin header.", - "Policy execution failed. Request origin http://example.com does not have permission to access the resource." + new LogData { + origin = "http://example.com", + method = "PUT", + headers = null, + originLogMessage = "The request has an origin header: 'http://example.com'.", + policyLogMessage = "Policy execution failed.", + failureReason = "Request origin http://example.com does not have permission to access the resource." + } }, { - "http://allowed.example.com", - "DELETE", - null, - "The request has an origin header.", - "Policy execution failed. Request method DELETE not allowed in CORS policy." + new LogData { + origin = "http://allowed.example.com", + method = "DELETE", + headers = null, + originLogMessage = "The request has an origin header: 'http://allowed.example.com'.", + policyLogMessage = "Policy execution failed.", + failureReason = "Request method DELETE not allowed in CORS policy." + } }, { - "http://allowed.example.com", - "PUT", - new[] { "test" }, - "The request has an origin header.", - "Policy execution failed. One or more request header(s) not allowed in CORS policy." - }, - { - "http://allowed.example.com", - "PUT", - null, - "The request has an origin header.", - "Policy execution successful." + new LogData { + origin = "http://allowed.example.com", + method = "PUT", + headers = new[] { "test" }, + originLogMessage = "The request has an origin header: 'http://allowed.example.com'.", + policyLogMessage = "Policy execution failed.", + failureReason = "Request header 'test' not allowed in CORS policy." + } }, }; } @@ -268,13 +270,13 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure [Theory] [MemberData(nameof(PreflightRequests_LoggingData))] - public void EvaluatePolicy_LoggingForPreflightRequests_HasOriginHeader(string origin, string method, string[] headers, string originLogMessage, string policyLogMessage) + public void EvaluatePolicy_LoggingForPreflightRequests_HasOriginHeader_PolicyFailed(LogData logData) { var sink = new TestSink(); var loggerFactory = new TestLoggerFactory(sink, enabled: true); var corsService = new CorsService(new TestCorsOptions(), loggerFactory); - var requestContext = GetHttpContext(method: "OPTIONS", origin: origin, accessControlRequestMethod: method, accessControlRequestHeaders: headers); + var requestContext = GetHttpContext(method: "OPTIONS", origin: logData.origin, accessControlRequestMethod: logData.method, accessControlRequestHeaders: logData.headers); var policy = new CorsPolicy(); policy.Origins.Add("http://allowed.example.com"); policy.Methods.Add("PUT"); @@ -282,9 +284,30 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure // Act var result = corsService.EvaluatePolicy(requestContext, policy); - Assert.Equal("This is a preflight request.", sink.Writes[0].State.ToString()); - Assert.Equal(originLogMessage, sink.Writes[1].State.ToString()); - Assert.Equal(policyLogMessage, sink.Writes[2].State.ToString()); + Assert.Equal("The request is a preflight request.", sink.Writes[0].State.ToString()); + Assert.Equal(logData.originLogMessage, sink.Writes[1].State.ToString()); + Assert.Equal(logData.policyLogMessage, sink.Writes[2].State.ToString()); + Assert.Equal(logData.failureReason, sink.Writes[3].State.ToString()); + } + + [Fact] + public void EvaluatePolicy_LoggingForPreflightRequests_HasOriginHeader_PolicySucceeded() + { + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + + var corsService = new CorsService(new TestCorsOptions(), loggerFactory); + var requestContext = GetHttpContext(method: "OPTIONS", origin: "http://allowed.example.com", accessControlRequestMethod: "PUT"); + var policy = new CorsPolicy(); + policy.Origins.Add("http://allowed.example.com"); + policy.Methods.Add("PUT"); + + // Act + var result = corsService.EvaluatePolicy(requestContext, policy); + + Assert.Equal("The request is a preflight request.", sink.Writes[0].State.ToString()); + Assert.Equal("The request has an origin header: 'http://allowed.example.com'.", sink.Writes[1].State.ToString()); + Assert.Equal("Policy execution successful.", sink.Writes[2].State.ToString()); } [Fact] @@ -302,47 +325,45 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure // Act var result = corsService.EvaluatePolicy(requestContext, policy); - Assert.Equal("This is a preflight request.", sink.Writes[0].State.ToString()); + Assert.Equal("The request is a preflight request.", sink.Writes[0].State.ToString()); Assert.Equal("The request does not have an origin header.", sink.Writes[1].State.ToString()); } - public static TheoryData NonPreflightRequests_LoggingData - { - get - { - return new TheoryData - { - { - "http://example.com", - "The request has an origin header.", - "Policy execution failed. Request origin http://example.com does not have permission to access the resource." - }, - { - "http://allowed.example.com", - "The request has an origin header.", - "Policy execution successful." - } - }; - } - } - - [Theory] - [MemberData(nameof(NonPreflightRequests_LoggingData))] - public void EvaluatePolicy_LoggingForNonPreflightRequests_HasOriginHeader(string origin, string originlogMessage, string policyLogMessage) + [Fact] + public void EvaluatePolicy_LoggingForNonPreflightRequests_HasOriginHeader_PolicyFailed() { var sink = new TestSink(); var loggerFactory = new TestLoggerFactory(sink, enabled: true); var corsService = new CorsService(new TestCorsOptions(), loggerFactory); - var requestContext = GetHttpContext(origin: origin); + var requestContext = GetHttpContext(origin: "http://example.com"); var policy = new CorsPolicy(); policy.Origins.Add("http://allowed.example.com"); // Act var result = corsService.EvaluatePolicy(requestContext, policy); - Assert.Equal(originlogMessage, sink.Writes[0].State.ToString()); - Assert.Equal(policyLogMessage, sink.Writes[1].State.ToString()); + Assert.Equal("The request has an origin header: 'http://example.com'.", sink.Writes[0].State.ToString()); + Assert.Equal("Policy execution failed.", sink.Writes[1].State.ToString()); + Assert.Equal("Request origin http://example.com does not have permission to access the resource.", sink.Writes[2].State.ToString()); + } + + [Fact] + public void EvaluatePolicy_LoggingForNonPreflightRequests_HasOriginHeader_PolicySucceeded() + { + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + + var corsService = new CorsService(new TestCorsOptions(), loggerFactory); + var requestContext = GetHttpContext(origin: "http://allowed.example.com"); + var policy = new CorsPolicy(); + policy.Origins.Add("http://allowed.example.com"); + + // Act + var result = corsService.EvaluatePolicy(requestContext, policy); + + Assert.Equal("The request has an origin header: 'http://allowed.example.com'.", sink.Writes[0].State.ToString()); + Assert.Equal("Policy execution successful.", sink.Writes[1].State.ToString()); } [Fact] @@ -1049,5 +1070,15 @@ namespace Microsoft.AspNetCore.Cors.Infrastructure return context; } + + public struct LogData + { + public string origin { get; set; } + public string method { get; set; } + public string[] headers { get; set; } + public string originLogMessage { get; set; } + public string policyLogMessage { get; set; } + public string failureReason { get; set; } + } } } \ No newline at end of file