From a500a93dfbb37762876e17dcd8d2b57feb610580 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 17 Dec 2015 14:33:23 -0800 Subject: [PATCH] Smarter antiforgery Adds the concept of an IAntiforgeryPolicy marker interface as well as the ability to overide policy with a 'closer' filter. Adds a new [IgnoreAntiforgeryToken] attribute for overriding a scoped antiforgery policy. Adds a new [AutoValidateAntiforgeryToken] attribute (good name tbd) for applying an application-wide antiforgery token. The idea is that you can configure this as a global filter if your site is acting as a pure browser-based or 1st party SPA. This new attribute only validates the token for unsafe HTTP methods, so you can apply it broadly. --- .../AutoValidateAntiforgeryTokenAttribute.cs | 33 ++++++ ...MvcViewFeaturesMvcCoreBuilderExtensions.cs | 6 ++ .../IgnoreAntiforgeryTokenAttribute.cs | 19 ++++ ...dateAntiforgeryTokenAuthorizationFilter.cs | 37 +++++++ ...dateAntiforgeryTokenAuthorizationFilter.cs | 69 ++++++++++++ .../ValidateAntiForgeryTokenAttribute.cs | 6 +- .../ViewFeatures/IAntiforgeryPolicy.cs | 14 +++ ...dateAntiforgeryTokenAuthorizationFilter.cs | 35 ------ ...AntiforgeryTokenAuthorizationFilterTest.cs | 101 ++++++++++++++++++ ...AntiforgeryTokenAuthorizationFilterTest.cs | 77 +++++++++++++ 10 files changed, 358 insertions(+), 39 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs create mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/IgnoreAntiforgeryTokenAttribute.cs create mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/AutoValidateAntiforgeryTokenAuthorizationFilter.cs create mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs create mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/IAntiforgeryPolicy.cs delete mode 100644 src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ValidateAntiforgeryTokenAuthorizationFilter.cs create mode 100644 test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/AutoValidateAntiforgeryTokenAuthorizationFilterTest.cs create mode 100644 test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/ValidateAntiforgeryTokenAuthorizationFilterTest.cs diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs new file mode 100644 index 0000000000..1370d8de60 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs @@ -0,0 +1,33 @@ +// 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.Filters; +using Microsoft.AspNet.Mvc.ViewFeatures.Internal; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An attribute that causes validation of antiforgery tokens for all unsafe HTTP methods. An antiforgery + /// token is required for HTTP methods other than GET, HEAD, OPTIONS, and TRACE. + /// + /// + /// can be applied at as a global filter to trigger + /// validation of antiforgery tokens by default for an application. Use + /// to suppress validation of the antiforgery token for + /// a controller or action. + /// + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + public class AutoValidateAntiforgeryTokenAttribute : Attribute, IFilterFactory, IOrderedFilter + { + /// + public int Order { get; set; } + + /// + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) + { + return serviceProvider.GetRequiredService(); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs index 4dbebfa8e9..531f2ce6a4 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/DependencyInjection/MvcViewFeaturesMvcCoreBuilderExtensions.cs @@ -136,6 +136,12 @@ namespace Microsoft.Extensions.DependencyInjection // This does caching so it should stay singleton services.TryAddSingleton(); + // + // Antiforgery + // + services.TryAddSingleton(); + services.TryAddSingleton(); + // These are stateless so their lifetime isn't really important. services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/IgnoreAntiforgeryTokenAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/IgnoreAntiforgeryTokenAttribute.cs new file mode 100644 index 0000000000..2ca61c83a7 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/IgnoreAntiforgeryTokenAttribute.cs @@ -0,0 +1,19 @@ +// 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.Filters; +using Microsoft.AspNet.Mvc.ViewFeatures; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An attribute which skips antiforgery token validation. + /// + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + public class IgnoreAntiforgeryTokenAttribute : Attribute, IAntiforgeryPolicy, IOrderedFilter + { + /// + public int Order { get; set; } + } +} diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/AutoValidateAntiforgeryTokenAuthorizationFilter.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/AutoValidateAntiforgeryTokenAuthorizationFilter.cs new file mode 100644 index 0000000000..cb3d2a4ce8 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/AutoValidateAntiforgeryTokenAuthorizationFilter.cs @@ -0,0 +1,37 @@ +// 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.Antiforgery; +using Microsoft.AspNet.Mvc.Filters; + +namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal +{ + public class AutoValidateAntiforgeryTokenAuthorizationFilter : ValidateAntiforgeryTokenAuthorizationFilter + { + public AutoValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery) + : base(antiforgery) + { + } + + protected override bool ShouldValidate(AuthorizationContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var method = context.HttpContext.Request.Method; + if (string.Equals("GET", method, StringComparison.OrdinalIgnoreCase) || + string.Equals("HEAD", method, StringComparison.OrdinalIgnoreCase) || + string.Equals("TRACE", method, StringComparison.OrdinalIgnoreCase) || + string.Equals("OPTIONS", method, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + // Anything else requires a token. + return true; + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs new file mode 100644 index 0000000000..50eb7792ea --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs @@ -0,0 +1,69 @@ +// 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.Collections.Generic; +using System.Diagnostics; +using System.Threading.Tasks; +using Microsoft.AspNet.Antiforgery; +using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.Internal; + +namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal +{ + public class ValidateAntiforgeryTokenAuthorizationFilter : IAsyncAuthorizationFilter, IAntiforgeryPolicy + { + private readonly IAntiforgery _antiforgery; + + public ValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery) + { + if (antiforgery == null) + { + throw new ArgumentNullException(nameof(antiforgery)); + } + + _antiforgery = antiforgery; + } + + public Task OnAuthorizationAsync(AuthorizationContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (IsClosestAntiforgeryPolicy(context.Filters) && ShouldValidate(context)) + { + return _antiforgery.ValidateRequestAsync(context.HttpContext); + } + + return TaskCache.CompletedTask; + } + + protected virtual bool ShouldValidate(AuthorizationContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + return true; + } + + private bool IsClosestAntiforgeryPolicy(IList filters) + { + // Determine if this instance is the 'effective' antiforgery policy. + for (var i = filters.Count - 1; i >= 0; i--) + { + var filter = filters[i]; + if (filter is IAntiforgeryPolicy) + { + return object.ReferenceEquals(this, filter); + } + } + + Debug.Fail("The current instance should be in the list of filters."); + return false; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs index cc4e203bce..6ac43a6aef 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs @@ -2,9 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNet.Antiforgery; using Microsoft.AspNet.Mvc.Filters; -using Microsoft.AspNet.Mvc.ViewFeatures; +using Microsoft.AspNet.Mvc.ViewFeatures.Internal; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNet.Mvc @@ -24,8 +23,7 @@ namespace Microsoft.AspNet.Mvc public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { - var antiforgery = serviceProvider.GetRequiredService(); - return new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery); + return serviceProvider.GetRequiredService(); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/IAntiforgeryPolicy.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/IAntiforgeryPolicy.cs new file mode 100644 index 0000000000..4e861b94d5 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/IAntiforgeryPolicy.cs @@ -0,0 +1,14 @@ +// 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.AspNet.Mvc.Filters; + +namespace Microsoft.AspNet.Mvc.ViewFeatures +{ + /// + /// A marker interface for filters which define a policy for antiforgery token validation. + /// + public interface IAntiforgeryPolicy : IFilterMetadata + { + } +} diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ValidateAntiforgeryTokenAuthorizationFilter.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ValidateAntiforgeryTokenAuthorizationFilter.cs deleted file mode 100644 index 09a7f2e4f8..0000000000 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/ValidateAntiforgeryTokenAuthorizationFilter.cs +++ /dev/null @@ -1,35 +0,0 @@ -// 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.Threading.Tasks; -using Microsoft.AspNet.Antiforgery; -using Microsoft.AspNet.Mvc.Filters; - -namespace Microsoft.AspNet.Mvc.ViewFeatures -{ - public class ValidateAntiforgeryTokenAuthorizationFilter : IAsyncAuthorizationFilter - { - private readonly IAntiforgery _antiforgery; - - public ValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery) - { - if (antiforgery == null) - { - throw new ArgumentNullException(nameof(antiforgery)); - } - - _antiforgery = antiforgery; - } - - public Task OnAuthorizationAsync(AuthorizationContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - return _antiforgery.ValidateRequestAsync(context.HttpContext); - } - } -} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/AutoValidateAntiforgeryTokenAuthorizationFilterTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/AutoValidateAntiforgeryTokenAuthorizationFilterTest.cs new file mode 100644 index 0000000000..aebece829b --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/AutoValidateAntiforgeryTokenAuthorizationFilterTest.cs @@ -0,0 +1,101 @@ +// 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.Threading.Tasks; +using Microsoft.AspNet.Antiforgery; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Routing; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal +{ + public class AutoValidateAntiforgeryTokenAuthorizationFilterTest + { + [Theory] + [InlineData("PUT")] + [InlineData("POsT")] + [InlineData("DeLETE")] + public async Task Filter_ValidatesAntiforgery_ForUnsafeMethod(string httpMethod) + { + // Arrange + var antiforgery = new Mock(MockBehavior.Strict); + antiforgery + .Setup(a => a.ValidateRequestAsync(It.IsAny())) + .Returns(Task.FromResult(0)) + .Verifiable(); + + var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object); + + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + actionContext.HttpContext.Request.Method = httpMethod; + + var context = new AuthorizationContext(actionContext, new[] { filter }); + + // Act + await filter.OnAuthorizationAsync(context); + + // Assert + antiforgery.Verify(); + } + + [Theory] + [InlineData("GET")] + [InlineData("HEAD")] + [InlineData("TracE")] + [InlineData("OPTIONs")] + public async Task Filter_SkipsAntiforgeryVerification_ForSafeMethod(string httpMethod) + { + // Arrange + var antiforgery = new Mock(MockBehavior.Strict); + antiforgery + .Setup(a => a.ValidateRequestAsync(It.IsAny())) + .Returns(Task.FromResult(0)) + .Verifiable(); + + var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object); + + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + actionContext.HttpContext.Request.Method = httpMethod; + + var context = new AuthorizationContext(actionContext, new[] { filter }); + + // Act + await filter.OnAuthorizationAsync(context); + + // Assert + antiforgery.Verify(a => a.ValidateRequestAsync(It.IsAny()), Times.Never()); + } + + [Fact] + public async Task Filter_SkipsAntiforgeryVerification_WhenOverridden() + { + // Arrange + var antiforgery = new Mock(MockBehavior.Strict); + antiforgery + .Setup(a => a.ValidateRequestAsync(It.IsAny())) + .Returns(Task.FromResult(0)) + .Verifiable(); + + var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object); + + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + actionContext.HttpContext.Request.Method = "POST"; + + var context = new AuthorizationContext(actionContext, new IFilterMetadata[] + { + filter, + new IgnoreAntiforgeryTokenAttribute(), + }); + + // Act + await filter.OnAuthorizationAsync(context); + + // Assert + antiforgery.Verify(a => a.ValidateRequestAsync(It.IsAny()), Times.Never()); + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/ValidateAntiforgeryTokenAuthorizationFilterTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/ValidateAntiforgeryTokenAuthorizationFilterTest.cs new file mode 100644 index 0000000000..3c645ffce4 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Internal/ValidateAntiforgeryTokenAuthorizationFilterTest.cs @@ -0,0 +1,77 @@ +// 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.Threading.Tasks; +using Microsoft.AspNet.Antiforgery; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Routing; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal +{ + public class ValidateAntiforgeryTokenAuthorizationFilterTest + { + [Theory] + [InlineData("PUT")] + [InlineData("POsT")] + [InlineData("DeLETE")] + [InlineData("GET")] + [InlineData("HEAD")] + [InlineData("TracE")] + [InlineData("OPTIONs")] + public async Task Filter_ValidatesAntiforgery_ForAllMethods(string httpMethod) + { + // Arrange + var antiforgery = new Mock(MockBehavior.Strict); + antiforgery + .Setup(a => a.ValidateRequestAsync(It.IsAny())) + .Returns(Task.FromResult(0)) + .Verifiable(); + + var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object); + + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + actionContext.HttpContext.Request.Method = httpMethod; + + var context = new AuthorizationContext(actionContext, new[] { filter }); + + // Act + await filter.OnAuthorizationAsync(context); + + // Assert + antiforgery.Verify(); + } + + [Fact] + public async Task Filter_SkipsAntiforgeryVerification_WhenOverridden() + { + // Arrange + var antiforgery = new Mock(MockBehavior.Strict); + antiforgery + .Setup(a => a.ValidateRequestAsync(It.IsAny())) + .Returns(Task.FromResult(0)) + .Verifiable(); + + var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object); + + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + actionContext.HttpContext.Request.Method = "POST"; + + var context = new AuthorizationContext(actionContext, new IFilterMetadata[] + { + filter, + new IgnoreAntiforgeryTokenAttribute(), + }); + + // Act + await filter.OnAuthorizationAsync(context); + + // Assert + antiforgery.Verify(a => a.ValidateRequestAsync(It.IsAny()), Times.Never()); + } + } +}