diff --git a/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryTokenSet.cs b/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryTokenSet.cs index 2b99141894..033e5e0731 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryTokenSet.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryTokenSet.cs @@ -23,22 +23,13 @@ namespace Microsoft.AspNetCore.Antiforgery string formFieldName, string headerName) { - if (string.IsNullOrEmpty(requestToken)) - { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(requestToken)); - } - if (formFieldName == null) { throw new ArgumentNullException(nameof(formFieldName)); } RequestToken = requestToken; - - // Cookie Token is allowed to be null in the case when the old cookie is valid - // and there is no new cookieToken generated. CookieToken = cookieToken; - FormFieldName = formFieldName; HeaderName = headerName; } diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs index 6d52cfa4da..1a581a5f54 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgery.cs @@ -93,6 +93,10 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CheckSSLConfig(httpContext); var tokens = await _tokenStore.GetRequestTokensAsync(httpContext); + if (tokens.CookieToken == null || tokens.RequestToken == null) + { + return false; + } // Extract cookie & request tokens var deserializedCookieToken = _tokenSerializer.Deserialize(tokens.CookieToken); @@ -118,6 +122,33 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal CheckSSLConfig(httpContext); var tokens = await _tokenStore.GetRequestTokensAsync(httpContext); + if (tokens.CookieToken == null) + { + throw new AntiforgeryValidationException( + Resources.FormatAntiforgery_CookieToken_MustBeProvided(_options.CookieName)); + } + + if (tokens.RequestToken == null) + { + if (_options.HeaderName == null) + { + var message = Resources.FormatAntiforgery_FormToken_MustBeProvided(_options.FormFieldName); + throw new AntiforgeryValidationException(message); + } + else if (!httpContext.Request.HasFormContentType) + { + var message = Resources.FormatAntiforgery_HeaderToken_MustBeProvided(_options.HeaderName); + throw new AntiforgeryValidationException(message); + } + else + { + var message = Resources.FormatAntiforgery_RequestToken_MustBeProvided( + _options.FormFieldName, + _options.HeaderName); + throw new AntiforgeryValidationException(message); + } + } + ValidateTokens(httpContext, tokens); } @@ -262,8 +293,8 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal private AntiforgeryTokenSet Serialize(AntiforgeryTokenSetInternal tokenSet) { return new AntiforgeryTokenSet( - tokenSet.RequestToken != null ? _tokenSerializer.Serialize(tokenSet.RequestToken) : null, - tokenSet.CookieToken != null ? _tokenSerializer.Serialize(tokenSet.CookieToken) : null, + _tokenSerializer.Serialize(tokenSet.RequestToken), + _tokenSerializer.Serialize(tokenSet.CookieToken), _options.FormFieldName, _options.HeaderName); } diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs index 52d852a27c..5b0f41426a 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs @@ -65,18 +65,13 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal throw new ArgumentNullException(nameof(httpContext)); } - var requestCookie = httpContext.Request.Cookies[_options.CookieName]; - if (string.IsNullOrEmpty(requestCookie)) - { - throw new AntiforgeryValidationException( - Resources.FormatAntiforgery_CookieToken_MustBeProvided(_options.CookieName)); - } + var cookieToken = httpContext.Request.Cookies[_options.CookieName]; StringValues requestToken; if (httpContext.Request.HasFormContentType) { // Check the content-type before accessing the form collection to make sure - // we throw gracefully. + // we report errors gracefully. var form = await httpContext.Request.ReadFormAsync(); requestToken = form[_options.FormFieldName]; } @@ -87,28 +82,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal requestToken = httpContext.Request.Headers[_options.HeaderName]; } - if (requestToken.Count == 0) - { - if (_options.HeaderName == null) - { - var message = Resources.FormatAntiforgery_FormToken_MustBeProvided(_options.FormFieldName); - throw new AntiforgeryValidationException(message); - } - else if (!httpContext.Request.HasFormContentType) - { - var message = Resources.FormatAntiforgery_HeaderToken_MustBeProvided(_options.HeaderName); - throw new AntiforgeryValidationException(message); - } - else - { - var message = Resources.FormatAntiforgery_RequestToken_MustBeProvided( - _options.FormFieldName, - _options.HeaderName); - throw new AntiforgeryValidationException(message); - } - } - - return new AntiforgeryTokenSet(requestToken, requestCookie, _options.FormFieldName, _options.HeaderName); + return new AntiforgeryTokenSet(requestToken, cookieToken, _options.FormFieldName, _options.HeaderName); } public void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token) diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs index bff9360ff9..967887a44f 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/IAntiforgeryTokenStore.cs @@ -6,14 +6,12 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Antiforgery.Internal { - // Provides an abstraction around how tokens are persisted and retrieved for a request public interface IAntiforgeryTokenStore { AntiforgeryToken GetCookieToken(HttpContext httpContext); /// - /// Gets the cookie and request tokens from the request. Will throw an exception if either token is - /// not present. + /// Gets the cookie and request tokens from the request. /// /// The for the current request. /// The . diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs index 0e8dfeb8dd..040cf866a4 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs @@ -591,6 +591,109 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal context.TokenGenerator.Verify(); } + [Fact] + public async Task ValidateRequestAsync_NoCookieToken_Throws() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions() + { + CookieName = "cookie-name", + FormFieldName = "form-field-name", + HeaderName = null, + }); + + var tokenSet = new AntiforgeryTokenSet(null, null, "form-field-name", null); + context.TokenStore + .Setup(s => s.GetRequestTokensAsync(context.HttpContext)) + .Returns(Task.FromResult(tokenSet)); + + var antiforgery = GetAntiforgery(context); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => antiforgery.ValidateRequestAsync(context.HttpContext)); + Assert.Equal("The required antiforgery cookie \"cookie-name\" is not present.", exception.Message); + } + + [Fact] + public async Task ValidateRequestAsync_NonFormRequest_HeaderDisabled_Throws() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions() + { + CookieName = "cookie-name", + FormFieldName = "form-field-name", + HeaderName = null, + }); + + var tokenSet = new AntiforgeryTokenSet(null, "cookie-token", "form-field-name", null); + context.TokenStore + .Setup(s => s.GetRequestTokensAsync(context.HttpContext)) + .Returns(Task.FromResult(tokenSet)); + + var antiforgery = GetAntiforgery(context); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => antiforgery.ValidateRequestAsync(context.HttpContext)); + Assert.Equal("The required antiforgery form field \"form-field-name\" is not present.", exception.Message); + } + + [Fact] + public async Task ValidateRequestAsync_NonFormRequest_NoHeaderValue_Throws() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions() + { + CookieName = "cookie-name", + FormFieldName = "form-field-name", + HeaderName = "header-name", + }); + + context.HttpContext.Request.ContentType = "application/json"; + + var tokenSet = new AntiforgeryTokenSet(null, "cookie-token", "form-field-name", "header-name"); + context.TokenStore + .Setup(s => s.GetRequestTokensAsync(context.HttpContext)) + .Returns(Task.FromResult(tokenSet)); + + var antiforgery = GetAntiforgery(context); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => antiforgery.ValidateRequestAsync(context.HttpContext)); + Assert.Equal("The required antiforgery header value \"header-name\" is not present.", exception.Message); + } + + [Fact] + public async Task ValidateRequestAsync_FormRequest_NoRequestTokenValue_Throws() + { + // Arrange + var context = CreateMockContext(new AntiforgeryOptions() + { + CookieName = "cookie-name", + FormFieldName = "form-field-name", + HeaderName = "header-name", + }); + + context.HttpContext.Request.ContentType = "application/x-www-form-urlencoded"; + + var tokenSet = new AntiforgeryTokenSet(null, "cookie-token", "form-field-name", "header-name"); + context.TokenStore + .Setup(s => s.GetRequestTokensAsync(context.HttpContext)) + .Returns(Task.FromResult(tokenSet)); + + var antiforgery = GetAntiforgery(context); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => antiforgery.ValidateRequestAsync(context.HttpContext)); + Assert.Equal( + "The required antiforgery request token was not provided in either form field \"form-field-name\" " + + "or header value \"header-name\".", + exception.Message); + } + [Theory] [InlineData(false, "SAMEORIGIN")] [InlineData(true, null)] diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs index 13f3ff7082..16ad4857a1 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs @@ -145,7 +145,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public async Task GetRequestTokens_CookieIsEmpty_Throws() + public async Task GetRequestTokens_CookieIsEmpty_ReturnsNullTokens() { // Arrange var httpContext = GetHttpContext(new RequestCookieCollection()); @@ -162,15 +162,15 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal tokenSerializer: Mock.Of()); // Act - var exception = await Assert.ThrowsAsync( - async () => await tokenStore.GetRequestTokensAsync(httpContext)); + var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); // Assert - Assert.Equal("The required antiforgery cookie \"cookie-name\" is not present.", exception.Message); + Assert.Null(tokenSet.CookieToken); + Assert.Null(tokenSet.RequestToken); } [Fact] - public async Task GetRequestTokens_NonFormContentType_HeaderDisabled_Throws() + public async Task GetRequestTokens_NonFormContentType_HeaderDisabled_ReturnsNullToken() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); @@ -191,11 +191,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); // Act - var exception = await Assert.ThrowsAsync( - async () => await tokenStore.GetRequestTokensAsync(httpContext)); + var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); // Assert - Assert.Equal("The required antiforgery form field \"form-field-name\" is not present.", exception.Message); + Assert.Equal("cookie-value", tokenSet.CookieToken); + Assert.Null(tokenSet.RequestToken); } [Fact] @@ -257,7 +257,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public async Task GetRequestTokens_NonFormContentType_UsesHeaderToken_ThrowsOnMissingValue() + public async Task GetRequestTokens_NonFormContentType_NoHeaderToken_ReturnsNullToken() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); @@ -278,15 +278,15 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider(), _pool)); // Act - var exception = await Assert.ThrowsAsync( - async () => await tokenStore.GetRequestTokensAsync(httpContext)); + var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); // Assert - Assert.Equal("The required antiforgery header value \"header-name\" is not present.", exception.Message); + Assert.Equal("cookie-value", tokenSet.CookieToken); + Assert.Null(tokenSet.RequestToken); } [Fact] - public async Task GetRequestTokens_BothFieldsEmpty_Throws() + public async Task GetRequestTokens_BothFieldsEmpty_ReturnsNullTokens() { // Arrange var httpContext = GetHttpContext("cookie-name", "cookie-value"); @@ -305,14 +305,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal tokenSerializer: Mock.Of()); // Act - var exception = await Assert.ThrowsAsync( - async () => await tokenStore.GetRequestTokensAsync(httpContext)); + var tokenSet = await tokenStore.GetRequestTokensAsync(httpContext); // Assert - Assert.Equal( - "The required antiforgery request token was not provided in either form field \"form-field-name\" " + - "or header value \"header-name\".", - exception.Message); + Assert.Equal("cookie-value", tokenSet.CookieToken); + Assert.Null(tokenSet.RequestToken); } [Fact]