From d0f363a1be3ca98399c903ad50a0ee47364c150f Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Thu, 27 Oct 2016 17:18:50 -0700 Subject: [PATCH] [Fixes #105] Disable caching when response uses antiforgery --- .../IAntiforgery.cs | 2 +- .../Internal/AntiforgeryLoggerExtensions.cs | 10 ++ .../Internal/DefaultAntiforgery.cs | 16 ++ .../Internal/DefaultAntiforgeryTest.cs | 162 ++++++++++++++++++ 4 files changed, 189 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs index 6ac2e44..68be15b 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs @@ -14,7 +14,7 @@ public interface IAntiforgery { /// /// Generates an for this request and stores the cookie token - /// in the response. + /// in the response. It also sets the "Cache-control" and "Pragma" headers to "no-cache". /// /// The associated with the current request. /// An with tokens for the response. diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryLoggerExtensions.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryLoggerExtensions.cs index 5e115bb..6d8658b 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryLoggerExtensions.cs @@ -15,6 +15,7 @@ internal static class AntiforgeryLoggerExtensions private static readonly Action _newCookieToken; private static readonly Action _reusedCookieToken; private static readonly Action _tokenDeserializeException; + private static readonly Action _responseNotCached; static AntiforgeryLoggerExtensions() { @@ -47,6 +48,10 @@ static AntiforgeryLoggerExtensions() LogLevel.Error, 7, "An exception was thrown while deserializing the token."); + _responseNotCached = LoggerMessage.Define( + LogLevel.Information, + 7, + "The 'Cache-Control' and 'Pragma' headers of the response have been set as 'no-cache'."); } public static void ValidationFailed(this ILogger logger, string message) @@ -83,5 +88,10 @@ public static void TokenDeserializeException(this ILogger logger, Exception exce { _tokenDeserializeException(logger, exception); } + + public static void ResponseNotCached(this ILogger logger) + { + _responseNotCached(logger, null); + } } } diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs index eeb92cf..dd53db2 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Antiforgery.Internal { @@ -66,6 +67,10 @@ public AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext) } } + // Explicitly set the cache headers to 'no-cache'. This could override any user set value but this is fine + // as a response with antiforgery token must never be cached. + SetDoNotCacheHeaders(httpContext); + return tokenSet; } @@ -237,6 +242,10 @@ public void SetCookieTokenAndHeader(HttpContext httpContext) { _logger.ReusedCookieToken(); } + + // Explicitly set the cache headers to 'no-cache'. This could override any user set value but this is fine + // as a response with antiforgery token must never be cached. + SetDoNotCacheHeaders(httpContext); } private void SaveCookieTokenAndHeader(HttpContext httpContext, string cookieToken) @@ -358,6 +367,13 @@ private IAntiforgeryFeature GetTokensInternal(HttpContext httpContext) return antiforgeryFeature; } + private void SetDoNotCacheHeaders(HttpContext httpContext) + { + httpContext.Response.Headers[HeaderNames.CacheControl] = "no-cache"; + httpContext.Response.Headers[HeaderNames.Pragma] = "no-cache"; + _logger.ResponseNotCached(); + } + private AntiforgeryTokenSet Serialize(IAntiforgeryFeature antiforgeryFeature) { // Should only be called after new tokens have been generated. diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs index 09de924..f238d9f 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Net.Http.Headers; using Moq; using Xunit; @@ -275,6 +276,67 @@ public void GetAndStoreTokens_ExistingValidCookieToken_NotOverriden() Assert.Equal(context.TestTokenSet.FormTokenString, antiforgeryFeature.NewRequestTokenString); } + [Fact] + public void GetAndStoreTokens_ExistingValidCookieToken_NotOverriden_AndSetsDoNotCacheHeaders() + { + // Arrange + var antiforgeryFeature = new AntiforgeryFeature(); + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + + // Act + var tokenSet = antiforgery.GetAndStoreTokens(context.HttpContext); + + // Assert + // We shouldn't have saved the cookie because it already existed. + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), + Times.Never); + + Assert.Null(tokenSet.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenSet.RequestToken); + + Assert.NotNull(antiforgeryFeature); + Assert.Equal(context.TestTokenSet.OldCookieToken, antiforgeryFeature.CookieToken); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.CacheControl]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.Pragma]); + } + + [Fact] + public void GetAndStoreTokens_ExistingCachingHeaders_Overriden() + { + // Arrange + var antiforgeryFeature = new AntiforgeryFeature(); + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + context.HttpContext.Response.Headers["Cache-Control"] = "public"; + + // Act + var tokenSet = antiforgery.GetAndStoreTokens(context.HttpContext); + + // Assert + // We shouldn't have saved the cookie because it already existed. + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), + Times.Never); + + Assert.Null(tokenSet.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenSet.RequestToken); + + Assert.NotNull(antiforgeryFeature); + Assert.Equal(context.TestTokenSet.OldCookieToken, antiforgeryFeature.CookieToken); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.CacheControl]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.Pragma]); + } + [Fact] public void GetAndStoreTokens_NoExistingCookieToken_Saved() { @@ -309,6 +371,36 @@ public void GetAndStoreTokens_NoExistingCookieToken_Saved() Assert.True(antiforgeryFeature.HaveStoredNewCookieToken); } + [Fact] + public void GetAndStoreTokens_NoExistingCookieToken_Saved_AndSetsDoNotCacheHeaders() + { + // Arrange + var antiforgeryFeature = new AntiforgeryFeature(); + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: false, + isOldCookieValid: false, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + + // Act + var tokenSet = antiforgery.GetAndStoreTokens(context.HttpContext); + + // Assert + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), context.TestTokenSet.NewCookieTokenString), + Times.Once); + + Assert.Equal(context.TestTokenSet.NewCookieTokenString, tokenSet.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenSet.RequestToken); + + Assert.NotNull(antiforgeryFeature); + Assert.True(antiforgeryFeature.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, antiforgeryFeature.CookieToken); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.CacheControl]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers[HeaderNames.Pragma]); + } + [Fact] public void GetAndStoreTokens_DoesNotSerializeTwice() { @@ -808,6 +900,76 @@ public void SetCookieTokenAndHeader_PreserveXFrameOptionsHeader() Assert.Equal(expectedHeaderValue, xFrameOptions); } + [Fact] + public void SetCookieTokenAndHeader_NewCookieToken_SetsDoNotCacheHeaders() + { + // Arrange + var options = new AntiforgeryOptions(); + var antiforgeryFeature = new AntiforgeryFeature(); + + // Generate a new cookie. + var context = CreateMockContext( + options, + useOldCookie: false, + isOldCookieValid: false, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + + // Act + antiforgery.SetCookieTokenAndHeader(context.HttpContext); + + // Assert + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Cache-Control"]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Pragma"]); + } + + [Fact] + public void SetCookieTokenAndHeader_ValidOldCookieToken_SetsDoNotCacheHeaders() + { + // Arrange + var options = new AntiforgeryOptions(); + var antiforgeryFeature = new AntiforgeryFeature(); + + // Generate a new cookie. + var context = CreateMockContext( + options, + useOldCookie: true, + isOldCookieValid: true, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + + // Act + antiforgery.SetCookieTokenAndHeader(context.HttpContext); + + // Assert + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Cache-Control"]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Pragma"]); + } + + [Fact] + public void SetCookieTokenAndHeader_OverridesExistingCachingHeaders() + { + // Arrange + var options = new AntiforgeryOptions(); + var antiforgeryFeature = new AntiforgeryFeature(); + + // Generate a new cookie. + var context = CreateMockContext( + options, + useOldCookie: true, + isOldCookieValid: true, + antiforgeryFeature: antiforgeryFeature); + var antiforgery = GetAntiforgery(context); + context.HttpContext.Response.Headers["Cache-Control"] = "public"; + + // Act + antiforgery.SetCookieTokenAndHeader(context.HttpContext); + + // Assert + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Cache-Control"]); + Assert.Equal("no-cache", context.HttpContext.Response.Headers["Pragma"]); + } + [Theory] [InlineData(false, "SAMEORIGIN")] [InlineData(true, null)]