From ac107b53717d7e66ba2d6c0c40dd6feabeac746b Mon Sep 17 00:00:00 2001
From: Ryan Nowak <nowakra@gmail.com>
Date: Fri, 12 Feb 2016 15:56:07 -0800
Subject: [PATCH] Make IsRequestValid check HTTP method

This code was popping up everywhere this method is called. Seems bad to
duplicate it. Really what the caller wants to know is 'is the request
valid or a potential CSRF exploit?'. This gets the API closer to that.
---
 .../IAntiforgery.cs                           |  7 +-
 .../Internal/DefaultAntiforgery.cs            | 10 +++
 .../Internal/DefaultAntiforgeryTest.cs        | 70 +++++++++++++++++++
 3 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs
index 0cdcf35a8b..8d62f67ec1 100644
--- a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs
+++ b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs
@@ -36,12 +36,13 @@ namespace Microsoft.AspNetCore.Antiforgery
         AntiforgeryTokenSet GetTokens(HttpContext httpContext);
 
         /// <summary>
-        /// Asynchronously returns a value indicating whether the request contains a valid antiforgery token.
+        /// Asynchronously returns a value indicating whether the request passes antiforgery validation. If the
+        /// request uses a safe HTTP method (GET, HEAD, OPTIONS, TRACE), the antiforgery token is not validated.
         /// </summary>
         /// <param name="httpContext">The <see cref="HttpContext"/> associated with the current request.</param>
         /// <returns>
-        /// A <see cref="Task{bool}"/> that, when completed, returns <c>true</c> if the request contains a
-        /// valid antiforgery token, otherwise returns <c>false</c>.
+        /// A <see cref="Task{bool}"/> that, when completed, returns <c>true</c> if the is requst uses a safe HTTP
+        /// method or contains a value antiforgery token, otherwise returns <c>false</c>.
         /// </returns>
         Task<bool> IsRequestValidAsync(HttpContext httpContext);
 
diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs
index a22576bd0b..7788a2bb0b 100644
--- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs
+++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs
@@ -95,6 +95,16 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
 
             CheckSSLConfig(httpContext);
 
+            var method = httpContext.Request.Method;
+            if (string.Equals(method, "GET", StringComparison.OrdinalIgnoreCase) ||
+                string.Equals(method, "HEAD", StringComparison.OrdinalIgnoreCase) ||
+                string.Equals(method, "OPTIONS", StringComparison.OrdinalIgnoreCase) ||
+                string.Equals(method, "TRACE", StringComparison.OrdinalIgnoreCase))
+            {
+                // Validation not needed for these request types.
+                return true;
+            }
+
             var tokens = await _tokenStore.GetRequestTokensAsync(httpContext);
             if (tokens.CookieToken == null)
             {
diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs
index 6fd193db1a..670b7f607b 100644
--- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs
+++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs
@@ -448,6 +448,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
             // Arrange
             var contextAccessor = new DefaultAntiforgeryContextAccessor();
             var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor);
+            context.HttpContext.Request.Method = "POST";
 
             string message;
             context.TokenGenerator
@@ -490,6 +491,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 },
             };
             var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor);
+            context.HttpContext.Request.Method = "POST";
 
             string message;
             context.TokenGenerator
@@ -519,6 +521,74 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 Times.Never);
         }
 
+        [Theory]
+        [InlineData("GeT")]
+        [InlineData("HEAD")]
+        [InlineData("options")]
+        [InlineData("TrAcE")]
+        public async Task IsRequestValidAsync_SkipsAntiforgery_ForSafeHttpMethods(string httpMethod)
+        {
+            // Arrange
+            var context = CreateMockContext(new AntiforgeryOptions());
+            context.HttpContext.Request.Method = httpMethod;
+
+            string message;
+            context.TokenGenerator
+                .Setup(o => o.TryValidateTokenSet(
+                    context.HttpContext,
+                    It.IsAny<AntiforgeryToken>(),
+                    It.IsAny<AntiforgeryToken>(),
+                    out message))
+                .Returns(false)
+                .Verifiable();
+
+            var antiforgery = GetAntiforgery(context);
+
+            // Act
+            var result = await antiforgery.IsRequestValidAsync(context.HttpContext);
+
+            // Assert
+            Assert.True(result);
+            context.TokenGenerator
+                .Verify(o => o.TryValidateTokenSet(
+                    context.HttpContext,
+                    It.IsAny<AntiforgeryToken>(),
+                    It.IsAny<AntiforgeryToken>(),
+                    out message),
+                    Times.Never);
+        }
+
+        [Theory]
+        [InlineData("PUT")]
+        [InlineData("post")]
+        [InlineData("Delete")]
+        [InlineData("Custom")]
+        public async Task IsRequestValidAsync_ValidatesAntiforgery_ForNonSafeHttpMethods(string httpMethod)
+        {
+            // Arrange
+            var context = CreateMockContext(new AntiforgeryOptions());
+            context.HttpContext.Request.Method = httpMethod;
+
+            string message;
+            context.TokenGenerator
+                .Setup(o => o.TryValidateTokenSet(
+                    context.HttpContext,
+                    It.IsAny<AntiforgeryToken>(),
+                    It.IsAny<AntiforgeryToken>(),
+                    out message))
+                .Returns(true)
+                .Verifiable();
+
+            var antiforgery = GetAntiforgery(context);
+
+            // Act
+            var result = await antiforgery.IsRequestValidAsync(context.HttpContext);
+
+            // Assert
+            Assert.True(result);
+            context.TokenGenerator.Verify();
+        }
+
         [Fact]
         public async Task ValidateRequestAsync_FromStore_Failure()
         {