From 85080ae621193f3bdd10eb83b1141b0809ea5e17 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Fri, 6 Nov 2015 09:45:58 -0800 Subject: [PATCH] Make Cors filters run before any other authorization filters --- .../CorsAuthorizationFilter.cs | 4 +- .../CorsAuthorizationFilterFactory.cs | 4 +- .../DisableCorsAuthorizationFilter.cs | 4 +- .../CorsTests.cs | 86 +++++++++++++++++++ .../Controllers/StoreController.cs | 46 ++++++++++ .../AllRequestsBlockingAuthorizationFilter.cs | 22 +++++ test/WebSites/CorsWebSite/Startup.cs | 20 +++++ 7 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 test/WebSites/CorsWebSite/Controllers/StoreController.cs create mode 100644 test/WebSites/CorsWebSite/Filters/AllRequestsBlockingAuthorizationFilter.cs diff --git a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilter.cs b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilter.cs index 52c1f4c9ad..038769d5ca 100644 --- a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilter.cs @@ -41,7 +41,9 @@ namespace Microsoft.AspNet.Mvc.Cors { get { - return int.MaxValue - 100; + // Since clients' preflight requests would not have data to authenticate requests, this + // filter must run before any other authorization filters. + return int.MinValue + 100; } } diff --git a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs index cd2224dc9f..5ebf36ca81 100644 --- a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs @@ -28,7 +28,9 @@ namespace Microsoft.AspNet.Mvc.Cors { get { - return int.MaxValue - 100; + // Since clients' preflight requests would not have data to authenticate requests, this + // filter must run before any other authorization filters. + return int.MinValue + 100; } } diff --git a/src/Microsoft.AspNet.Mvc.Cors/DisableCorsAuthorizationFilter.cs b/src/Microsoft.AspNet.Mvc.Cors/DisableCorsAuthorizationFilter.cs index fcf55ea402..acc48cf837 100644 --- a/src/Microsoft.AspNet.Mvc.Cors/DisableCorsAuthorizationFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Cors/DisableCorsAuthorizationFilter.cs @@ -20,7 +20,9 @@ namespace Microsoft.AspNet.Mvc.Cors { get { - return int.MaxValue - 100; + // Since clients' preflight requests would not have data to authenticate requests, this + // filter must run before any other authorization filters. + return int.MinValue + 100; } } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/CorsTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/CorsTests.cs index 4d45fa9164..db74c7d151 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/CorsTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/CorsTests.cs @@ -215,5 +215,91 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var content = await response.Content.ReadAsStringAsync(); Assert.Empty(content); } + + [Theory] + [InlineData("http://localhost/api/store/actionusingcontrollercorssettings")] + [InlineData("http://localhost/api/store/actionwithcorssettings")] + public async Task CorsFilter_RunsBeforeOtherAuthorizationFilters(string url) + { + // Arrange + var request = new HttpRequestMessage(new HttpMethod(CorsConstants.PreflightHttpMethod), url); + + // Adding a custom header makes it a non-simple request. + request.Headers.Add(CorsConstants.Origin, "http://example.com"); + request.Headers.Add(CorsConstants.AccessControlRequestMethod, "GET"); + request.Headers.Add(CorsConstants.AccessControlRequestHeaders, "Custom"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseHeaders = response.Headers; + Assert.Equal( + new[] { "http://example.com" }, + responseHeaders.GetValues(CorsConstants.AccessControlAllowOrigin).ToArray()); + Assert.Equal( + new[] { "true" }, + responseHeaders.GetValues(CorsConstants.AccessControlAllowCredentials).ToArray()); + Assert.Equal( + new[] { "Custom" }, + responseHeaders.GetValues(CorsConstants.AccessControlAllowHeaders).ToArray()); + + var content = await response.Content.ReadAsStringAsync(); + Assert.Empty(content); + } + + [Fact] + public async Task DisableCorsFilter_RunsBeforeOtherAuthorizationFilters() + { + // Controller has an authorization filter and Cors filter and the action has a DisableCors filter + // In this scenario, the CorsFilter should be executed before any other authorization filters + // i.e irrespective of where the Cors filter is applied(controller or action), Cors filters must + // always be executed before any other type of authorization filters. + + // Arrange + var request = new HttpRequestMessage( + new HttpMethod(CorsConstants.PreflightHttpMethod), + "http://localhost/api/store/actionwithcorsdisabled"); + + // Adding a custom header makes it a non-simple request. + request.Headers.Add(CorsConstants.Origin, "http://example.com"); + request.Headers.Add(CorsConstants.AccessControlRequestMethod, "GET"); + request.Headers.Add(CorsConstants.AccessControlRequestHeaders, "Custom"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Empty(response.Headers); + + // Nothing gets executed for a pre-flight request. + var content = await response.Content.ReadAsStringAsync(); + Assert.Empty(content); + } + + [Fact] + public async Task CorsFilter_OnAction_PreferredOverController_AndAuthorizationFiltersRunAfterCors() + { + // Arrange + var request = new HttpRequestMessage( + new HttpMethod(CorsConstants.PreflightHttpMethod), + "http://localhost/api/store/actionwithdifferentcorspolicy"); + request.Headers.Add(CorsConstants.Origin, "http://notexpecteddomain.com"); + request.Headers.Add(CorsConstants.AccessControlRequestMethod, "GET"); + request.Headers.Add(CorsConstants.AccessControlRequestHeaders, "Custom"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Empty(response.Headers); + + // Nothing gets executed for a pre-flight request. + var content = await response.Content.ReadAsStringAsync(); + Assert.Empty(content); + } } } \ No newline at end of file diff --git a/test/WebSites/CorsWebSite/Controllers/StoreController.cs b/test/WebSites/CorsWebSite/Controllers/StoreController.cs new file mode 100644 index 0000000000..ce25ba0bf0 --- /dev/null +++ b/test/WebSites/CorsWebSite/Controllers/StoreController.cs @@ -0,0 +1,46 @@ +// 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.Collections.Generic; +using Microsoft.AspNet.Cors; +using Microsoft.AspNet.Mvc; + +namespace CorsWebSite.Controllers +{ + [AllRequestsBlockingAuthorizationFilter] + [EnableCors("AllowAll")] + [Route("api/store/[action]")] + public class StoreController : Controller + { + [HttpGet] + public IEnumerable ActionUsingControllerCorsSettings() + { + return new string[] { "product1", "product2" }; + } + + [HttpGet] + [EnableCors("Allow example.com")] + public string ActionWithCorsSettings() + { + return "product1"; + } + + // Irrespective of where(controller or action) the Cors filter is applied, Cors filters should be + // executed before any other type of authorization filters. + [HttpGet] + [DisableCors] + public string ActionWithCorsDisabled() + { + return "product1"; + } + + // Irrespective of where(controller or action) the Cors filter is applied, Cors filters should be + // executed before any other type of authorization filters. + [HttpGet] + [EnableCors("Allow example.com")] + public string ActionWithDifferentCorsPolicy() + { + return "product1"; + } + } +} diff --git a/test/WebSites/CorsWebSite/Filters/AllRequestsBlockingAuthorizationFilter.cs b/test/WebSites/CorsWebSite/Filters/AllRequestsBlockingAuthorizationFilter.cs new file mode 100644 index 0000000000..0d6c6b5b0f --- /dev/null +++ b/test/WebSites/CorsWebSite/Filters/AllRequestsBlockingAuthorizationFilter.cs @@ -0,0 +1,22 @@ +// 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; +using Microsoft.AspNet.Mvc; +using Microsoft.AspNet.Mvc.Filters; + +namespace CorsWebSite +{ + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + public class AllRequestsBlockingAuthorizationFilter : Attribute, IAuthorizationFilter + { + public void OnAuthorization(AuthorizationContext context) + { + context.Result = new ContentResult() + { + Content = "You are unauthorized!!", + StatusCode = 401 + }; + } + } +} diff --git a/test/WebSites/CorsWebSite/Startup.cs b/test/WebSites/CorsWebSite/Startup.cs index 8fb686f5da..acc91855e1 100644 --- a/test/WebSites/CorsWebSite/Startup.cs +++ b/test/WebSites/CorsWebSite/Startup.cs @@ -47,6 +47,26 @@ namespace CorsWebSite .WithMethods("PUT", "POST") .WithExposedHeaders("exposed1", "exposed2"); }); + + options.AddPolicy( + "AllowAll", + builder => + { + builder.AllowCredentials() + .AllowAnyMethod() + .AllowAnyHeader() + .AllowAnyOrigin(); + }); + + options.AddPolicy( + "Allow example.com", + builder => + { + builder.AllowCredentials() + .AllowAnyMethod() + .AllowAnyHeader() + .WithOrigins("http://example.com"); + }); }); }