Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Return BadRequest response when antiforgery token validation fails
Browse files Browse the repository at this point in the history
  • Loading branch information
ajaybhargavb committed Jan 8, 2016
1 parent 8afe28c commit bf1fcf6
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
using System;
using Microsoft.AspNet.Antiforgery;
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal
{
public class AutoValidateAntiforgeryTokenAuthorizationFilter : ValidateAntiforgeryTokenAuthorizationFilter
{
public AutoValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery)
: base(antiforgery)
public AutoValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery, ILoggerFactory loggerFactory)
: base(antiforgery, loggerFactory)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@
using Microsoft.AspNet.Antiforgery;
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.AspNet.Mvc.Internal;
using Microsoft.AspNet.Mvc.Logging;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNet.Mvc.ViewFeatures.Internal
{
public class ValidateAntiforgeryTokenAuthorizationFilter : IAsyncAuthorizationFilter, IAntiforgeryPolicy
{
private readonly IAntiforgery _antiforgery;
private readonly ILogger _logger;

public ValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery)
public ValidateAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery, ILoggerFactory loggerFactory)
{
if (antiforgery == null)
{
throw new ArgumentNullException(nameof(antiforgery));
}

_antiforgery = antiforgery;
_logger = loggerFactory.CreateLogger<ValidateAntiforgeryTokenAuthorizationFilter>();
}

public Task OnAuthorizationAsync(AuthorizationContext context)
public async Task OnAuthorizationAsync(AuthorizationContext context)
{
if (context == null)
{
Expand All @@ -34,10 +38,16 @@ public Task OnAuthorizationAsync(AuthorizationContext context)

if (IsClosestAntiforgeryPolicy(context.Filters) && ShouldValidate(context))
{
return _antiforgery.ValidateRequestAsync(context.HttpContext);
try
{
await _antiforgery.ValidateRequestAsync(context.HttpContext);
}
catch (AntiforgeryValidationException exception)
{
_logger.AntiforgeryTokenInvalid(exception.Message, exception);
context.Result = new BadRequestResult();
}
}

return TaskCache.CompletedTask;
}

protected virtual bool ShouldValidate(AuthorizationContext context)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.Extensions.Logging;

namespace Microsoft.AspNet.Mvc.Logging
{
public static class ValidateAntiforgeryTokenAuthorizationFilterLoggerExtensions
{
private static readonly Action<ILogger, string, Exception> _antiforgeryTokenInvalid;

static ValidateAntiforgeryTokenAuthorizationFilterLoggerExtensions()
{
_antiforgeryTokenInvalid = LoggerMessage.Define<string>(
LogLevel.Information,
1,
"Antiforgery token validation failed. {Message}");
}

public static void AntiforgeryTokenInvalid(this ILogger logger, string message, Exception exception)
{
_antiforgeryTokenInvalid(logger, message, exception);
}
}
}
29 changes: 29 additions & 0 deletions test/Microsoft.AspNet.Mvc.FunctionalTests/AntiforgeryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNet.Antiforgery;
using Xunit;

namespace Microsoft.AspNet.Mvc.FunctionalTests
Expand Down Expand Up @@ -116,5 +117,33 @@ public async Task SetCookieAndHeaderBeforeFlushAsync_PostToForm()
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("OK", await response.Content.ReadAsStringAsync());
}

[Fact]
public async Task Antiforgery_HeaderNotSet_SendsBadRequest()
{
// Arrange
var getResponse = await Client.GetAsync("http://localhost/Antiforgery/Login");
var responseBody = await getResponse.Content.ReadAsStringAsync();

var formToken = AntiforgeryTestHelper.RetrieveAntiforgeryToken(
responseBody,
"Antiforgery/Login");

var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/Antiforgery/Login");
var nameValueCollection = new List<KeyValuePair<string, string>>
{
new KeyValuePair<string,string>("__RequestVerificationToken", formToken),
new KeyValuePair<string,string>("UserName", "test"),
new KeyValuePair<string,string>("Password", "password"),
};

request.Content = new FormUrlEncodedContent(nameValueCollection);

// Act
var response = await Client.SendAsync(request);

// Assert
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.AspNet.Routing;
using Microsoft.Extensions.Logging.Testing;
using Moq;
using Xunit;

Expand All @@ -28,7 +29,7 @@ public async Task Filter_ValidatesAntiforgery_ForUnsafeMethod(string httpMethod)
.Returns(Task.FromResult(0))
.Verifiable();

var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object);
var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object, NullLoggerFactory.Instance);

var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
actionContext.HttpContext.Request.Method = httpMethod;
Expand Down Expand Up @@ -56,7 +57,7 @@ public async Task Filter_SkipsAntiforgeryVerification_ForSafeMethod(string httpM
.Returns(Task.FromResult(0))
.Verifiable();

var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object);
var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object, NullLoggerFactory.Instance);

var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
actionContext.HttpContext.Request.Method = httpMethod;
Expand All @@ -80,7 +81,7 @@ public async Task Filter_SkipsAntiforgeryVerification_WhenOverridden()
.Returns(Task.FromResult(0))
.Verifiable();

var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object);
var filter = new AutoValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object, NullLoggerFactory.Instance);

var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
actionContext.HttpContext.Request.Method = "POST";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.AspNet.Mvc.Filters;
using Microsoft.AspNet.Routing;
using Microsoft.Extensions.Logging.Testing;
using Moq;
using Xunit;

Expand All @@ -32,7 +33,7 @@ public async Task Filter_ValidatesAntiforgery_ForAllMethods(string httpMethod)
.Returns(Task.FromResult(0))
.Verifiable();

var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object);
var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object, NullLoggerFactory.Instance);

var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
actionContext.HttpContext.Request.Method = httpMethod;
Expand All @@ -56,7 +57,7 @@ public async Task Filter_SkipsAntiforgeryVerification_WhenOverridden()
.Returns(Task.FromResult(0))
.Verifiable();

var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object);
var filter = new ValidateAntiforgeryTokenAuthorizationFilter(antiforgery.Object, NullLoggerFactory.Instance);

var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
actionContext.HttpContext.Request.Method = "POST";
Expand Down

0 comments on commit bf1fcf6

Please sign in to comment.