From 73695fc4439889ecad7b6656235de7eb0feafe5f Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 5 Feb 2016 15:15:09 -0800 Subject: [PATCH] Serialize cookie token at most once - #23 part 3 - `Get[AndStore]Tokens()` would deserialize cookie token from request even if `IsRequestValidAsync()` already had - `GetAndStoreTokens()` serialized an old (never saved) cookie token once and a new one twice - refactor serialization from `DefaultAntiforgeryTokenStore` to `DefaultAntiforgery` - divide responsibilities and ease overall fix - above refactoring took `IAntiforgeryContextAccessor` responsibilities along to `DefaultAntiforgery` as well - store all tokens in `IAntiforgeryContextAccessor` to avoid repeated (de)serializations - remove `AntiforgeryTokenSetInternal` nits: - bit more parameter renaming to `httpContext` - remove argument checks in helper methods - did _not_ do a sweep through the repo; just files in this PR --- .../Internal/AntiforgeryContext.cs | 23 +- .../Internal/DefaultAntiforgery.cs | 237 ++++++--- .../Internal/DefaultAntiforgeryTokenStore.cs | 57 +- .../Internal/IAntiforgeryTokenStore.cs | 4 +- .../Internal/DefaultAntiforgeryTest.cs | 491 ++++++++++++++++-- .../DefaultAntiforgeryTokenStoreTest.cs | 149 +----- 6 files changed, 660 insertions(+), 301 deletions(-) diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryContext.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryContext.cs index f1abeefe92..d3f81c694d 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryContext.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/AntiforgeryContext.cs @@ -4,10 +4,31 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal { /// - /// Used as a per request state. + /// Used to hold per-request state. /// public class AntiforgeryContext { + public bool HaveDeserializedCookieToken { get; set; } + public AntiforgeryToken CookieToken { get; set; } + + public bool HaveDeserializedRequestToken { get; set; } + + public AntiforgeryToken RequestToken { get; set; } + + public bool HaveGeneratedNewCookieToken { get; set; } + + // After HaveGeneratedNewCookieToken is true, remains null if CookieToken is valid. + public AntiforgeryToken NewCookieToken { get; set; } + + // After HaveGeneratedNewCookieToken is true, remains null if CookieToken is valid. + public string NewCookieTokenString { get; set; } + + public AntiforgeryToken NewRequestToken { get; set; } + + public string NewRequestTokenString { get; set; } + + // Always false if NewCookieToken is null. Never store null cookie token or re-store cookie token from request. + public bool HaveStoredNewCookieToken { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs index db4d29e62f..6356d3f6d7 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs @@ -5,6 +5,7 @@ using System; using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -46,10 +47,17 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CheckSSLConfig(httpContext); - var tokenSet = GetTokensInternal(httpContext); - if (tokenSet.IsNewCookieToken) + var antiforgeryContext = GetTokensInternal(httpContext); + var tokenSet = Serialize(antiforgeryContext); + + if (!antiforgeryContext.HaveStoredNewCookieToken && antiforgeryContext.NewCookieToken != null) { - SaveCookieTokenAndHeader(httpContext, tokenSet.CookieToken); + // Serialize handles the new cookie token string. + Debug.Assert(antiforgeryContext.NewCookieTokenString != null); + + SaveCookieTokenAndHeader(httpContext, antiforgeryContext.NewCookieTokenString); + antiforgeryContext.HaveStoredNewCookieToken = true; + _logger.NewCookieToken(); } else @@ -57,7 +65,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal _logger.ReusedCookieToken(); } - return Serialize(tokenSet); + return tokenSet; } /// @@ -70,8 +78,8 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CheckSSLConfig(httpContext); - var tokenSet = GetTokensInternal(httpContext); - return Serialize(tokenSet); + var antiforgeryContext = GetTokensInternal(httpContext); + return Serialize(antiforgeryContext); } /// @@ -90,6 +98,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal _logger.MissingCookieToken(_options.CookieName); return false; } + if (tokens.RequestToken == null) { _logger.MissingRequestToken(_options.FormFieldName, _options.HeaderName); @@ -97,8 +106,9 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } // Extract cookie & request tokens - var deserializedCookieToken = _tokenSerializer.Deserialize(tokens.CookieToken); - var deserializedRequestToken = _tokenSerializer.Deserialize(tokens.RequestToken); + AntiforgeryToken deserializedCookieToken; + AntiforgeryToken deserializedRequestToken; + DeserializeTokens(httpContext, tokens, out deserializedCookieToken, out deserializedRequestToken); // Validate string message; @@ -165,30 +175,17 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal private void ValidateTokens(HttpContext httpContext, AntiforgeryTokenSet antiforgeryTokenSet) { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } - - CheckSSLConfig(httpContext); - - if (string.IsNullOrEmpty(antiforgeryTokenSet.CookieToken)) - { - throw new ArgumentException( - Resources.Antiforgery_CookieToken_MustBeProvided_Generic, - nameof(antiforgeryTokenSet)); - } - - if (string.IsNullOrEmpty(antiforgeryTokenSet.RequestToken)) - { - throw new ArgumentException( - Resources.Antiforgery_RequestToken_MustBeProvided_Generic, - nameof(antiforgeryTokenSet)); - } + Debug.Assert(!string.IsNullOrEmpty(antiforgeryTokenSet.CookieToken)); + Debug.Assert(!string.IsNullOrEmpty(antiforgeryTokenSet.RequestToken)); // Extract cookie & request tokens - var deserializedCookieToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.CookieToken); - var deserializedRequestToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.RequestToken); + AntiforgeryToken deserializedCookieToken; + AntiforgeryToken deserializedRequestToken; + DeserializeTokens( + httpContext, + antiforgeryTokenSet, + out deserializedCookieToken, + out deserializedRequestToken); // Validate string message; @@ -212,38 +209,26 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CheckSSLConfig(httpContext); - var cookieToken = GetCookieTokenDoesNotThrow(httpContext); - cookieToken = ValidateAndGenerateNewCookieToken(cookieToken); - SaveCookieTokenAndHeader(httpContext, cookieToken); + var antiforgeryContext = GetCookieTokens(httpContext); + if (!antiforgeryContext.HaveStoredNewCookieToken && antiforgeryContext.NewCookieToken != null) + { + if (antiforgeryContext.NewCookieTokenString == null) + { + antiforgeryContext.NewCookieTokenString = + _tokenSerializer.Serialize(antiforgeryContext.NewCookieToken); + } + + SaveCookieTokenAndHeader(httpContext, antiforgeryContext.NewCookieTokenString); + antiforgeryContext.HaveStoredNewCookieToken = true; + } } - // This method returns null if oldCookieToken is valid. - private AntiforgeryToken ValidateAndGenerateNewCookieToken(AntiforgeryToken cookieToken) + private void SaveCookieTokenAndHeader(HttpContext httpContext, string cookieToken) { - if (!_tokenGenerator.IsCookieTokenValid(cookieToken)) - { - // Need to make sure we're always operating with a good cookie token. - var newCookieToken = _tokenGenerator.GenerateCookieToken(); - Debug.Assert(_tokenGenerator.IsCookieTokenValid(newCookieToken)); - return newCookieToken; - } - - return null; - } - - private void SaveCookieTokenAndHeader( - HttpContext context, - AntiforgeryToken cookieToken) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - if (cookieToken != null) { // Persist the new cookie if it is not null. - _tokenStore.SaveCookieToken(context, cookieToken); + _tokenStore.SaveCookieToken(httpContext, cookieToken); } if (!_options.SuppressXFrameOptionsHeader) @@ -251,7 +236,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Adding X-Frame-Options header to prevent ClickJacking. See // http://tools.ietf.org/html/draft-ietf-websec-x-frame-options-10 // for more information. - context.Response.Headers["X-Frame-Options"] = "SAMEORIGIN"; + httpContext.Response.Headers["X-Frame-Options"] = "SAMEORIGIN"; } } @@ -266,11 +251,64 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } } - private AntiforgeryToken GetCookieTokenDoesNotThrow(HttpContext context) + private AntiforgeryContext GetCookieTokens(HttpContext httpContext) + { + var services = httpContext.RequestServices; + var contextAccessor = services.GetRequiredService(); + if (contextAccessor.Value == null) + { + contextAccessor.Value = new AntiforgeryContext(); + } + + var antiforgeryContext = contextAccessor.Value; + if (antiforgeryContext.HaveGeneratedNewCookieToken) + { + Debug.Assert(antiforgeryContext.HaveDeserializedCookieToken); + + // Have executed this method earlier in the context of this request. + return antiforgeryContext; + } + + AntiforgeryToken cookieToken; + if (antiforgeryContext.HaveDeserializedCookieToken) + { + cookieToken = antiforgeryContext.CookieToken; + } + else + { + cookieToken = GetCookieTokenDoesNotThrow(httpContext); + + antiforgeryContext.CookieToken = cookieToken; + antiforgeryContext.HaveDeserializedCookieToken = true; + } + + AntiforgeryToken newCookieToken; + if (_tokenGenerator.IsCookieTokenValid(cookieToken)) + { + // No need for the cookie token from the request after it has been verified. + newCookieToken = null; + } + else + { + // Need to make sure we're always operating with a good cookie token. + newCookieToken = _tokenGenerator.GenerateCookieToken(); + Debug.Assert(_tokenGenerator.IsCookieTokenValid(newCookieToken)); + } + + antiforgeryContext.HaveGeneratedNewCookieToken = true; + antiforgeryContext.NewCookieToken = newCookieToken; + + return antiforgeryContext; + } + + private AntiforgeryToken GetCookieTokenDoesNotThrow(HttpContext httpContext) { try { - return _tokenStore.GetCookieToken(context); + var serializedToken = _tokenStore.GetCookieToken(httpContext); + var token = _tokenSerializer.Deserialize(serializedToken); + + return token; } catch { @@ -279,43 +317,80 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } } - private AntiforgeryTokenSetInternal GetTokensInternal(HttpContext httpContext) + private AntiforgeryContext GetTokensInternal(HttpContext httpContext) { - var cookieToken = GetCookieTokenDoesNotThrow(httpContext); - var newCookieToken = ValidateAndGenerateNewCookieToken(cookieToken); - if (newCookieToken != null) + var antiforgeryContext = GetCookieTokens(httpContext); + if (antiforgeryContext.NewRequestToken == null) { - cookieToken = newCookieToken; + var cookieToken = antiforgeryContext.NewCookieToken ?? antiforgeryContext.CookieToken; + antiforgeryContext.NewRequestToken = _tokenGenerator.GenerateRequestToken(httpContext, cookieToken); } - var requestToken = _tokenGenerator.GenerateRequestToken( - httpContext, - cookieToken); - return new AntiforgeryTokenSetInternal() - { - // Note : The new cookie would be null if the old cookie is valid. - CookieToken = cookieToken, - RequestToken = requestToken, - IsNewCookieToken = newCookieToken != null - }; + return antiforgeryContext; } - private AntiforgeryTokenSet Serialize(AntiforgeryTokenSetInternal tokenSet) + private AntiforgeryTokenSet Serialize(AntiforgeryContext antiforgeryContext) { + // Should only be called after new tokens have been generated. + Debug.Assert(antiforgeryContext.HaveGeneratedNewCookieToken); + Debug.Assert(antiforgeryContext.NewRequestToken != null); + + if (antiforgeryContext.NewRequestTokenString == null) + { + antiforgeryContext.NewRequestTokenString = + _tokenSerializer.Serialize(antiforgeryContext.NewRequestToken); + } + + if (antiforgeryContext.NewCookieTokenString == null && antiforgeryContext.NewCookieToken != null) + { + antiforgeryContext.NewCookieTokenString = + _tokenSerializer.Serialize(antiforgeryContext.NewCookieToken); + } + return new AntiforgeryTokenSet( - _tokenSerializer.Serialize(tokenSet.RequestToken), - _tokenSerializer.Serialize(tokenSet.CookieToken), + antiforgeryContext.NewRequestTokenString, + antiforgeryContext.NewCookieTokenString, _options.FormFieldName, _options.HeaderName); } - private class AntiforgeryTokenSetInternal + private void DeserializeTokens( + HttpContext httpContext, + AntiforgeryTokenSet antiforgeryTokenSet, + out AntiforgeryToken cookieToken, + out AntiforgeryToken requestToken) { - public AntiforgeryToken RequestToken { get; set; } + var services = httpContext.RequestServices; + var contextAccessor = services.GetRequiredService(); + if (contextAccessor.Value == null) + { + contextAccessor.Value = new AntiforgeryContext(); + } - public AntiforgeryToken CookieToken { get; set; } + var antiforgeryContext = contextAccessor.Value; + if (antiforgeryContext.HaveDeserializedCookieToken) + { + cookieToken = antiforgeryContext.CookieToken; + } + else + { + cookieToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.CookieToken); - public bool IsNewCookieToken { get; set; } + antiforgeryContext.CookieToken = cookieToken; + antiforgeryContext.HaveDeserializedCookieToken = true; + } + + if (antiforgeryContext.HaveDeserializedRequestToken) + { + requestToken = antiforgeryContext.RequestToken; + } + else + { + requestToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.RequestToken); + + antiforgeryContext.RequestToken = requestToken; + antiforgeryContext.HaveDeserializedRequestToken = true; + } } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs index 5b0f41426a..cd20eea956 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs @@ -5,7 +5,6 @@ using System; using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -14,39 +13,20 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal public class DefaultAntiforgeryTokenStore : IAntiforgeryTokenStore { private readonly AntiforgeryOptions _options; - private readonly IAntiforgeryTokenSerializer _tokenSerializer; - public DefaultAntiforgeryTokenStore( - IOptions optionsAccessor, - IAntiforgeryTokenSerializer tokenSerializer) + public DefaultAntiforgeryTokenStore(IOptions optionsAccessor) { if (optionsAccessor == null) { throw new ArgumentNullException(nameof(optionsAccessor)); } - if (tokenSerializer == null) - { - throw new ArgumentNullException(nameof(tokenSerializer)); - } - _options = optionsAccessor.Value; - _tokenSerializer = tokenSerializer; } - public AntiforgeryToken GetCookieToken(HttpContext httpContext) + public string GetCookieToken(HttpContext httpContext) { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } - - var services = httpContext.RequestServices; - var contextAccessor = services.GetRequiredService(); - if (contextAccessor.Value != null) - { - return contextAccessor.Value.CookieToken; - } + Debug.Assert(httpContext != null); var requestCookie = httpContext.Request.Cookies[_options.CookieName]; if (string.IsNullOrEmpty(requestCookie)) @@ -55,15 +35,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal return null; } - return _tokenSerializer.Deserialize(requestCookie); + return requestCookie; } public async Task GetRequestTokensAsync(HttpContext httpContext) { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } + Debug.Assert(httpContext != null); var cookieToken = httpContext.Request.Cookies[_options.CookieName]; @@ -85,27 +62,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal return new AntiforgeryTokenSet(requestToken, cookieToken, _options.FormFieldName, _options.HeaderName); } - public void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token) + public void SaveCookieToken(HttpContext httpContext, string token) { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } + Debug.Assert(httpContext != null); + Debug.Assert(token != null); - if (token == null) - { - throw new ArgumentNullException(nameof(token)); - } - - // Add the cookie to the request based context. - // This is useful if the cookie needs to be reloaded in the context of the same request. - - var services = httpContext.RequestServices; - var contextAccessor = services.GetRequiredService(); - Debug.Assert(contextAccessor.Value == null, "AntiforgeryContext should be set only once per request."); - contextAccessor.Value = new AntiforgeryContext() { CookieToken = token }; - - var serializedToken = _tokenSerializer.Serialize(token); var options = new CookieOptions() { HttpOnly = true }; // Note: don't use "newCookie.Secure = _options.RequireSSL;" since the default @@ -115,7 +76,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal options.Secure = true; } - httpContext.Response.Cookies.Append(_options.CookieName, serializedToken, options); + httpContext.Response.Cookies.Append(_options.CookieName, token, options); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs index 967887a44f..1b4aa8ec05 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal { public interface IAntiforgeryTokenStore { - AntiforgeryToken GetCookieToken(HttpContext httpContext); + string GetCookieToken(HttpContext httpContext); /// /// Gets the cookie and request tokens from the request. @@ -17,6 +17,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal /// The . Task GetRequestTokensAsync(HttpContext httpContext); - void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token); + void SaveCookieToken(HttpContext httpContext, string token); } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs index 8969b05c31..d1571fc59b 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs @@ -125,19 +125,30 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal public void GetTokens_ExistingInvalidCookieToken_GeneratesANewCookieTokenAndANewFormToken() { // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor(); // Generate a new cookie. var context = CreateMockContext( new AntiforgeryOptions(), useOldCookie: false, - isOldCookieValid: false); + isOldCookieValid: false, + contextAccessor: contextAccessor); var antiforgery = GetAntiforgery(context); // Act var tokenset = antiforgery.GetTokens(context.HttpContext); // Assert - Assert.Equal("serialized-new-cookie-token", tokenset.CookieToken); - Assert.Equal("serialized-form-token", tokenset.RequestToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, tokenset.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenset.RequestToken); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveGeneratedNewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieToken, contextAccessor.Value.NewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, contextAccessor.Value.NewCookieTokenString); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.NewRequestToken); + Assert.Equal(context.TestTokenSet.FormTokenString, contextAccessor.Value.NewRequestTokenString); } [Fact] @@ -150,10 +161,13 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal useOldCookie: false, isOldCookieValid: false); - // This will cause the cookieToken to be null. + // Exception will cause the cookieToken to be null. context.TokenSerializer - .Setup(o => o.Deserialize("serialized-old-cookie-token")) + .Setup(o => o.Deserialize(context.TestTokenSet.OldCookieTokenString)) .Throws(new Exception("should be swallowed")); + context.TokenGenerator + .Setup(o => o.IsCookieTokenValid(null)) + .Returns(false); var antiforgery = GetAntiforgery(context); @@ -161,36 +175,88 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal var tokenset = antiforgery.GetTokens(context.HttpContext); // Assert - Assert.Equal("serialized-new-cookie-token", tokenset.CookieToken); - Assert.Equal("serialized-form-token", tokenset.RequestToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, tokenset.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenset.RequestToken); } [Fact] public void GetTokens_ExistingValidCookieToken_GeneratesANewFormToken() { // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor(); var context = CreateMockContext( new AntiforgeryOptions(), useOldCookie: true, - isOldCookieValid: true); + isOldCookieValid: true, + contextAccessor: contextAccessor); var antiforgery = GetAntiforgery(context); // Act var tokenset = antiforgery.GetTokens(context.HttpContext); // Assert - Assert.Equal("serialized-old-cookie-token", tokenset.CookieToken); - Assert.Equal("serialized-form-token", tokenset.RequestToken); + Assert.Null(tokenset.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenset.RequestToken); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveGeneratedNewCookieToken); + Assert.Null(contextAccessor.Value.NewCookieToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.NewRequestToken); + Assert.Equal(context.TestTokenSet.FormTokenString, contextAccessor.Value.NewRequestTokenString); + } + + [Fact] + public void GetTokens_DoesNotSerializeTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + HaveGeneratedNewCookieToken = true, + NewRequestToken = new AntiforgeryToken(), + NewRequestTokenString = "serialized-form-token-from-context", + }, + }; + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + contextAccessor: contextAccessor); + + var antiforgery = GetAntiforgery(context); + + // Act + var tokenset = antiforgery.GetTokens(context.HttpContext); + + // Assert + Assert.Null(tokenset.CookieToken); + Assert.Equal("serialized-form-token-from-context", tokenset.RequestToken); + + Assert.Null(contextAccessor.Value.NewCookieToken); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); } [Fact] public void GetAndStoreTokens_ExistingValidCookieToken_NotOverriden() { // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor(); var context = CreateMockContext( new AntiforgeryOptions(), useOldCookie: true, - isOldCookieValid: true); + isOldCookieValid: true, + contextAccessor: contextAccessor); var antiforgery = GetAntiforgery(context); // Act @@ -199,20 +265,31 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert // We shouldn't have saved the cookie because it already existed. context.TokenStore.Verify( - t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Never); + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), + Times.Never); - Assert.Equal("serialized-old-cookie-token", tokenSet.CookieToken); - Assert.Equal("serialized-form-token", tokenSet.RequestToken); + Assert.Null(tokenSet.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenSet.RequestToken); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveGeneratedNewCookieToken); + Assert.Null(contextAccessor.Value.NewCookieToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.NewRequestToken); + Assert.Equal(context.TestTokenSet.FormTokenString, contextAccessor.Value.NewRequestTokenString); } [Fact] public void GetAndStoreTokens_NoExistingCookieToken_Saved() { // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor(); var context = CreateMockContext( new AntiforgeryOptions(), useOldCookie: false, - isOldCookieValid: false); + isOldCookieValid: false, + contextAccessor: contextAccessor); var antiforgery = GetAntiforgery(context); // Act @@ -220,17 +297,125 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert context.TokenStore.Verify( - t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Once); + t => t.SaveCookieToken(It.IsAny(), context.TestTokenSet.NewCookieTokenString), + Times.Once); - Assert.Equal("serialized-new-cookie-token", tokenSet.CookieToken); - Assert.Equal("serialized-form-token", tokenSet.RequestToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, tokenSet.CookieToken); + Assert.Equal(context.TestTokenSet.FormTokenString, tokenSet.RequestToken); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveGeneratedNewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieToken, contextAccessor.Value.NewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, contextAccessor.Value.NewCookieTokenString); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.NewRequestToken); + Assert.Equal(context.TestTokenSet.FormTokenString, contextAccessor.Value.NewRequestTokenString); + Assert.True(contextAccessor.Value.HaveStoredNewCookieToken); + } + + [Fact] + public void GetAndStoreTokens_DoesNotSerializeTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + HaveGeneratedNewCookieToken = true, + NewCookieToken = new AntiforgeryToken(), + NewCookieTokenString = "serialized-cookie-token-from-context", + NewRequestToken = new AntiforgeryToken(), + NewRequestTokenString = "serialized-form-token-from-context", + }, + }; + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + contextAccessor: contextAccessor); + var antiforgery = GetAntiforgery(context); + + context.TokenStore + .Setup(t => t.SaveCookieToken(context.HttpContext, "serialized-cookie-token-from-context")) + .Verifiable(); + + // Act + var tokenset = antiforgery.GetAndStoreTokens(context.HttpContext); + + // Assert + // Token store used once, with expected arguments. + // Passed context's cookie token though request's cookie token was valid. + context.TokenStore.Verify( + t => t.SaveCookieToken(context.HttpContext, "serialized-cookie-token-from-context"), + Times.Once); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); + + Assert.Equal("serialized-cookie-token-from-context", tokenset.CookieToken); + Assert.Equal("serialized-form-token-from-context", tokenset.RequestToken); + + Assert.True(contextAccessor.Value.HaveStoredNewCookieToken); + } + + [Fact] + public void GetAndStoreTokens_DoesNotStoreTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + HaveGeneratedNewCookieToken = true, + HaveStoredNewCookieToken = true, + NewCookieToken = new AntiforgeryToken(), + NewCookieTokenString = "serialized-cookie-token-from-context", + NewRequestToken = new AntiforgeryToken(), + NewRequestTokenString = "serialized-form-token-from-context", + }, + }; + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + contextAccessor: contextAccessor); + var antiforgery = GetAntiforgery(context); + + // Act + var tokenset = antiforgery.GetAndStoreTokens(context.HttpContext); + + // Assert + // Token store not used. + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), + Times.Never); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); + + Assert.Equal("serialized-cookie-token-from-context", tokenset.CookieToken); + Assert.Equal("serialized-form-token-from-context", tokenset.RequestToken); } [Fact] public async Task IsRequestValidAsync_FromStore_Failure() { // Arrange - var context = CreateMockContext(new AntiforgeryOptions()); + var contextAccessor = new DefaultAntiforgeryContextAccessor(); + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); string message; context.TokenGenerator @@ -249,13 +434,21 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert Assert.False(result); context.TokenGenerator.Verify(); + + // Failed _after_ updating the AntiforgeryContext. + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveDeserializedRequestToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.RequestToken); } [Fact] public async Task IsRequestValidAsync_FromStore_Success() { // Arrange - var context = CreateMockContext(new AntiforgeryOptions()); + var contextAccessor = new DefaultAntiforgeryContextAccessor(); + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); string message; context.TokenGenerator @@ -275,13 +468,64 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert Assert.True(result); context.TokenGenerator.Verify(); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveDeserializedRequestToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.RequestToken); + } + + [Fact] + public async Task IsRequestValidAsync_DoesNotDeserializeTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + CookieToken = new AntiforgeryToken(), + HaveDeserializedRequestToken = true, + RequestToken = new AntiforgeryToken(), + }, + }; + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); + + string message; + context.TokenGenerator + .Setup(o => o.TryValidateTokenSet( + context.HttpContext, + contextAccessor.Value.CookieToken, + contextAccessor.Value.RequestToken, + out message)) + .Returns(true) + .Verifiable(); + + var antiforgery = GetAntiforgery(context); + + // Act + var result = await antiforgery.IsRequestValidAsync(context.HttpContext); + + // Assert + Assert.True(result); + context.TokenGenerator.Verify(); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); } [Fact] public async Task ValidateRequestAsync_FromStore_Failure() { // Arrange - var context = CreateMockContext(new AntiforgeryOptions()); + var contextAccessor = new DefaultAntiforgeryContextAccessor(); + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); var message = "my-message"; context.TokenGenerator @@ -299,13 +543,21 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal () => antiforgery.ValidateRequestAsync(context.HttpContext)); Assert.Equal("my-message", exception.Message); context.TokenGenerator.Verify(); + + // Failed _after_ updating the AntiforgeryContext. + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveDeserializedRequestToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.RequestToken); } [Fact] public async Task ValidateRequestAsync_FromStore_Success() { // Arrange - var context = CreateMockContext(new AntiforgeryOptions()); + var contextAccessor = new DefaultAntiforgeryContextAccessor(); + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); string message; context.TokenGenerator @@ -324,6 +576,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert context.TokenGenerator.Verify(); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveDeserializedRequestToken); + Assert.Equal(context.TestTokenSet.RequestToken, contextAccessor.Value.RequestToken); } [Fact] @@ -429,6 +687,49 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal exception.Message); } + [Fact] + public async Task ValidateRequestAsync_DoesNotDeserializeTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + CookieToken = new AntiforgeryToken(), + HaveDeserializedRequestToken = true, + RequestToken = new AntiforgeryToken(), + }, + }; + var context = CreateMockContext(new AntiforgeryOptions(), contextAccessor: contextAccessor); + + string message; + context.TokenGenerator + .Setup(o => o.TryValidateTokenSet( + context.HttpContext, + contextAccessor.Value.CookieToken, + contextAccessor.Value.RequestToken, + out message)) + .Returns(true) + .Verifiable(); + + var antiforgery = GetAntiforgery(context); + + // Act + await antiforgery.ValidateRequestAsync(context.HttpContext); + + // Assert (does not throw) + context.TokenGenerator.Verify(); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); + } + [Theory] [InlineData(false, "SAMEORIGIN")] [InlineData(true, null)] @@ -441,9 +742,14 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal { SuppressXFrameOptionsHeader = suppressXFrameOptions }; + var contextAccessor = new DefaultAntiforgeryContextAccessor(); // Generate a new cookie. - var context = CreateMockContext(options, useOldCookie: false, isOldCookieValid: false); + var context = CreateMockContext( + options, + useOldCookie: false, + isOldCookieValid: false, + contextAccessor: contextAccessor); var antiforgery = GetAntiforgery(context); // Act @@ -452,6 +758,102 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal // Assert var xFrameOptions = context.HttpContext.Response.Headers["X-Frame-Options"]; Assert.Equal(expectedHeaderValue, xFrameOptions); + + Assert.NotNull(contextAccessor.Value); + Assert.True(contextAccessor.Value.HaveDeserializedCookieToken); + Assert.Equal(context.TestTokenSet.OldCookieToken, contextAccessor.Value.CookieToken); + Assert.True(contextAccessor.Value.HaveGeneratedNewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieToken, contextAccessor.Value.NewCookieToken); + Assert.Equal(context.TestTokenSet.NewCookieTokenString, contextAccessor.Value.NewCookieTokenString); + Assert.True(contextAccessor.Value.HaveStoredNewCookieToken); + } + + [Fact] + public void SetCookieTokenAndHeader_DoesNotDeserializeTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + HaveGeneratedNewCookieToken = true, + NewCookieToken = new AntiforgeryToken(), + NewCookieTokenString = "serialized-cookie-token-from-context", + NewRequestToken = new AntiforgeryToken(), + NewRequestTokenString = "serialized-form-token-from-context", + }, + }; + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + contextAccessor: contextAccessor); + var antiforgery = GetAntiforgery(context); + + context.TokenStore + .Setup(t => t.SaveCookieToken(context.HttpContext, "serialized-cookie-token-from-context")) + .Verifiable(); + + // Act + antiforgery.SetCookieTokenAndHeader(context.HttpContext); + + // Assert + // Token store used once, with expected arguments. + // Passed context's cookie token though request's cookie token was valid. + context.TokenStore.Verify( + t => t.SaveCookieToken(context.HttpContext, "serialized-cookie-token-from-context"), + Times.Once); + + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); + } + + [Fact] + public void SetCookieTokenAndHeader_DoesNotStoreTwice() + { + // Arrange + var contextAccessor = new DefaultAntiforgeryContextAccessor + { + Value = new AntiforgeryContext + { + HaveDeserializedCookieToken = true, + HaveGeneratedNewCookieToken = true, + HaveStoredNewCookieToken = true, + NewCookieToken = new AntiforgeryToken(), + NewCookieTokenString = "serialized-cookie-token-from-context", + NewRequestToken = new AntiforgeryToken(), + NewRequestTokenString = "serialized-form-token-from-context", + }, + }; + var context = CreateMockContext( + new AntiforgeryOptions(), + useOldCookie: true, + isOldCookieValid: true, + contextAccessor: contextAccessor); + var antiforgery = GetAntiforgery(context); + + // Act + antiforgery.SetCookieTokenAndHeader(context.HttpContext); + + // Assert + // Token serializer not used. + context.TokenSerializer.Verify( + o => o.Deserialize(It.IsAny()), + Times.Never); + context.TokenSerializer.Verify( + o => o.Serialize(It.IsAny()), + Times.Never); + + // Token store not used. + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), + Times.Never); } private DefaultAntiforgery GetAntiforgery( @@ -484,13 +886,20 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal return builder.BuildServiceProvider(); } - private HttpContext GetHttpContext() + private HttpContext GetHttpContext(IAntiforgeryContextAccessor contextAccessor) { var httpContext = new DefaultHttpContext(); httpContext.RequestServices = GetServices(); httpContext.User = new ClaimsPrincipal(new ClaimsIdentity("some-auth")); + + contextAccessor = contextAccessor ?? new DefaultAntiforgeryContextAccessor(); + + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(contextAccessor); + httpContext.RequestServices = serviceCollection.BuildServiceProvider(); + return httpContext; } @@ -509,24 +918,27 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal TestTokenSet testTokenSet, bool saveNewCookie = true) { - var oldCookieToken = testTokenSet.OldCookieToken; - var formToken = testTokenSet.RequestToken; + var oldCookieToken = testTokenSet.OldCookieTokenString; + var formToken = testTokenSet.FormTokenString; var mockTokenStore = new Mock(MockBehavior.Strict); - mockTokenStore.Setup(o => o.GetCookieToken(context)) - .Returns(oldCookieToken); + mockTokenStore + .Setup(o => o.GetCookieToken(context)) + .Returns(oldCookieToken); - mockTokenStore.Setup(o => o.GetRequestTokensAsync(context)) - .Returns(() => Task.FromResult(new AntiforgeryTokenSet( - testTokenSet.FormTokenString, - testTokenSet.OldCookieTokenString, - "form", - "header"))); + mockTokenStore + .Setup(o => o.GetRequestTokensAsync(context)) + .Returns(() => Task.FromResult(new AntiforgeryTokenSet( + formToken, + oldCookieToken, + "form", + "header"))); if (saveNewCookie) { - var newCookieToken = testTokenSet.NewCookieToken; - mockTokenStore.Setup(o => o.SaveCookieToken(context, newCookieToken)) - .Verifiable(); + var newCookieToken = testTokenSet.NewCookieTokenString; + mockTokenStore + .Setup(o => o.SaveCookieToken(context, newCookieToken)) + .Verifiable(); } return mockTokenStore; @@ -554,10 +966,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal private AntiforgeryMockContext CreateMockContext( AntiforgeryOptions options, bool useOldCookie = false, - bool isOldCookieValid = true) + bool isOldCookieValid = true, + IAntiforgeryContextAccessor contextAccessor = null) { // Arrange - var httpContext = GetHttpContext(); + var httpContext = GetHttpContext(contextAccessor); var testTokenSet = GetTokenSet(); var mockSerializer = GetTokenSerializer(testTokenSet); diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs index 16ad4857a1..2b1f6152a3 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs @@ -4,11 +4,8 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; -using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Primitives; using Moq; using Xunit; @@ -17,8 +14,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal { public class DefaultAntiforgeryTokenStoreTest { - private static readonly ObjectPool _pool = - new DefaultObjectPoolProvider().Create(new AntiforgerySerializationContextPooledObjectPolicy()); private readonly string _cookieName = "cookie-name"; [Fact] @@ -31,9 +26,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CookieName = _cookieName }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var token = tokenStore.GetCookieToken(httpContext); @@ -42,33 +35,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal Assert.Null(token); } - [Fact] - public void GetCookieToken_CookieIsMissingInRequest_LooksUpCookieInAntiforgeryContext() - { - // Arrange - var contextAccessor = new DefaultAntiforgeryContextAccessor(); - var httpContext = GetHttpContext(_cookieName, string.Empty, contextAccessor); - - // add a cookie explicitly. - var cookie = new AntiforgeryToken(); - contextAccessor.Value = new AntiforgeryContext() { CookieToken = cookie }; - - var options = new AntiforgeryOptions - { - CookieName = _cookieName - }; - - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); - - // Act - var token = tokenStore.GetCookieToken(httpContext); - - // Assert - Assert.Equal(cookie, token); - } - [Fact] public void GetCookieToken_CookieIsEmpty_ReturnsNull() { @@ -79,9 +45,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CookieName = _cookieName }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var token = tokenStore.GetCookieToken(httpContext); @@ -91,57 +55,24 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public void GetCookieToken_CookieIsInvalid_PropagatesException() + public void GetCookieToken_CookieIsNotEmpty_ReturnsToken() { // Arrange - var httpContext = GetHttpContext(_cookieName, "invalid-value"); - - var expectedException = new AntiforgeryValidationException("some exception"); - var mockSerializer = new Mock(); - mockSerializer - .Setup(o => o.Deserialize("invalid-value")) - .Throws(expectedException); + var expectedToken = "valid-value"; + var httpContext = GetHttpContext(_cookieName, expectedToken); var options = new AntiforgeryOptions() { CookieName = _cookieName }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: mockSerializer.Object); - - // Act & assert - var ex = Assert.Throws(() => tokenStore.GetCookieToken(httpContext)); - Assert.Same(expectedException, ex); - } - - [Fact] - public void GetCookieToken_CookieIsValid_ReturnsToken() - { - // Arrange - var expectedToken = new AntiforgeryToken(); - var httpContext = GetHttpContext(_cookieName, "valid-value"); - - var mockSerializer = new Mock(); - mockSerializer - .Setup(o => o.Deserialize("valid-value")) - .Returns(expectedToken); - - var options = new AntiforgeryOptions() - { - CookieName = _cookieName - }; - - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: mockSerializer.Object); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var token = tokenStore.GetCookieToken(httpContext); // Assert - Assert.Same(expectedToken, token); + Assert.Equal(expectedToken, token); } [Fact] @@ -157,9 +88,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal FormFieldName = "form-field-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); @@ -186,9 +115,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = null, }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); @@ -214,9 +141,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = "header-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokens = await tokenStore.GetRequestTokensAsync(httpContext); @@ -244,9 +169,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = "header-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokens = await tokenStore.GetRequestTokensAsync(httpContext); @@ -273,9 +196,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = "header-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); @@ -300,9 +221,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = "header-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); @@ -331,9 +250,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal HeaderName = "header-name", }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: Mock.Of()); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act var tokens = await tokenStore.GetRequestTokensAsync(httpContext); @@ -349,7 +266,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal public void SaveCookieToken(bool requireSsl, bool? expectedCookieSecureFlag) { // Arrange - var token = new AntiforgeryToken(); + var token = "serialized-value"; bool defaultCookieSecureValue = expectedCookieSecureFlag ?? false; // pulled from config; set by ctor var cookies = new MockResponseCookieCollection(); @@ -358,32 +275,19 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal .Setup(o => o.Response.Cookies) .Returns(cookies); - var contextAccessor = new DefaultAntiforgeryContextAccessor(); - mockHttpContext - .SetupGet(o => o.RequestServices) - .Returns(GetServiceProvider(contextAccessor)); - - var mockSerializer = new Mock(); - mockSerializer - .Setup(o => o.Serialize(token)) - .Returns("serialized-value"); - var options = new AntiforgeryOptions() { CookieName = _cookieName, RequireSsl = requireSsl }; - var tokenStore = new DefaultAntiforgeryTokenStore( - optionsAccessor: new TestOptionsManager(options), - tokenSerializer: mockSerializer.Object); + var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act tokenStore.SaveCookieToken(mockHttpContext.Object, token); // Assert Assert.Equal(1, cookies.Count); - Assert.NotNull(contextAccessor.Value.CookieToken); Assert.NotNull(cookies); Assert.Equal(_cookieName, cookies.Key); Assert.Equal("serialized-value", cookies.Value); @@ -391,39 +295,24 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal Assert.Equal(defaultCookieSecureValue, cookies.Options.Secure); } - private HttpContext GetHttpContext( - string cookieName, - string cookieValue, - IAntiforgeryContextAccessor contextAccessor = null) + private HttpContext GetHttpContext(string cookieName, string cookieValue) { var cookies = new RequestCookieCollection(new Dictionary { { cookieName, cookieValue }, }); - return GetHttpContext(cookies, contextAccessor); + return GetHttpContext(cookies); } - private HttpContext GetHttpContext( - IRequestCookieCollection cookies, - IAntiforgeryContextAccessor contextAccessor = null) + private HttpContext GetHttpContext(IRequestCookieCollection cookies) { var httpContext = new DefaultHttpContext(); httpContext.Request.Cookies = cookies; - contextAccessor = contextAccessor ?? new DefaultAntiforgeryContextAccessor(); - httpContext.RequestServices = GetServiceProvider(contextAccessor); - return httpContext; } - private static IServiceProvider GetServiceProvider(IAntiforgeryContextAccessor contextAccessor) - { - var serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(contextAccessor); - return serviceCollection.BuildServiceProvider(); - } - private class MockResponseCookieCollection : IResponseCookies { public string Key { get; set; }