From 9bcecf39948ff9f7e5755b7972ec348af0eb900e Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 30 Jul 2015 15:04:50 -0700 Subject: [PATCH] Fix #5 - better error message for missing tokens This fix changes the model for error messaging in antiforgery. Now only the token store will report a detailed error message including the names of form field and cookie. Other components will give more generic errors and assume that this was handled by the token store. This way you still see an error if the user creates a token store that doesn't throw, but it's a generic error that doesn't give incorrect information. --- .../DefaultAntiforgery.cs | 46 +++----- .../DefaultAntiforgeryTokenGenerator.cs | 19 ++-- .../DefaultAntiforgeryTokenStore.cs | 22 ++-- .../IAntiforgeryTokenGenerator.cs | 10 +- .../IAntiforgeryTokenStore.cs | 15 ++- .../Properties/Resources.Designer.cs | 104 ++++++++++++------ .../Resources.resx | 20 ++-- .../DefaultAntiforgeryTest.cs | 69 ++++++++---- .../DefaultAntiforgeryTokenGeneratorTest.cs | 57 ++-------- .../DefaultAntiforgeryTokenStoreTest.cs | 88 +++++++-------- 10 files changed, 239 insertions(+), 211 deletions(-) diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs index 93bb6e855b..b4497ebfed 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs @@ -76,12 +76,8 @@ namespace Microsoft.AspNet.Antiforgery { CheckSSLConfig(context); - // Extract cookie & form tokens - var cookieToken = _tokenStore.GetCookieToken(context); - var formToken = await _tokenStore.GetFormTokenAsync(context); - - // Validate - _tokenGenerator.ValidateTokens(context, cookieToken, formToken); + var tokens = await _tokenStore.GetRequestTokensAsync(context); + ValidateTokens(context, tokens); } /// @@ -89,9 +85,23 @@ namespace Microsoft.AspNet.Antiforgery { CheckSSLConfig(context); + if (string.IsNullOrEmpty(antiforgeryTokenSet.CookieToken)) + { + throw new ArgumentException( + Resources.Antiforgery_CookieToken_MustBeProvided_Generic, + nameof(antiforgeryTokenSet)); + } + + if (string.IsNullOrEmpty(antiforgeryTokenSet.FormToken)) + { + throw new ArgumentException( + Resources.Antiforgery_FormToken_MustBeProvided_Generic, + nameof(antiforgeryTokenSet)); + } + // Extract cookie & form tokens - var deserializedCookieToken = DeserializeToken(antiforgeryTokenSet.CookieToken); - var deserializedFormToken = DeserializeToken(antiforgeryTokenSet.FormToken); + var deserializedCookieToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.CookieToken); + var deserializedFormToken = _tokenSerializer.Deserialize(antiforgeryTokenSet.FormToken); // Validate _tokenGenerator.ValidateTokens( @@ -154,26 +164,6 @@ namespace Microsoft.AspNet.Antiforgery } } - private AntiforgeryToken DeserializeToken(string serializedToken) - { - return (!string.IsNullOrEmpty(serializedToken)) - ? _tokenSerializer.Deserialize(serializedToken) - : null; - } - - private AntiforgeryToken DeserializeTokenDoesNotThrow(string serializedToken) - { - try - { - return DeserializeToken(serializedToken); - } - catch - { - // ignore failures since we'll just generate a new token - return null; - } - } - private AntiforgeryToken GetCookieTokenDoesNotThrow(HttpContext context) { try diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs index 05ab540c4e..700ecaaa6e 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs @@ -6,22 +6,18 @@ using System.Diagnostics; using System.Security.Claims; using System.Security.Principal; using Microsoft.AspNet.Http; -using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Antiforgery { public class DefaultAntiforgeryTokenGenerator : IAntiforgeryTokenGenerator { private readonly IClaimUidExtractor _claimUidExtractor; - private readonly AntiforgeryOptions _options; private readonly IAntiforgeryAdditionalDataProvider _additionalDataProvider; public DefaultAntiforgeryTokenGenerator( - IOptions optionsAccessor, IClaimUidExtractor claimUidExtractor, IAntiforgeryAdditionalDataProvider additionalDataProvider) { - _options = optionsAccessor.Options; _claimUidExtractor = claimUidExtractor; _additionalDataProvider = additionalDataProvider; } @@ -96,23 +92,24 @@ namespace Microsoft.AspNet.Antiforgery AntiforgeryToken sessionToken, AntiforgeryToken fieldToken) { - // Were the tokens even present at all? if (sessionToken == null) { - throw new InvalidOperationException( - Resources.FormatAntiforgeryToken_CookieMissing(_options.CookieName)); + throw new ArgumentNullException( + nameof(sessionToken), + Resources.Antiforgery_CookieToken_MustBeProvided_Generic); } + if (fieldToken == null) { - throw new InvalidOperationException( - Resources.FormatAntiforgeryToken_FormFieldMissing(_options.FormFieldName)); + throw new ArgumentNullException( + nameof(fieldToken), + Resources.Antiforgery_FormToken_MustBeProvided_Generic); } // Do the tokens have the correct format? if (!sessionToken.IsSessionToken || fieldToken.IsSessionToken) { - throw new InvalidOperationException( - Resources.FormatAntiforgeryToken_TokensSwapped(_options.CookieName, _options.FormFieldName)); + throw new InvalidOperationException(Resources.AntiforgeryToken_TokensSwapped); } // Are the security tokens embedded in each incoming token identical? diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs index 833730e525..7a566d4295 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNet.Http; @@ -43,17 +44,24 @@ namespace Microsoft.AspNet.Antiforgery return _tokenSerializer.Deserialize(requestCookie); } - public async Task GetFormTokenAsync(HttpContext httpContext) + public async Task GetRequestTokensAsync([NotNull] HttpContext httpContext) { - var form = await httpContext.Request.ReadFormAsync(); - var value = form[_options.FormFieldName]; - if (string.IsNullOrEmpty(value)) + var requestCookie = httpContext.Request.Cookies[_options.CookieName]; + if (string.IsNullOrEmpty(requestCookie)) { - // did not exist - return null; + throw new InvalidOperationException( + Resources.FormatAntiforgery_CookieToken_MustBeProvided(_options.CookieName)); } - return _tokenSerializer.Deserialize(value); + var form = await httpContext.Request.ReadFormAsync(); + var formField = form[_options.FormFieldName]; + if (string.IsNullOrEmpty(formField)) + { + throw new InvalidOperationException( + Resources.FormatAntiforgery_FormToken_MustBeProvided(_options.FormFieldName)); + } + + return new AntiforgeryTokenSet(formField, requestCookie); } public void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token) diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs index 8ca4510bf7..bdf01af11e 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs @@ -1,8 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Security.Claims; using Microsoft.AspNet.Http; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { @@ -17,7 +17,7 @@ namespace Microsoft.AspNet.Antiforgery // Given a cookie token, generates a corresponding form token. // The incoming cookie token must be valid. AntiforgeryToken GenerateFormToken( - HttpContext httpContext, + [NotNull] HttpContext httpContext, AntiforgeryToken cookieToken); // Determines whether an existing cookie token is valid (well-formed). @@ -26,8 +26,8 @@ namespace Microsoft.AspNet.Antiforgery // Validates a (cookie, form) token pair. void ValidateTokens( - HttpContext httpContext, - AntiforgeryToken cookieToken, - AntiforgeryToken formToken); + [NotNull] HttpContext httpContext, + [NotNull] AntiforgeryToken cookieToken, + [NotNull] AntiforgeryToken formToken); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs index 0f2ab27024..afa43014d6 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs @@ -3,14 +3,23 @@ using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { // Provides an abstraction around how tokens are persisted and retrieved for a request public interface IAntiforgeryTokenStore { - AntiforgeryToken GetCookieToken(HttpContext httpContext); - Task GetFormTokenAsync(HttpContext httpContext); - void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token); + AntiforgeryToken GetCookieToken([NotNull] HttpContext httpContext); + + /// + /// Gets the cookie and form tokens from the request. Will throw an exception if either token is + /// not present. + /// + /// The for the current request. + /// The . + Task GetRequestTokensAsync([NotNull] HttpContext httpContext); + + void SaveCookieToken([NotNull] HttpContext httpContext, [NotNull] AntiforgeryToken token); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Antiforgery/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Antiforgery/Properties/Resources.Designer.cs index f012e4b91e..e4542a56f5 100644 --- a/src/Microsoft.AspNet.Antiforgery/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Antiforgery/Properties/Resources.Designer.cs @@ -58,22 +58,6 @@ namespace Microsoft.AspNet.Antiforgery return GetString("AntiforgeryToken_ClaimUidMismatch"); } - /// - /// The required antiforgery cookie "{0}" is not present. - /// - internal static string AntiforgeryToken_CookieMissing - { - get { return GetString("AntiforgeryToken_CookieMissing"); } - } - - /// - /// The required antiforgery cookie "{0}" is not present. - /// - internal static string FormatAntiforgeryToken_CookieMissing(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("AntiforgeryToken_CookieMissing"), p0); - } - /// /// The antiforgery token could not be decrypted. /// @@ -90,22 +74,6 @@ namespace Microsoft.AspNet.Antiforgery return GetString("AntiforgeryToken_DeserializationFailed"); } - /// - /// The required antiforgery form field "{0}" is not present. - /// - internal static string AntiforgeryToken_FormFieldMissing - { - get { return GetString("AntiforgeryToken_FormFieldMissing"); } - } - - /// - /// The required antiforgery form field "{0}" is not present. - /// - internal static string FormatAntiforgeryToken_FormFieldMissing(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("AntiforgeryToken_FormFieldMissing"), p0); - } - /// /// The antiforgery cookie token and form field token do not match. /// @@ -123,7 +91,7 @@ namespace Microsoft.AspNet.Antiforgery } /// - /// Validation of the provided antiforgery token failed. The cookie "{0}" and the form field "{1}" were swapped. + /// Validation of the provided antiforgery token failed. The cookie token and the form token were swapped. /// internal static string AntiforgeryToken_TokensSwapped { @@ -131,11 +99,11 @@ namespace Microsoft.AspNet.Antiforgery } /// - /// Validation of the provided antiforgery token failed. The cookie "{0}" and the form field "{1}" were swapped. + /// Validation of the provided antiforgery token failed. The cookie token and the form token were swapped. /// - internal static string FormatAntiforgeryToken_TokensSwapped(object p0, object p1) + internal static string FormatAntiforgeryToken_TokensSwapped() { - return string.Format(CultureInfo.CurrentCulture, GetString("AntiforgeryToken_TokensSwapped"), p0, p1); + return GetString("AntiforgeryToken_TokensSwapped"); } /// @@ -170,6 +138,70 @@ namespace Microsoft.AspNet.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_MustBeProvided + { + get { return GetString("Antiforgery_CookieToken_MustBeProvided"); } + } + + /// + /// The required antiforgery cookie "{0}" is not present. + /// + internal static string FormatAntiforgery_CookieToken_MustBeProvided(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("Antiforgery_CookieToken_MustBeProvided"), p0); + } + + /// + /// The cookie token must be provided. + /// + internal static string Antiforgery_CookieToken_MustBeProvided_Generic + { + get { return GetString("Antiforgery_CookieToken_MustBeProvided_Generic"); } + } + + /// + /// The cookie token must be provided. + /// + internal static string FormatAntiforgery_CookieToken_MustBeProvided_Generic() + { + return GetString("Antiforgery_CookieToken_MustBeProvided_Generic"); + } + + /// + /// The required antiforgery form field "{0}" is not present. + /// + internal static string Antiforgery_FormToken_MustBeProvided + { + get { return GetString("Antiforgery_FormToken_MustBeProvided"); } + } + + /// + /// The required antiforgery form field "{0}" is not present. + /// + internal static string FormatAntiforgery_FormToken_MustBeProvided(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("Antiforgery_FormToken_MustBeProvided"), p0); + } + + /// + /// The form token must be provided. + /// + internal static string Antiforgery_FormToken_MustBeProvided_Generic + { + get { return GetString("Antiforgery_FormToken_MustBeProvided_Generic"); } + } + + /// + /// The form token must be provided. + /// + internal static string FormatAntiforgery_FormToken_MustBeProvided_Generic() + { + return GetString("Antiforgery_FormToken_MustBeProvided_Generic"); + } + /// /// Value cannot be null or empty. /// diff --git a/src/Microsoft.AspNet.Antiforgery/Resources.resx b/src/Microsoft.AspNet.Antiforgery/Resources.resx index 2cad9e6cf6..cb8f9c3db8 100644 --- a/src/Microsoft.AspNet.Antiforgery/Resources.resx +++ b/src/Microsoft.AspNet.Antiforgery/Resources.resx @@ -127,20 +127,14 @@ The provided antiforgery token was meant for a different claims-based user than the current user. - - The required antiforgery cookie "{0}" is not present. - The antiforgery token could not be decrypted. - - The required antiforgery form field "{0}" is not present. - The antiforgery cookie token and form field token do not match. - Validation of the provided antiforgery token failed. The cookie "{0}" and the form field "{1}" were swapped. + Validation of the provided antiforgery token failed. The cookie token and the form token were swapped. The provided antiforgery token was meant for user "{0}", but the current user is "{1}". @@ -149,6 +143,18 @@ 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 required antiforgery cookie "{0}" is not present. + + + The cookie token must be provided. + + + The required antiforgery form field "{0}" is not present. + + + The form token must be provided. + Value cannot be null or empty. diff --git a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs index c775e5cd36..b283ee5aac 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs @@ -318,7 +318,7 @@ namespace Microsoft.AspNet.Antiforgery } [Fact] - public void Validate_FromInvalidStrings_Throws() + public void ValidateTokens_FromInvalidStrings_Throws() { // Arrange var context = CreateMockContext(new AntiforgeryOptions()); @@ -344,7 +344,7 @@ namespace Microsoft.AspNet.Antiforgery } [Fact] - public void Validate_FromValidStrings_TokensValidatedSuccessfully() + public void ValidateTokens_FromValidStrings_TokensValidatedSuccessfully() { // Arrange var context = CreateMockContext(new AntiforgeryOptions()); @@ -369,16 +369,37 @@ namespace Microsoft.AspNet.Antiforgery } [Fact] - public async Task Validate_FromStore_Failure() + public void ValidateTokens_MissingCookieInTokenSet_Throws() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions()); + var antiforgery = GetAntiforgery(context); + + var tokenSet = new AntiforgeryTokenSet("hi", cookieToken: null); + + + // Act + var exception = Assert.Throws( + () => antiforgery.ValidateTokens(context.HttpContext, tokenSet)); + + // Assert + var trimmed = exception.Message.Substring(0, exception.Message.IndexOf(Environment.NewLine)); + Assert.Equal("The cookie token must be provided.", trimmed); + } + + [Fact] + public async Task ValidateRequestAsync_FromStore_Failure() { // Arrange var context = CreateMockContext(new AntiforgeryOptions()); - context.TokenGenerator.Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken)) - .Throws(new InvalidOperationException("my-message")); - context.TokenSerializer = null; + context.TokenGenerator + .Setup(o => o.ValidateTokens( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.FormToken)) + .Throws(new InvalidOperationException("my-message")); + var antiforgery = GetAntiforgery(context); // Act & assert @@ -388,16 +409,18 @@ namespace Microsoft.AspNet.Antiforgery } [Fact] - public async Task Validate_FromStore_Success() + public async Task ValidateRequestAsync_FromStore_Success() { // Arrange var context = CreateMockContext(new AntiforgeryOptions()); - context.TokenGenerator.Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken)) - .Verifiable(); - context.TokenSerializer = null; + context.TokenGenerator + .Setup(o => o.ValidateTokens( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.FormToken)) + .Verifiable(); + var antiforgery = GetAntiforgery(context); // Act @@ -482,8 +505,11 @@ namespace Microsoft.AspNet.Antiforgery var mockTokenStore = new Mock(MockBehavior.Strict); mockTokenStore.Setup(o => o.GetCookieToken(context)) .Returns(oldCookieToken); - mockTokenStore.Setup(o => o.GetFormTokenAsync(context)) - .Returns(Task.FromResult(formToken)); + + mockTokenStore.Setup(o => o.GetRequestTokensAsync(context)) + .Returns(() => Task.FromResult(new AntiforgeryTokenSet( + testTokenSet.FormTokenString, + testTokenSet.OldCookieTokenString))); if (saveNewCookie) { @@ -502,11 +528,13 @@ namespace Microsoft.AspNet.Antiforgery var formToken = testTokenSet.FormToken; var mockSerializer = new Mock(MockBehavior.Strict); mockSerializer.Setup(o => o.Serialize(formToken)) - .Returns("serialized-form-token"); - mockSerializer.Setup(o => o.Deserialize("serialized-old-cookie-token")) + .Returns(testTokenSet.FormTokenString); + mockSerializer.Setup(o => o.Deserialize(testTokenSet.FormTokenString)) + .Returns(formToken); + mockSerializer.Setup(o => o.Deserialize(testTokenSet.OldCookieTokenString)) .Returns(oldCookieToken); mockSerializer.Setup(o => o.Serialize(newCookieToken)) - .Returns("serialized-new-cookie-token"); + .Returns(testTokenSet.NewCookieTokenString); return mockSerializer; } @@ -558,8 +586,11 @@ namespace Microsoft.AspNet.Antiforgery return new TestTokenSet() { FormToken = new AntiforgeryToken() { IsSessionToken = false }, + FormTokenString = "serialized-form-token", OldCookieToken = new AntiforgeryToken() { IsSessionToken = isOldCookieTokenSessionToken }, + OldCookieTokenString = "serialized-old-cookie-token", NewCookieToken = new AntiforgeryToken() { IsSessionToken = isNewCookieSessionToken }, + NewCookieTokenString = "serialized-new-cookie-token", }; } diff --git a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs index 8412e95208..9f8b9bd2b1 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs +++ b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenGeneratorTest.cs @@ -19,7 +19,6 @@ namespace Microsoft.AspNet.Antiforgery { // Arrange var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -40,7 +39,6 @@ namespace Microsoft.AspNet.Antiforgery Assert.False(httpContext.User.Identity.IsAuthenticated); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -74,7 +72,6 @@ namespace Microsoft.AspNet.Antiforgery var claimUidExtractor = new Mock().Object; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: claimUidExtractor, additionalDataProvider: null); @@ -109,7 +106,6 @@ namespace Microsoft.AspNet.Antiforgery var claimUidExtractor = new Mock().Object; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: claimUidExtractor, additionalDataProvider: mockAdditionalDataProvider.Object); @@ -148,7 +144,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(base64ClaimUId); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); @@ -182,7 +177,6 @@ namespace Microsoft.AspNet.Antiforgery var claimUidExtractor = new Mock().Object; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: claimUidExtractor, additionalDataProvider: null); @@ -209,7 +203,6 @@ namespace Microsoft.AspNet.Antiforgery }; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -226,7 +219,6 @@ namespace Microsoft.AspNet.Antiforgery // Arrange AntiforgeryToken cookieToken = null; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -247,7 +239,6 @@ namespace Microsoft.AspNet.Antiforgery }; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -268,21 +259,16 @@ namespace Microsoft.AspNet.Antiforgery var fieldtoken = new AntiforgeryToken() { IsSessionToken = false }; - var options = new AntiforgeryOptions() - { - CookieName = "my-cookie-name" - }; - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(options), claimUidExtractor: null, additionalDataProvider: null); // Act & assert - var ex = - Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, null, fieldtoken)); - Assert.Equal(@"The required antiforgery cookie ""my-cookie-name"" is not present.", ex.Message); + var ex = Assert.Throws( + () => tokenProvider.ValidateTokens(httpContext, null, fieldtoken)); + + var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); + Assert.Equal(@"The cookie token must be provided.", trimmed); } [Fact] @@ -294,21 +280,16 @@ namespace Microsoft.AspNet.Antiforgery var sessionToken = new AntiforgeryToken() { IsSessionToken = true }; - var options = new AntiforgeryOptions() - { - FormFieldName = "my-form-field-name" - }; - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(options), claimUidExtractor: null, additionalDataProvider: null); // Act & assert - var ex = - Assert.Throws( - () => tokenProvider.ValidateTokens(httpContext, sessionToken, null)); - Assert.Equal(@"The required antiforgery form field ""my-form-field-name"" is not present.", ex.Message); + var ex = Assert.Throws( + () => tokenProvider.ValidateTokens(httpContext, sessionToken, null)); + + var trimmed = ex.Message.Substring(0, ex.Message.IndexOf(Environment.NewLine)); + Assert.Equal("The form token must be provided.", trimmed); } [Fact] @@ -321,14 +302,7 @@ namespace Microsoft.AspNet.Antiforgery var sessionToken = new AntiforgeryToken() { IsSessionToken = true }; var fieldtoken = new AntiforgeryToken() { IsSessionToken = false }; - var options = new AntiforgeryOptions() - { - CookieName = "my-cookie-name", - FormFieldName = "my-form-field-name" - }; - var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(options), claimUidExtractor: null, additionalDataProvider: null); @@ -338,7 +312,7 @@ namespace Microsoft.AspNet.Antiforgery () => tokenProvider.ValidateTokens(httpContext, fieldtoken, fieldtoken)); Assert.Equal( "Validation of the provided antiforgery token failed. " + - @"The cookie ""my-cookie-name"" and the form field ""my-form-field-name"" were swapped.", + @"The cookie token and the form token were swapped.", ex1.Message); var ex2 = @@ -346,7 +320,7 @@ namespace Microsoft.AspNet.Antiforgery () => tokenProvider.ValidateTokens(httpContext, sessionToken, sessionToken)); Assert.Equal( "Validation of the provided antiforgery token failed. " + - @"The cookie ""my-cookie-name"" and the form field ""my-form-field-name"" were swapped.", + @"The cookie token and the form token were swapped.", ex2.Message); } @@ -361,7 +335,6 @@ namespace Microsoft.AspNet.Antiforgery var fieldtoken = new AntiforgeryToken() { IsSessionToken = false }; var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: null); @@ -399,7 +372,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns((string)null); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); @@ -434,7 +406,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(Convert.ToBase64String(differentToken.GetData())); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); @@ -468,7 +439,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(false); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: mockAdditionalDataProvider.Object); @@ -500,7 +470,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(true); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: null, additionalDataProvider: mockAdditionalDataProvider.Object); @@ -533,7 +502,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(true); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: new Mock().Object, additionalDataProvider: mockAdditionalDataProvider.Object); @@ -565,7 +533,6 @@ namespace Microsoft.AspNet.Antiforgery .Returns(Convert.ToBase64String(fieldtoken.ClaimUid.GetData())); var tokenProvider = new DefaultAntiforgeryTokenGenerator( - optionsAccessor: new TestOptionsManager(), claimUidExtractor: mockClaimUidExtractor.Object, additionalDataProvider: null); diff --git a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs index d2bfc7b458..a3cf81ce87 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; using Microsoft.Framework.DependencyInjection; using Moq; using Xunit; @@ -160,20 +161,16 @@ namespace Microsoft.AspNet.Antiforgery } [Fact] - public async Task GetFormToken_FormFieldIsEmpty_ReturnsNull() + public async Task GetRequestTokens_CookieIsEmpty_Throws() { // Arrange - var mockHttpContext = new Mock(); - var requestContext = new Mock(); - var formCollection = new Mock(); - formCollection.Setup(f => f["form-field-name"]).Returns(string.Empty); - requestContext.Setup(o => o.ReadFormAsync(CancellationToken.None)) - .Returns(Task.FromResult(formCollection.Object)); - mockHttpContext.Setup(o => o.Request) - .Returns(requestContext.Object); + var httpContext = new DefaultHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary()); + httpContext.Request.Cookies = new ReadableStringCollection(new Dictionary()); var options = new AntiforgeryOptions() { + CookieName = "cookie-name", FormFieldName = "form-field-name", }; @@ -182,81 +179,72 @@ namespace Microsoft.AspNet.Antiforgery tokenSerializer: null); // Act - var token = await tokenStore.GetFormTokenAsync(mockHttpContext.Object); + var exception = await Assert.ThrowsAsync( + async () => await tokenStore.GetRequestTokensAsync(httpContext)); - // Assert - Assert.Null(token); + // Assert + Assert.Equal("The required antiforgery cookie \"cookie-name\" is not present.", exception.Message); } [Fact] - public async Task GetFormToken_FormFieldIsInvalid_PropagatesException() + public async Task GetRequestTokens_FormFieldIsEmpty_Throws() { // Arrange - var formCollection = new Mock(); - formCollection.Setup(f => f["form-field-name"]).Returns("invalid-value"); - - var requestContext = new Mock(); - requestContext.Setup(o => o.ReadFormAsync(CancellationToken.None)) - .Returns(Task.FromResult(formCollection.Object)); - - var mockHttpContext = new Mock(); - mockHttpContext.Setup(o => o.Request) - .Returns(requestContext.Object); - - var expectedException = new InvalidOperationException("some exception"); - var mockSerializer = new Mock(); - mockSerializer.Setup(o => o.Deserialize("invalid-value")) - .Throws(expectedException); + var httpContext = new DefaultHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary()); + httpContext.Request.Cookies = new ReadableStringCollection(new Dictionary() + { + { "cookie-name", new string[] { "cookie-value" } }, + }); var options = new AntiforgeryOptions() { + CookieName = "cookie-name", FormFieldName = "form-field-name", }; var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: mockSerializer.Object); + tokenSerializer: null); - // Act & assert + // Act var exception = await Assert.ThrowsAsync( - async () => await tokenStore.GetFormTokenAsync(mockHttpContext.Object)); - Assert.Same(expectedException, exception); + async () => await tokenStore.GetRequestTokensAsync(httpContext)); + + // Assert + Assert.Equal("The required antiforgery form field \"form-field-name\" is not present.", exception.Message); } [Fact] public async Task GetFormToken_FormFieldIsValid_ReturnsToken() { // Arrange - var expectedToken = new AntiforgeryToken(); - - // Arrange - var mockHttpContext = new Mock(); - var requestContext = new Mock(); - var formCollection = new Mock(); - formCollection.Setup(f => f["form-field-name"]).Returns("valid-value"); - requestContext.Setup(o => o.ReadFormAsync(CancellationToken.None)) - .Returns(Task.FromResult(formCollection.Object)); - mockHttpContext.Setup(o => o.Request) - .Returns(requestContext.Object); - - var mockSerializer = new Mock(); - mockSerializer.Setup(o => o.Deserialize("valid-value")) - .Returns(expectedToken); + var httpContext = new DefaultHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary() + { + { "form-field-name", new string[] { "form-value" } }, + }); + httpContext.Request.Cookies = new ReadableStringCollection(new Dictionary() + { + { "cookie-name", new string[] { "cookie-value" } }, + }); var options = new AntiforgeryOptions() { + CookieName = "cookie-name", FormFieldName = "form-field-name", }; var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: mockSerializer.Object); + tokenSerializer: null); // Act - var retVal = await tokenStore.GetFormTokenAsync(mockHttpContext.Object); + var tokens = await tokenStore.GetRequestTokensAsync(httpContext); // Assert - Assert.Same(expectedToken, retVal); + Assert.Equal("cookie-value", tokens.CookieToken); + Assert.Equal("form-value", tokens.FormToken); } [Theory]