From 20140c4c152cf25b1d71a6991bf8fb6f3d022ac6 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 2 Feb 2016 10:37:39 -0800 Subject: [PATCH] Adds a IsRequestValidAsync method Some other misc cleanup - docs for IAntiforgeryTokenGenerator - Add HttpContext parameter where to all IAntiforgeryGenerator methods - rename parameters on DefaultAntiforgery --- .../DefaultAntiforgery.cs | 106 +++--- .../DefaultAntiforgeryTokenGenerator.cs | 43 ++- .../IAntiforgery.cs | 37 ++- .../IAntiforgeryTokenGenerator.cs | 40 ++- .../Properties/Resources.Designer.cs | 8 + .../Resources.resx | 3 + .../DefaultAntiforgeryTest.cs | 194 ++++++++--- .../DefaultAntiforgeryTokenGeneratorTest.cs | 308 +++++++++++------- .../project.json | 1 + 9 files changed, 510 insertions(+), 230 deletions(-) diff --git a/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgery.cs index c4a10c2955..2f602fb077 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgery.cs @@ -36,75 +36,100 @@ namespace Microsoft.AspNetCore.Antiforgery } /// - public IHtmlContent GetHtml(HttpContext context) + public IHtmlContent GetHtml(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); - var tokenSet = GetAndStoreTokens(context); + var tokenSet = GetAndStoreTokens(httpContext); return new InputContent(_options.FormFieldName, tokenSet.RequestToken); } /// - public AntiforgeryTokenSet GetAndStoreTokens(HttpContext context) + public AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); - var tokenSet = GetTokensInternal(context); + var tokenSet = GetTokensInternal(httpContext); if (tokenSet.IsNewCookieToken) { - SaveCookieTokenAndHeader(context, tokenSet.CookieToken); + SaveCookieTokenAndHeader(httpContext, tokenSet.CookieToken); } return Serialize(tokenSet); } /// - public AntiforgeryTokenSet GetTokens(HttpContext context) + public AntiforgeryTokenSet GetTokens(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); - var tokenSet = GetTokensInternal(context); + var tokenSet = GetTokensInternal(httpContext); return Serialize(tokenSet); } /// - public async Task ValidateRequestAsync(HttpContext context) + public async Task IsRequestValidAsync(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); - var tokens = await _tokenStore.GetRequestTokensAsync(context); - ValidateTokens(context, tokens); + var tokens = await _tokenStore.GetRequestTokensAsync(httpContext); + + // Extract cookie & request tokens + var deserializedCookieToken = _tokenSerializer.Deserialize(tokens.CookieToken); + var deserializedRequestToken = _tokenSerializer.Deserialize(tokens.RequestToken); + + // Validate + string message; + return _tokenGenerator.TryValidateTokenSet( + httpContext, + deserializedCookieToken, + deserializedRequestToken, + out message); } /// - public void ValidateTokens(HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet) + public async Task ValidateRequestAsync(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); + + var tokens = await _tokenStore.GetRequestTokensAsync(httpContext); + ValidateTokens(httpContext, tokens); + } + + /// + public void ValidateTokens(HttpContext httpContext, AntiforgeryTokenSet antiforgeryTokenSet) + { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + + CheckSSLConfig(httpContext); if (string.IsNullOrEmpty(antiforgeryTokenSet.CookieToken)) { @@ -125,25 +150,30 @@ namespace Microsoft.AspNetCore.Antiforgery var deserializedRequestToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.RequestToken); // Validate - _tokenGenerator.ValidateTokens( - context, + string message; + if (!_tokenGenerator.TryValidateTokenSet( + httpContext, deserializedCookieToken, - deserializedRequestToken); + deserializedRequestToken, + out message)) + { + throw new AntiforgeryValidationException(message); + } } /// - public void SetCookieTokenAndHeader(HttpContext context) + public void SetCookieTokenAndHeader(HttpContext httpContext) { - if (context == null) + if (httpContext == null) { - throw new ArgumentNullException(nameof(context)); + throw new ArgumentNullException(nameof(httpContext)); } - CheckSSLConfig(context); + CheckSSLConfig(httpContext); - var cookieToken = GetCookieTokenDoesNotThrow(context); + var cookieToken = GetCookieTokenDoesNotThrow(httpContext); cookieToken = ValidateAndGenerateNewCookieToken(cookieToken); - SaveCookieTokenAndHeader(context, cookieToken); + SaveCookieTokenAndHeader(httpContext, cookieToken); } // This method returns null if oldCookieToken is valid. @@ -208,16 +238,16 @@ namespace Microsoft.AspNetCore.Antiforgery } } - private AntiforgeryTokenSetInternal GetTokensInternal(HttpContext context) + private AntiforgeryTokenSetInternal GetTokensInternal(HttpContext httpContext) { - var cookieToken = GetCookieTokenDoesNotThrow(context); + var cookieToken = GetCookieTokenDoesNotThrow(httpContext); var newCookieToken = ValidateAndGenerateNewCookieToken(cookieToken); if (newCookieToken != null) { cookieToken = newCookieToken; } var requestToken = _tokenGenerator.GenerateRequestToken( - context, + httpContext, cookieToken); return new AntiforgeryTokenSetInternal() diff --git a/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgeryTokenGenerator.cs index 27e82d9a96..36c01beff7 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/DefaultAntiforgeryTokenGenerator.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Security.Claims; using System.Security.Principal; using Microsoft.AspNetCore.Http; @@ -22,6 +21,7 @@ namespace Microsoft.AspNetCore.Antiforgery _additionalDataProvider = additionalDataProvider; } + /// public AntiforgeryToken GenerateCookieToken() { return new AntiforgeryToken() @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Antiforgery }; } + /// public AntiforgeryToken GenerateRequestToken( HttpContext httpContext, AntiforgeryToken cookieToken) @@ -40,7 +41,17 @@ namespace Microsoft.AspNetCore.Antiforgery throw new ArgumentNullException(nameof(httpContext)); } - Debug.Assert(IsCookieTokenValid(cookieToken)); + if (cookieToken == null) + { + throw new ArgumentNullException(nameof(cookieToken)); + } + + if (!IsCookieTokenValid(cookieToken)) + { + throw new ArgumentException( + Resources.Antiforgery_CookieToken_IsInvalid, + nameof(cookieToken)); + } var requestToken = new AntiforgeryToken() { @@ -87,15 +98,18 @@ namespace Microsoft.AspNetCore.Antiforgery return requestToken; } + /// public bool IsCookieTokenValid(AntiforgeryToken cookieToken) { - return (cookieToken != null && cookieToken.IsCookieToken); + return cookieToken != null && cookieToken.IsCookieToken; } - public void ValidateTokens( + /// + public bool TryValidateTokenSet( HttpContext httpContext, AntiforgeryToken cookieToken, - AntiforgeryToken requestToken) + AntiforgeryToken requestToken, + out string message) { if (httpContext == null) { @@ -119,13 +133,15 @@ namespace Microsoft.AspNetCore.Antiforgery // Do the tokens have the correct format? if (!cookieToken.IsCookieToken || requestToken.IsCookieToken) { - throw new AntiforgeryValidationException(Resources.AntiforgeryToken_TokensSwapped); + message = Resources.AntiforgeryToken_TokensSwapped; + return false; } // Are the security tokens embedded in each incoming token identical? if (!object.Equals(cookieToken.SecurityToken, requestToken.SecurityToken)) { - throw new AntiforgeryValidationException(Resources.AntiforgeryToken_SecurityTokenMismatch); + message = Resources.AntiforgeryToken_SecurityTokenMismatch; + return false; } // Is the incoming token meant for the current user? @@ -153,21 +169,26 @@ namespace Microsoft.AspNetCore.Antiforgery if (!comparer.Equals(requestToken.Username, currentUsername)) { - throw new AntiforgeryValidationException( - Resources.FormatAntiforgeryToken_UsernameMismatch(requestToken.Username, currentUsername)); + message = Resources.FormatAntiforgeryToken_UsernameMismatch(requestToken.Username, currentUsername); + return false; } if (!object.Equals(requestToken.ClaimUid, currentClaimUid)) { - throw new AntiforgeryValidationException(Resources.AntiforgeryToken_ClaimUidMismatch); + message = Resources.AntiforgeryToken_ClaimUidMismatch; + return false; } // Is the AdditionalData valid? if (_additionalDataProvider != null && !_additionalDataProvider.ValidateAdditionalData(httpContext, requestToken.AdditionalData)) { - throw new AntiforgeryValidationException(Resources.AntiforgeryToken_AdditionalDataCheckFailed); + message = Resources.AntiforgeryToken_AdditionalDataCheckFailed; + return false; } + + message = null; + return true; } private static BinaryBlob GetClaimUidBlob(string base64ClaimUid) diff --git a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs index ca2d8341ae..d2eac4c95a 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgery.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Antiforgery /// /// Generates an <input type="hidden"> element for an antiforgery token. /// - /// The associated with the current request. + /// The associated with the current request. /// /// A containing an <input type="hidden"> element. This element should be put /// inside a <form>. @@ -25,50 +25,63 @@ namespace Microsoft.AspNetCore.Antiforgery /// This method has a side effect: /// A response cookie is set if there is no valid cookie associated with the request. /// - IHtmlContent GetHtml(HttpContext context); + IHtmlContent GetHtml(HttpContext httpContext); /// /// Generates an for this request and stores the cookie token /// in the response. /// - /// The associated with the current request. + /// The associated with the current request. /// An with tokens for the response. /// /// This method has a side effect: /// A response cookie is set if there is no valid cookie associated with the request. /// - AntiforgeryTokenSet GetAndStoreTokens(HttpContext context); + AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext); /// /// Generates an for this request. /// - /// The associated with the current request. + /// The associated with the current request. /// /// Unlike , this method has no side effect. The caller /// is responsible for setting the response cookie and injecting the returned /// form token as appropriate. /// - AntiforgeryTokenSet GetTokens(HttpContext context); + AntiforgeryTokenSet GetTokens(HttpContext httpContext); + + /// + /// Asynchronously returns a value indicating whether the request contains a valid antiforgery token. + /// + /// The associated with the current request. + /// + /// A that, when completed, returns true if the request contains a + /// valid antiforgery token, otherwise returns false. + /// + Task IsRequestValidAsync(HttpContext httpContext); /// /// Validates an antiforgery token that was supplied as part of the request. /// - /// The associated with the current request. - Task ValidateRequestAsync(HttpContext context); + /// The associated with the current request. + /// + /// Thrown when the request does not include a valid antiforgery token. + /// + Task ValidateRequestAsync(HttpContext httpContext); /// /// Validates an for the current request. /// - /// The associated with the current request. + /// The associated with the current request. /// /// The (cookie and request token) for this request. /// - void ValidateTokens(HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet); + void ValidateTokens(HttpContext httpContext, AntiforgeryTokenSet antiforgeryTokenSet); /// /// Generates and stores an antiforgery cookie token if one is not available or not valid. /// - /// The associated with the current request. - void SetCookieTokenAndHeader(HttpContext context); + /// The associated with the current request. + void SetCookieTokenAndHeader(HttpContext httpContext); } } diff --git a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgeryTokenGenerator.cs index 79fc447a45..e615d8c933 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/IAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/IAntiforgeryTokenGenerator.cs @@ -10,23 +10,41 @@ namespace Microsoft.AspNetCore.Antiforgery /// public interface IAntiforgeryTokenGenerator { - // Generates a new random cookie token. + /// + /// Generates a new random cookie token. + /// + /// An . AntiforgeryToken GenerateCookieToken(); - // Given a cookie token, generates a corresponding request token. - // The incoming cookie token must be valid. - AntiforgeryToken GenerateRequestToken( - HttpContext httpContext, - AntiforgeryToken cookieToken); + /// + /// Generates a request token corresponding to . + /// + /// The associated with the current request. + /// A valid cookie token. + /// An . + AntiforgeryToken GenerateRequestToken(HttpContext httpContext, AntiforgeryToken cookieToken); - // Determines whether an existing cookie token is valid (well-formed). - // If it is not, the caller must call GenerateCookieToken() before calling GenerateFormToken(). + /// + /// Attempts to validate a cookie token. + /// + /// A valid cookie token. + /// true if the cookie token is valid, otherwise false. bool IsCookieTokenValid(AntiforgeryToken cookieToken); - // Validates a (cookie, request) token pair. - void ValidateTokens( + /// + /// Attempts to validate a cookie and request token set for the given . + /// + /// The associated with the current request. + /// A cookie token. + /// A request token. + /// + /// Will be set to the validation message if the tokens are invalid, otherwise null. + /// + /// true if the tokens are valid, otherwise false. + bool TryValidateTokenSet( HttpContext httpContext, AntiforgeryToken cookieToken, - AntiforgeryToken requestToken); + AntiforgeryToken requestToken, + out string message); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Antiforgery/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Antiforgery/Properties/Resources.Designer.cs index 87e05dabab..6b12221bd8 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Properties/Resources.Designer.cs @@ -138,6 +138,14 @@ namespace Microsoft.AspNetCore.Antiforgery return string.Format(CultureInfo.CurrentCulture, GetString("AntiforgeryWorker_RequireSSL"), p0, p1, p2); } + /// + /// The required antiforgery cookie "{0}" is not present. + /// + internal static string Antiforgery_CookieToken_IsInvalid + { + get { return GetString("Antiforgery_CookieToken_IsInvalid"); } + } + /// /// The required antiforgery cookie "{0}" is not present. /// diff --git a/src/Microsoft.AspNetCore.Antiforgery/Resources.resx b/src/Microsoft.AspNetCore.Antiforgery/Resources.resx index 8e84c4cac5..bda2f0f353 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Resources.resx +++ b/src/Microsoft.AspNetCore.Antiforgery/Resources.resx @@ -143,6 +143,9 @@ The antiforgery system has the configuration value {0}.{1} = {2}, but the current request is not an SSL request. 0 = nameof(AntiforgeryOptions), 1 = nameof(RequireSsl), 2 = bool.TrueString + + The antiforgery cookie token is invalid. + The required antiforgery cookie "{0}" is not present. diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTest.cs index ddb190a256..cba6a7dce4 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTest.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; +using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Options; using Microsoft.Extensions.WebEncoders.Testing; using Moq; @@ -34,9 +35,31 @@ namespace Microsoft.AspNetCore.Antiforgery var exception = await Assert.ThrowsAsync( async () => await antiforgery.ValidateRequestAsync(httpContext)); Assert.Equal( - @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + - "but the current request is not an SSL request.", - exception.Message); + @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + + "but the current request is not an SSL request.", + exception.Message); + } + + [Fact] + public async Task ChecksSSL_IsRequestValidAsync_Throws() + { + // Arrange + var httpContext = new DefaultHttpContext(); + + var options = new AntiforgeryOptions() + { + RequireSsl = true + }; + + var antiforgery = GetAntiforgery(options); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + async () => await antiforgery.IsRequestValidAsync(httpContext)); + Assert.Equal( + @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + + "but the current request is not an SSL request.", + exception.Message); } [Fact] @@ -56,9 +79,9 @@ namespace Microsoft.AspNetCore.Antiforgery var exception = Assert.Throws( () => antiforgery.ValidateTokens(httpContext, new AntiforgeryTokenSet("hello", "world"))); Assert.Equal( - @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + - "but the current request is not an SSL request.", - exception.Message); + @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + + "but the current request is not an SSL request.", + exception.Message); } [Fact] @@ -162,6 +185,11 @@ namespace Microsoft.AspNetCore.Antiforgery var antiforgery = GetAntiforgery(context); var encoder = new HtmlTestEncoder(); + // Setup so that the null cookie token returned is treated as invalid. + context.TokenGenerator + .Setup(o => o.IsCookieTokenValid(null)) + .Returns(false); + // Act var inputElement = antiforgery.GetHtml(context.HttpContext); @@ -197,7 +225,7 @@ namespace Microsoft.AspNetCore.Antiforgery .Setup(o => o.GetCookieToken(context.HttpContext)) .Throws(new Exception("should be swallowed")); - // Setup so that the null cookie token returned is treated as invalid. + // Setup so that the null cookie token returned is treated as invalid. context.TokenGenerator .Setup(o => o.IsCookieTokenValid(null)) .Returns(false); @@ -303,12 +331,10 @@ namespace Microsoft.AspNetCore.Antiforgery isOldCookieValid: false); // This will cause the cookieToken to be null. - context.TokenSerializer.Setup(o => o.Deserialize("serialized-old-cookie-token")) - .Throws(new Exception("should be swallowed")); + context.TokenSerializer + .Setup(o => o.Deserialize("serialized-old-cookie-token")) + .Throws(new Exception("should be swallowed")); - // Setup so that the null cookie token returned is treated as invalid. - context.TokenGenerator.Setup(o => o.IsCookieTokenValid(null)) - .Returns(false); var antiforgery = GetAntiforgery(context); // Act @@ -381,7 +407,7 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void ValidateTokens_FromInvalidStrings_Throws() + public void ValidateTokens_InvalidTokens_Throws() { // Arrange var context = CreateMockContext(new AntiforgeryOptions()); @@ -393,17 +419,20 @@ namespace Microsoft.AspNetCore.Antiforgery .Setup(o => o.Deserialize("form-token")) .Returns(context.TestTokenSet.RequestToken); - context.TokenGenerator - .Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, - context.TestTokenSet.RequestToken)) - .Throws(new InvalidOperationException("my-message")); - context.TokenStore = null; - var antiforgery = GetAntiforgery(context); + // You can't really do Moq with out-parameters :( + var tokenGenerator = new TestTokenGenerator() + { + Message = "my-message", + }; - // Act & assert - var exception = Assert.Throws( + var antiforgery = new DefaultAntiforgery( + new TestOptionsManager(), + tokenGenerator, + context.TokenSerializer.Object, + tokenStore: null); + + // Act & Assert + var exception = Assert.Throws( () => antiforgery.ValidateTokens( context.HttpContext, new AntiforgeryTokenSet("form-token", "cookie-token"))); @@ -423,11 +452,14 @@ namespace Microsoft.AspNetCore.Antiforgery .Setup(o => o.Deserialize("form-token")) .Returns(context.TestTokenSet.RequestToken); + string message; context.TokenGenerator - .Setup(o => o.ValidateTokens( + .Setup(o => o.TryValidateTokenSet( context.HttpContext, context.TestTokenSet.OldCookieToken, - context.TestTokenSet.RequestToken)) + context.TestTokenSet.RequestToken, + out message)) + .Returns(true) .Verifiable(); context.TokenStore = null; var antiforgery = GetAntiforgery(context); @@ -450,12 +482,60 @@ namespace Microsoft.AspNetCore.Antiforgery // Act - var exception = Assert.Throws( - () => antiforgery.ValidateTokens(context.HttpContext, tokenSet)); + ExceptionAssert.ThrowsArgument( + () => antiforgery.ValidateTokens(context.HttpContext, tokenSet), + "antiforgeryTokenSet", + "The required antiforgery cookie token must be provided."); + } + + [Fact] + public async Task IsRequestValueAsync_FromStore_Failure() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions()); + + string message; + context.TokenGenerator + .Setup(o => o.TryValidateTokenSet( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.RequestToken, + out message)) + .Returns(false); + + var antiforgery = GetAntiforgery(context); + + // Act + var result = await antiforgery.IsRequestValidAsync(context.HttpContext); // Assert - var trimmed = exception.Message.Substring(0, exception.Message.IndexOf(Environment.NewLine)); - Assert.Equal("The required antiforgery cookie token must be provided.", trimmed); + Assert.False(result); + } + + [Fact] + public async Task IsRequestValidAsync_FromStore_Success() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions()); + + string message; + context.TokenGenerator + .Setup(o => o.TryValidateTokenSet( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.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(); } [Fact] @@ -464,17 +544,20 @@ namespace Microsoft.AspNetCore.Antiforgery // Arrange var context = CreateMockContext(new AntiforgeryOptions()); - context.TokenGenerator - .Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, - context.TestTokenSet.RequestToken)) - .Throws(new InvalidOperationException("my-message")); + // You can't really do Moq with out-parameters :( + var tokenGenerator = new TestTokenGenerator() + { + Message = "my-message", + }; - var antiforgery = GetAntiforgery(context); + var antiforgery = new DefaultAntiforgery( + new TestOptionsManager(), + tokenGenerator, + context.TokenSerializer.Object, + context.TokenStore.Object); // Act & assert - var exception = await Assert.ThrowsAsync( + var exception = await Assert.ThrowsAsync( async () => await antiforgery.ValidateRequestAsync(context.HttpContext)); Assert.Equal("my-message", exception.Message); } @@ -485,11 +568,14 @@ namespace Microsoft.AspNetCore.Antiforgery // Arrange var context = CreateMockContext(new AntiforgeryOptions()); + string message; context.TokenGenerator - .Setup(o => o.ValidateTokens( + .Setup(o => o.TryValidateTokenSet( context.HttpContext, context.TestTokenSet.OldCookieToken, - context.TestTokenSet.RequestToken)) + context.TestTokenSet.RequestToken, + out message)) + .Returns(true) .Verifiable(); var antiforgery = GetAntiforgery(context); @@ -696,5 +782,35 @@ namespace Microsoft.AspNetCore.Antiforgery { public AntiforgeryOptions Value { get; set; } = new AntiforgeryOptions(); } + + private class TestTokenGenerator : IAntiforgeryTokenGenerator + { + public string Message { get; set; } + + public AntiforgeryToken GenerateCookieToken() + { + throw new NotImplementedException(); + } + + public AntiforgeryToken GenerateRequestToken(HttpContext httpContext, AntiforgeryToken cookieToken) + { + throw new NotImplementedException(); + } + + public bool IsCookieTokenValid(AntiforgeryToken cookieToken) + { + throw new NotImplementedException(); + } + + public bool TryValidateTokenSet( + HttpContext httpContext, + AntiforgeryToken cookieToken, + AntiforgeryToken requestToken, + out string message) + { + message = Message; + return false; + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs index fbbd6a50e4..f277504d0b 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs @@ -5,6 +5,7 @@ using System; using System.Security.Claims; using System.Security.Cryptography; using Microsoft.AspNetCore.Http.Internal; +using Microsoft.AspNetCore.Testing; using Moq; using Xunit; @@ -28,7 +29,27 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void GenerateFormToken_AnonymousUser() + public void GenerateRequestToken_InvalidCookieToken() + { + // Arrange + var cookieToken = new AntiforgeryToken() { IsCookieToken = false }; + var httpContext = new DefaultHttpContext(); + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + Assert.False(httpContext.User.Identity.IsAuthenticated); + + var tokenProvider = new DefaultAntiforgeryTokenGenerator( + claimUidExtractor: null, + additionalDataProvider: null); + + // Act & Assert + ExceptionAssert.ThrowsArgument( + () => tokenProvider.GenerateRequestToken(httpContext, cookieToken), + "cookieToken", + "The antiforgery cookie token is invalid."); + } + + [Fact] + public void GenerateRequestToken_AnonymousUser() { // Arrange var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; @@ -53,7 +74,7 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void GenerateFormToken_AuthenticatedWithoutUsernameAndNoAdditionalData_NoAdditionalData() + public void GenerateRequestToken_AuthenticatedWithoutUsernameAndNoAdditionalData_NoAdditionalData() { // Arrange var cookieToken = new AntiforgeryToken() @@ -87,7 +108,7 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void GenerateFormToken_AuthenticatedWithoutUsername_WithAdditionalData() + public void GenerateRequestToken_AuthenticatedWithoutUsername_WithAdditionalData() { // Arrange var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; @@ -118,7 +139,7 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void GenerateFormToken_ClaimsBasedIdentity() + public void GenerateRequestToken_ClaimsBasedIdentity() { // Arrange var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; @@ -156,7 +177,7 @@ namespace Microsoft.AspNetCore.Antiforgery } [Fact] - public void GenerateFormToken_RegularUserWithUsername() + public void GenerateRequestToken_RegularUserWithUsername() { // Arrange var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; @@ -202,10 +223,10 @@ namespace Microsoft.AspNetCore.Antiforgery additionalDataProvider: null); // Act - bool retVal = tokenProvider.IsCookieTokenValid(cookieToken); + var isValid = tokenProvider.IsCookieTokenValid(cookieToken); // Assert - Assert.False(retVal); + Assert.False(isValid); } [Fact] @@ -246,7 +267,7 @@ namespace Microsoft.AspNetCore.Antiforgery [Fact] - public void ValidateTokens_CookieTokenMissing() + public void TryValidateTokenSet_CookieTokenMissing() { // Arrange var httpContext = new DefaultHttpContext(); @@ -254,98 +275,126 @@ namespace Microsoft.AspNetCore.Antiforgery var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: null, - additionalDataProvider: null); - - // Act & assert - var ex = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, null, fieldtoken)); - - var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); - Assert.Equal(@"The required antiforgery cookie token must be provided.", trimmed); - } - - [Fact] - public void ValidateTokens_FieldTokenMissing() - { - // Arrange - var httpContext = new DefaultHttpContext(); - httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); - - var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; - - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: null, - additionalDataProvider: null); - - // Act & assert - var ex = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, null)); - - var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); - Assert.Equal("The required antiforgery request token must be provided.", trimmed); - } - - [Fact] - public void ValidateTokens_FieldAndCookieTokensSwapped() - { - // Arrange - var httpContext = new DefaultHttpContext(); - httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); - - var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; - var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; - - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - claimUidExtractor: null, - additionalDataProvider: null); - - // Act & assert - var ex1 = - Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, fieldtoken, fieldtoken)); - Assert.Equal( - "Validation of the provided antiforgery token failed. " + - @"The cookie token and the request token were swapped.", - ex1.Message); - - var ex2 = - Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, cookieToken)); - Assert.Equal( - "Validation of the provided antiforgery token failed. " + - @"The cookie token and the request token were swapped.", - ex2.Message); - } - - [Fact] - public void ValidateTokens_FieldAndCookieTokensHaveDifferentSecurityKeys() - { - // Arrange - var httpContext = new DefaultHttpContext(); - httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); - - var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; - var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; - var tokenProvider = new DefaultAntiforgeryTokenGenerator( claimUidExtractor: null, additionalDataProvider: null); // Act & Assert - var exception = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken)); - Assert.Equal( - @"The antiforgery cookie token and request token do not match.", - exception.Message); + string message; + var ex = Assert.Throws( + () => tokenProvider.TryValidateTokenSet(httpContext, null, fieldtoken, out message)); + + var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); + Assert.Equal(@"The required antiforgery cookie token must be provided.", trimmed); + } + + [Fact] + public void TryValidateTokenSet_FieldTokenMissing() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + + var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; + + var tokenProvider = new DefaultAntiforgeryTokenGenerator( + claimUidExtractor: null, + additionalDataProvider: null); + + + // Act & Assert + string message; + var ex = Assert.Throws( + () => tokenProvider.TryValidateTokenSet(httpContext, cookieToken, null, out message)); + + var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); + Assert.Equal("The required antiforgery request token must be provided.", trimmed); + } + + [Fact] + public void TryValidateTokenSet_FieldAndCookieTokensSwapped_FieldTokenDuplicated() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + + var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; + var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; + + var tokenProvider = new DefaultAntiforgeryTokenGenerator( + claimUidExtractor: null, + additionalDataProvider: null); + + string expectedMessage = + "Validation of the provided antiforgery token failed. " + + "The cookie token and the request token were swapped."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, fieldtoken, fieldtoken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); + } + + [Fact] + public void TryValidateTokenSet_FieldAndCookieTokensSwapped_CookieDuplicated() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + + var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; + var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; + + var tokenProvider = new DefaultAntiforgeryTokenGenerator( + claimUidExtractor: null, + additionalDataProvider: null); + + string expectedMessage = + "Validation of the provided antiforgery token failed. " + + "The cookie token and the request token were swapped."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, cookieToken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); + } + + [Fact] + public void TryValidateTokenSet_FieldAndCookieTokensHaveDifferentSecurityKeys() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity()); + + var cookieToken = new AntiforgeryToken() { IsCookieToken = true }; + var fieldtoken = new AntiforgeryToken() { IsCookieToken = false }; + + var tokenProvider = new DefaultAntiforgeryTokenGenerator( + claimUidExtractor: null, + additionalDataProvider: null); + + string expectedMessage = "The antiforgery cookie token and request token do not match."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); } [Theory] [InlineData("the-user", "the-other-user")] [InlineData("http://example.com/uri-casing", "http://example.com/URI-casing")] [InlineData("https://example.com/secure-uri-casing", "https://example.com/secure-URI-casing")] - public void ValidateTokens_UsernameMismatch(string identityUsername, string embeddedUsername) + public void TryValidateTokenSet_UsernameMismatch(string identityUsername, string embeddedUsername) { // Arrange var httpContext = new DefaultHttpContext(); @@ -368,17 +417,21 @@ namespace Microsoft.AspNetCore.Antiforgery claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); - // Act & Assert - var exception = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken)); - Assert.Equal( - @"The provided antiforgery token was meant for user """ + embeddedUsername + - @""", but the current user is """ + identityUsername + @""".", - exception.Message); + string expectedMessage = + $"The provided antiforgery token was meant for user \"{embeddedUsername}\", " + + $"but the current user is \"{identityUsername}\"."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); } [Fact] - public void ValidateTokens_ClaimUidMismatch() + public void TryValidateTokenSet_ClaimUidMismatch() { // Arrange var httpContext = new DefaultHttpContext(); @@ -402,16 +455,21 @@ namespace Microsoft.AspNetCore.Antiforgery claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); - // Act & assert - var exception = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken)); - Assert.Equal( - @"The provided antiforgery token was meant for a different claims-based user than the current user.", - exception.Message); + string expectedMessage = + "The provided antiforgery token was meant for a different " + + "claims-based user than the current user."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); } [Fact] - public void ValidateTokens_AdditionalDataRejected() + public void TryValidateTokenSet_AdditionalDataRejected() { // Arrange var httpContext = new DefaultHttpContext(); @@ -428,21 +486,27 @@ namespace Microsoft.AspNetCore.Antiforgery }; var mockAdditionalDataProvider = new Mock(); - mockAdditionalDataProvider.Setup(o => o.ValidateAdditionalData(httpContext, "some-additional-data")) - .Returns(false); + mockAdditionalDataProvider + .Setup(o => o.ValidateAdditionalData(httpContext, "some-additional-data")) + .Returns(false); var tokenProvider = new DefaultAntiforgeryTokenGenerator( claimUidExtractor: null, additionalDataProvider: mockAdditionalDataProvider.Object); - // Act & assert - var exception = Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken)); - Assert.Equal(@"The provided antiforgery token failed a custom data check.", exception.Message); + string expectedMessage = "The provided antiforgery token failed a custom data check."; + + // Act + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); + + // Assert + Assert.False(result); + Assert.Equal(expectedMessage, message); } [Fact] - public void ValidateTokens_Success_AnonymousUser() + public void TryValidateTokenSet_Success_AnonymousUser() { // Arrange var httpContext = new DefaultHttpContext(); @@ -467,14 +531,16 @@ namespace Microsoft.AspNetCore.Antiforgery additionalDataProvider: mockAdditionalDataProvider.Object); // Act - tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken); + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); // Assert - // Nothing to assert - if we got this far, success! + Assert.True(result); + Assert.Null(message); } [Fact] - public void ValidateTokens_Success_AuthenticatedUserWithUsername() + public void TryValidateTokenSet_Success_AuthenticatedUserWithUsername() { // Arrange var httpContext = new DefaultHttpContext(); @@ -499,14 +565,16 @@ namespace Microsoft.AspNetCore.Antiforgery additionalDataProvider: mockAdditionalDataProvider.Object); // Act - tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken); + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); // Assert - // Nothing to assert - if we got this far, success! + Assert.True(result); + Assert.Null(message); } [Fact] - public void ValidateTokens_Success_ClaimsBasedUser() + public void TryValidateTokenSet_Success_ClaimsBasedUser() { // Arrange var httpContext = new DefaultHttpContext(); @@ -530,10 +598,12 @@ namespace Microsoft.AspNetCore.Antiforgery additionalDataProvider: null); // Act - tokenProvider.ValidateTokens(httpContext, cookieToken, fieldtoken); + string message; + var result = tokenProvider.TryValidateTokenSet(httpContext, cookieToken, fieldtoken, out message); // Assert - // Nothing to assert - if we got this far, success! + Assert.True(result); + Assert.Null(message); } private static ClaimsIdentity GetAuthenticatedIdentity(string identityUsername) diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/project.json b/test/Microsoft.AspNetCore.Antiforgery.Test/project.json index fee3adecdb..caabfc7fd0 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/project.json +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/project.json @@ -6,6 +6,7 @@ "dependencies": { "Microsoft.AspNetCore.Antiforgery": "1.0.0-*", "Microsoft.AspNetCore.Http": "1.0.0-*", + "Microsoft.AspNetCore.Testing": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.0.0-*", "Microsoft.Extensions.WebEncoders": "1.0.0-*", "Microsoft.NETCore.Platforms": "1.0.1-*",