From f02f55c33d09206e75547061d526e1e99a3825c8 Mon Sep 17 00:00:00 2001 From: Ryan Brandenburg Date: Fri, 28 Oct 2016 16:21:50 -0700 Subject: [PATCH] Antiforgery goes at the end of filters --- Mvc.sln | 15 +++++ .../AutoValidateAntiforgeryTokenAttribute.cs | 19 +++++- .../ValidateAntiForgeryTokenAttribute.cs | 19 +++++- .../AntiforgeryAuthTests.cs | 45 +++++++++++++ .../project.json | 1 + .../Controllers/HomeController.cs | 41 ++++++++++++ test/WebSites/SecurityWebSite/Program.cs | 22 +++++++ .../SecurityWebSite/SecurityWebSite.xproj | 23 +++++++ test/WebSites/SecurityWebSite/Startup.cs | 63 +++++++++++++++++++ .../SecurityWebSite/Views/Home/Index.cshtml | 21 +++++++ .../Views/Shared/_Layout.cshtml | 37 +++++++++++ .../SecurityWebSite/Views/_ViewImports.cshtml | 2 + .../SecurityWebSite/Views/_ViewStart.cshtml | 3 + .../WebSites/SecurityWebSite/appsettings.json | 10 +++ test/WebSites/SecurityWebSite/project.json | 48 ++++++++++++++ test/WebSites/SecurityWebSite/web.config | 14 +++++ 16 files changed, 379 insertions(+), 4 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryAuthTests.cs create mode 100644 test/WebSites/SecurityWebSite/Controllers/HomeController.cs create mode 100644 test/WebSites/SecurityWebSite/Program.cs create mode 100644 test/WebSites/SecurityWebSite/SecurityWebSite.xproj create mode 100644 test/WebSites/SecurityWebSite/Startup.cs create mode 100644 test/WebSites/SecurityWebSite/Views/Home/Index.cshtml create mode 100644 test/WebSites/SecurityWebSite/Views/Shared/_Layout.cshtml create mode 100644 test/WebSites/SecurityWebSite/Views/_ViewImports.cshtml create mode 100644 test/WebSites/SecurityWebSite/Views/_ViewStart.cshtml create mode 100644 test/WebSites/SecurityWebSite/appsettings.json create mode 100644 test/WebSites/SecurityWebSite/project.json create mode 100644 test/WebSites/SecurityWebSite/web.config diff --git a/Mvc.sln b/Mvc.sln index 8c6313883b..65ac025971 100644 --- a/Mvc.sln +++ b/Mvc.sln @@ -123,6 +123,8 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "MvcSandbox", "samples\MvcSa EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SimpleWebSite", "test\WebSites\SimpleWebSite\SimpleWebSite.xproj", "{396B40D7-AC70-49A7-B33C-ED42129FEBE3}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SecurityWebSite", "test\WebSites\SecurityWebSite\SecurityWebSite.xproj", "{D28CAC79-7004-4B69-993B-EDEB4653BFA8}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -727,6 +729,18 @@ Global {396B40D7-AC70-49A7-B33C-ED42129FEBE3}.Release|Mixed Platforms.Build.0 = Release|Any CPU {396B40D7-AC70-49A7-B33C-ED42129FEBE3}.Release|x86.ActiveCfg = Release|Any CPU {396B40D7-AC70-49A7-B33C-ED42129FEBE3}.Release|x86.Build.0 = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|Any CPU.Build.0 = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|x86.ActiveCfg = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Debug|x86.Build.0 = Debug|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|Any CPU.ActiveCfg = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|Any CPU.Build.0 = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|x86.ActiveCfg = Release|Any CPU + {D28CAC79-7004-4B69-993B-EDEB4653BFA8}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -787,5 +801,6 @@ Global {9879B5D5-2325-4A81-B4DF-F279FE8FEEB4} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} {14ED4476-9F24-4776-8417-EA6927F6C9C9} = {DAAE4C74-D06F-4874-A166-33305D2643CE} {396B40D7-AC70-49A7-B33C-ED42129FEBE3} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} + {D28CAC79-7004-4B69-993B-EDEB4653BFA8} = {16703B76-C9F7-4C75-AE6C-53D92E308E3C} EndGlobalSection EndGlobal diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs index e40fea31ad..64015f7d8b 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs @@ -21,8 +21,23 @@ namespace Microsoft.AspNetCore.Mvc [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public class AutoValidateAntiforgeryTokenAttribute : Attribute, IFilterFactory, IOrderedFilter { - /// - public int Order { get; set; } + /// + /// Gets the order value for determining the order of execution of filters. Filters execute in + /// ascending numeric value of the property. + /// + /// + /// + /// Filters are executed in an ordering determined by an ascending sort of the property. + /// + /// + /// The default Order for this attribute is 1000 because it must run after any filter which does authentication + /// or login in order to allow them to behave as expected (ie Unauthenticated or Redirect instead of 400). + /// + /// + /// Look at for more detailed info. + /// + /// + public int Order { get; set; } = 1000; /// public bool IsReusable => true; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs index 938f9c160d..fcf1bd3f8f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs @@ -20,8 +20,23 @@ namespace Microsoft.AspNetCore.Mvc [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public class ValidateAntiForgeryTokenAttribute : Attribute, IFilterFactory, IOrderedFilter { - /// - public int Order { get; set; } + /// + /// Gets the order value for determining the order of execution of filters. Filters execute in + /// ascending numeric value of the property. + /// + /// + /// + /// Filters are executed in an ordering determined by an ascending sort of the property. + /// + /// + /// The default Order for this attribute is 1000 because it must run after any filter which does authentication + /// or login in order to allow them to behave as expected (ie Unauthenticated or Redirect instead of 400). + /// + /// + /// Look at for more detailed info. + /// + /// + public int Order { get; set; } = 1000; /// public bool IsReusable => true; diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryAuthTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryAuthTests.cs new file mode 100644 index 0000000000..5f19fbe851 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryAuthTests.cs @@ -0,0 +1,45 @@ +// 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 System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using SecurityWebSite; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class AntiforgeryAuthTests : IClassFixture> + { + public AntiforgeryAuthTests(MvcTestFixture fixture) + { + Client = fixture.Client; + } + + public HttpClient Client { get; } + + [Fact] + public async Task AutomaticAuthenticationBeforeAntiforgery() + { + // Arrange & Act + var response = await Client.PostAsync("http://localhost/Home/AutoAntiforgery", null); + + // Assert + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal("/Home/Login", response.Headers.Location.AbsolutePath, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public async Task AuthBeforeAntiforgery() + { + // Arrange & Act + var response = await Client.GetAsync("http://localhost/Home/Antiforgery"); + + // Assert + // Redirected to login page, Antiforgery didn't fail yet + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal("/Home/Login", response.Headers.Location.AbsolutePath, StringComparer.OrdinalIgnoreCase); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/project.json b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/project.json index f7bc2e2b76..f507dc997d 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/project.json +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/project.json @@ -36,6 +36,7 @@ "RazorPageExecutionInstrumentationWebSite": "1.0.0", "RazorWebSite": "1.0.0", "RoutingWebSite": "1.0.0", + "SecurityWebSite": "1.0.0", "SimpleWebSite": "1.0.0", "TagHelpersWebSite": "1.0.0", "VersioningWebSite": "1.0.0", diff --git a/test/WebSites/SecurityWebSite/Controllers/HomeController.cs b/test/WebSites/SecurityWebSite/Controllers/HomeController.cs new file mode 100644 index 0000000000..2a639ae399 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Controllers/HomeController.cs @@ -0,0 +1,41 @@ +// 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 Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; + +namespace SecurityWebSite.Controllers +{ + public class HomeController : Controller + { + public IActionResult Index() + { + return View(); + } + + [AutoValidateAntiforgeryToken] + [Authorize] + [HttpPost] + public IActionResult AutoAntiforgery() + { + return Content("Automaticaly doesn't matter"); + } + + [Authorize] + [ValidateAntiForgeryToken] + public IActionResult Antiforgery() + { + return Content("Doesn't matter"); + } + + public IActionResult Login() + { + return Content("Login!"); + } + + public IActionResult Logout() + { + return Content("Logout!"); + } + } +} diff --git a/test/WebSites/SecurityWebSite/Program.cs b/test/WebSites/SecurityWebSite/Program.cs new file mode 100644 index 0000000000..e9748d7609 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Program.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.IO; +using Microsoft.AspNetCore.Hosting; + +namespace SecurityWebSite +{ + public class Program + { + public static void Main(string[] args) + { + var host = new WebHostBuilder() + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseStartup() + .Build(); + + host.Run(); + } + } +} diff --git a/test/WebSites/SecurityWebSite/SecurityWebSite.xproj b/test/WebSites/SecurityWebSite/SecurityWebSite.xproj new file mode 100644 index 0000000000..fc7b205954 --- /dev/null +++ b/test/WebSites/SecurityWebSite/SecurityWebSite.xproj @@ -0,0 +1,23 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + d28cac79-7004-4b69-993b-edeb4653bfa8 + AjaxAntiForgeryValidation + .\obj + .\bin\ + v4.5.2 + + + 2.0 + + + + + + + diff --git a/test/WebSites/SecurityWebSite/Startup.cs b/test/WebSites/SecurityWebSite/Startup.cs new file mode 100644 index 0000000000..fe7fa7b9f2 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Startup.cs @@ -0,0 +1,63 @@ +// 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 Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace SecurityWebSite +{ + public class Startup + { + public Startup(IHostingEnvironment env) + { + var builder = new ConfigurationBuilder() + .SetBasePath(env.ContentRootPath) + .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true) + .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true) + .AddEnvironmentVariables(); + Configuration = builder.Build(); + } + + public IConfigurationRoot Configuration { get; } + + // This method gets called by the runtime. Use this method to add services to the container. + public void ConfigureServices(IServiceCollection services) + { + // Add framework services. + services.AddMvc(); + services.AddAntiforgery(); + services.AddAuthentication(); + } + + // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. + public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory) + { + loggerFactory.AddConsole(Configuration.GetSection("Logging")); + loggerFactory.AddDebug(); + + if (env.IsDevelopment()) + { + app.UseDeveloperExceptionPage(); + } + + app.UseStaticFiles(); + app.UseCookieAuthentication(new CookieAuthenticationOptions + { + LoginPath = "/Home/Login", + LogoutPath = "/Home/Logout", + AutomaticAuthenticate = true, + AutomaticChallenge = true + }); + + app.UseMvc(routes => + { + routes.MapRoute( + name: "default", + template: "{controller=Home}/{action=Index}/{id?}"); + }); + } + } +} diff --git a/test/WebSites/SecurityWebSite/Views/Home/Index.cshtml b/test/WebSites/SecurityWebSite/Views/Home/Index.cshtml new file mode 100644 index 0000000000..cb0dbdf126 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Views/Home/Index.cshtml @@ -0,0 +1,21 @@ +@{ + ViewData["Title"] = "Home Page"; +} +

Hello!

+ + diff --git a/test/WebSites/SecurityWebSite/Views/Shared/_Layout.cshtml b/test/WebSites/SecurityWebSite/Views/Shared/_Layout.cshtml new file mode 100644 index 0000000000..dfa4dbe9f1 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Views/Shared/_Layout.cshtml @@ -0,0 +1,37 @@ + + + + + + @ViewData["Title"] - SecurityWebSite + + +
+
+
+ + SecurityWebSite +
+
+ +
+
+
+
+ @RenderBody() +
+
+

© 2016 - SecurityWebSite

+
+
+ + diff --git a/test/WebSites/SecurityWebSite/Views/_ViewImports.cshtml b/test/WebSites/SecurityWebSite/Views/_ViewImports.cshtml new file mode 100644 index 0000000000..f65eca4041 --- /dev/null +++ b/test/WebSites/SecurityWebSite/Views/_ViewImports.cshtml @@ -0,0 +1,2 @@ +@using SecurityWebSite + @addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers diff --git a/test/WebSites/SecurityWebSite/Views/_ViewStart.cshtml b/test/WebSites/SecurityWebSite/Views/_ViewStart.cshtml new file mode 100644 index 0000000000..a5f10045db --- /dev/null +++ b/test/WebSites/SecurityWebSite/Views/_ViewStart.cshtml @@ -0,0 +1,3 @@ +@{ + Layout = "_Layout"; +} diff --git a/test/WebSites/SecurityWebSite/appsettings.json b/test/WebSites/SecurityWebSite/appsettings.json new file mode 100644 index 0000000000..fa8ce71a97 --- /dev/null +++ b/test/WebSites/SecurityWebSite/appsettings.json @@ -0,0 +1,10 @@ +{ + "Logging": { + "IncludeScopes": false, + "LogLevel": { + "Default": "Debug", + "System": "Information", + "Microsoft": "Information" + } + } +} diff --git a/test/WebSites/SecurityWebSite/project.json b/test/WebSites/SecurityWebSite/project.json new file mode 100644 index 0000000000..a1bfa558d5 --- /dev/null +++ b/test/WebSites/SecurityWebSite/project.json @@ -0,0 +1,48 @@ +{ + "buildOptions": { + "emitEntryPoint": true, + "preserveCompilationContext": true + }, + "dependencies": { + "Microsoft.Extensions.Logging.Debug": "1.1.0-*", + "Microsoft.Extensions.Logging.Console": "1.1.0-*", + "Microsoft.Extensions.Configuration.Json": "1.1.0-*", + "Microsoft.Extensions.Configuration.FileExtensions": "1.1.0-*", + "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.1.0-*", + "Microsoft.Extensions.Configuration": "1.1.0-*", + "Microsoft.AspNetCore.StaticFiles": "1.1.0-*", + "Microsoft.AspNetCore.Server.Kestrel": "1.1.0-*", + "Microsoft.AspNetCore.Mvc": "1.1.0-*", + "Microsoft.AspNetCore.Mvc.ViewFeatures": "1.1.0-*", + "Microsoft.AspNetCore.Identity.EntityFrameworkCore": "1.1.0-*", + "Microsoft.AspNetCore.Identity": "1.1.0-*", + "Microsoft.AspNetCore.Hosting": "1.1.0-*", + "Microsoft.AspNetCore.Diagnostics": "1.1.0-*" + }, + "frameworks": { + "netcoreapp1.1": { + "dependencies": { + "Microsoft.NETCore.App": { + "version": "1.1.0-*", + "type": "platform" + } + }, + "imports": "portable-net451+win8" + }, + "net451": {} + }, + "publishOptions": { + "include": [ + "wwwroot", + "appsettings.json", + "web.config" + ] + }, + "scripts": { + "postpublish": [ "dotnet publish-iis --publish-folder %publish:OutputPath% --framework %publish:FullTargetFramework%" ] + }, + "tools": { + "Microsoft.AspNetCore.Razor.Tools": "1.0.0-preview2-final", + "Microsoft.AspNetCore.Server.IISIntegration.Tools": "1.0.0-preview2-final" + } +} diff --git a/test/WebSites/SecurityWebSite/web.config b/test/WebSites/SecurityWebSite/web.config new file mode 100644 index 0000000000..dc0514fca5 --- /dev/null +++ b/test/WebSites/SecurityWebSite/web.config @@ -0,0 +1,14 @@ + + + + + + + + + + + +