diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs index a10370c334..d46e2153cf 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs @@ -3,8 +3,8 @@ using System; using System.Diagnostics; -using System.Text.Encodings.Web; using System.Threading.Tasks; +using Microsoft.AspNet.Html.Abstractions; using Microsoft.AspNet.Http; using Microsoft.Extensions.OptionsModel; @@ -16,7 +16,6 @@ namespace Microsoft.AspNet.Antiforgery /// public class DefaultAntiforgery : IAntiforgery { - private readonly HtmlEncoder _htmlEncoder; private readonly AntiforgeryOptions _options; private readonly IAntiforgeryTokenGenerator _tokenGenerator; private readonly IAntiforgeryTokenSerializer _tokenSerializer; @@ -26,18 +25,16 @@ namespace Microsoft.AspNet.Antiforgery IOptions antiforgeryOptionsAccessor, IAntiforgeryTokenGenerator tokenGenerator, IAntiforgeryTokenSerializer tokenSerializer, - IAntiforgeryTokenStore tokenStore, - HtmlEncoder htmlEncoder) + IAntiforgeryTokenStore tokenStore) { _options = antiforgeryOptionsAccessor.Value; _tokenGenerator = tokenGenerator; _tokenSerializer = tokenSerializer; _tokenStore = tokenStore; - _htmlEncoder = htmlEncoder; } /// - public string GetHtml(HttpContext context) + public IHtmlContent GetHtml(HttpContext context) { if (context == null) { @@ -48,12 +45,17 @@ namespace Microsoft.AspNet.Antiforgery var tokenSet = GetAndStoreTokens(context); - var inputTag = string.Format( - "", - _htmlEncoder.Encode(_options.FormFieldName), - _htmlEncoder.Encode("hidden"), - _htmlEncoder.Encode(tokenSet.FormToken)); - return inputTag; + // Though FormToken normally contains only US-ASCII letters, numbers, '-', and '_', must assume the + // IAntiforgeryTokenSerializer implementation has been overridden. Similarly, users may choose a + // FormFieldName containing almost any character. + var content = new HtmlContentBuilder() + .AppendHtml(""); + + return content; } /// @@ -71,6 +73,7 @@ namespace Microsoft.AspNet.Antiforgery { SaveCookieTokenAndHeader(context, tokenSet.CookieToken); } + return Serialize(tokenSet); } diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs index f57c5689ae..51aecc0826 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Threading.Tasks; +using Microsoft.AspNet.Html.Abstractions; using Microsoft.AspNet.Http; namespace Microsoft.AspNet.Antiforgery @@ -13,18 +14,18 @@ namespace Microsoft.AspNet.Antiforgery public interface IAntiforgery { /// - /// Generates an input field for an antiforgery token. + /// Generates an <input type="hidden"> element for an antiforgery token. /// /// The associated with the current request. /// - /// A string containing an <input type="hidden"> element. This element should be put inside - /// a <form>. + /// A containing an <input type="hidden"> element. This element should be put + /// inside a <form>. /// /// /// This method has a side effect: /// A response cookie is set if there is no valid cookie associated with the request. /// - string GetHtml(HttpContext context); + IHtmlContent GetHtml(HttpContext context); /// /// Generates an for this request and stores the cookie token diff --git a/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs index e78899c7ba..f728cdb40f 100644 --- a/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs @@ -18,7 +18,6 @@ namespace Microsoft.Extensions.DependencyInjection } services.AddDataProtection(); - services.AddWebEncoders(); // Don't overwrite any options setups that a user may have added. services.TryAddEnumerable( diff --git a/src/Microsoft.AspNet.Antiforgery/project.json b/src/Microsoft.AspNet.Antiforgery/project.json index bd7c4464a8..9222a0c515 100644 --- a/src/Microsoft.AspNet.Antiforgery/project.json +++ b/src/Microsoft.AspNet.Antiforgery/project.json @@ -11,12 +11,12 @@ }, "dependencies": { "Microsoft.AspNet.DataProtection": "1.0.0-*", + "Microsoft.AspNet.Html.Abstractions": "1.0.0-*", "Microsoft.AspNet.Http.Abstractions": "1.0.0-*", - "Microsoft.AspNet.WebUtilities": "1.0.0-*", - "Microsoft.Extensions.WebEncoders": "1.0.0-*" + "Microsoft.AspNet.WebUtilities": "1.0.0-*" }, "frameworks": { - "net451": {}, - "dotnet5.4": {} + "dotnet5.4": { }, + "net451": { } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs index 1a709dccad..cd06a911b8 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs +++ b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs @@ -2,8 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Security.Claims; using System.Threading.Tasks; +using Microsoft.AspNet.Html.Abstractions; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; using Microsoft.Extensions.OptionsModel; @@ -73,8 +75,7 @@ namespace Microsoft.AspNet.Antiforgery var antiforgery = GetAntiforgery(options); // Act & Assert - var exception = Assert.Throws( - () => antiforgery.GetHtml(httpContext)); + var exception = Assert.Throws(() => antiforgery.GetHtml(httpContext)); Assert.Equal( @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " + "but the current request is not an SSL request.", @@ -159,15 +160,22 @@ namespace Microsoft.AspNet.Antiforgery // Make sure the existing cookie is invalid. var context = CreateMockContext(options, isOldCookieValid: false); var antiforgery = GetAntiforgery(context); + var encoder = new HtmlTestEncoder(); // Act var inputElement = antiforgery.GetHtml(context.HttpContext); // Assert - Assert.Equal( - @"", - inputElement); + using (var writer = new StringWriter()) + { + inputElement.WriteTo(writer, encoder); + + Assert.Equal( + @"", + writer.ToString()); + } + context.TokenStore.Verify(); } @@ -185,21 +193,31 @@ namespace Microsoft.AspNet.Antiforgery var antiforgery = GetAntiforgery(context); // This will cause the cookieToken to be null. - context.TokenStore.Setup(o => o.GetCookieToken(context.HttpContext)) - .Throws(new Exception("should be swallowed")); + context.TokenStore + .Setup(o => o.GetCookieToken(context.HttpContext)) + .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); + context.TokenGenerator + .Setup(o => o.IsCookieTokenValid(null)) + .Returns(false); + + var encoder = new HtmlTestEncoder(); // Act var inputElement = antiforgery.GetHtml(context.HttpContext); // Assert - Assert.Equal( - @"", - inputElement); + using (var writer = new StringWriter()) + { + inputElement.WriteTo(writer, encoder); + + Assert.Equal( + @"", + writer.ToString()); + } + context.TokenStore.Verify(); } @@ -215,15 +233,21 @@ namespace Microsoft.AspNet.Antiforgery // Make sure the existing cookie is valid and use the same cookie for the mock Token Provider. var context = CreateMockContext(options, useOldCookie: true, isOldCookieValid: true); var antiforgery = GetAntiforgery(context); + var encoder = new HtmlTestEncoder(); // Act var inputElement = antiforgery.GetHtml(context.HttpContext); // Assert - Assert.Equal( - @"", - inputElement); + using (var writer = new StringWriter()) + { + inputElement.WriteTo(writer, encoder); + + Assert.Equal( + @"", + writer.ToString()); + } } [Theory] @@ -237,12 +261,12 @@ namespace Microsoft.AspNet.Antiforgery SuppressXFrameOptionsHeader = suppressXFrameOptions }; - // Genreate a new cookie. + // Generate a new cookie. var context = CreateMockContext(options, useOldCookie: false, isOldCookieValid: false); var antiforgery = GetAntiforgery(context); // Act - var inputElement = antiforgery.GetHtml(context.HttpContext); + antiforgery.GetHtml(context.HttpContext); // Assert string xFrameOptions = context.HttpContext.Response.Headers["X-Frame-Options"]; @@ -253,7 +277,7 @@ namespace Microsoft.AspNet.Antiforgery public void GetTokens_ExistingInvalidCookieToken_GeneratesANewCookieTokenAndANewFormToken() { // Arrange - // Genreate a new cookie. + // Generate a new cookie. var context = CreateMockContext( new AntiforgeryOptions(), useOldCookie: false, @@ -328,8 +352,9 @@ namespace Microsoft.AspNet.Antiforgery // Assert // We shouldn't have saved the cookie because it already existed. - context.TokenStore.Verify(t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Never); - + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Never); + Assert.Equal("serialized-old-cookie-token", tokenSet.CookieToken); Assert.Equal("serialized-form-token", tokenSet.FormToken); } @@ -348,7 +373,8 @@ namespace Microsoft.AspNet.Antiforgery var tokenSet = antiforgery.GetAndStoreTokens(context.HttpContext); // Assert - context.TokenStore.Verify(t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Once); + context.TokenStore.Verify( + t => t.SaveCookieToken(It.IsAny(), It.IsAny()), Times.Once); Assert.Equal("serialized-new-cookie-token", tokenSet.CookieToken); Assert.Equal("serialized-form-token", tokenSet.FormToken); @@ -360,22 +386,26 @@ namespace Microsoft.AspNet.Antiforgery // Arrange var context = CreateMockContext(new AntiforgeryOptions()); - context.TokenSerializer.Setup(o => o.Deserialize("cookie-token")) - .Returns(context.TestTokenSet.OldCookieToken); - context.TokenSerializer.Setup(o => o.Deserialize("form-token")) - .Returns(context.TestTokenSet.FormToken); + context.TokenSerializer + .Setup(o => o.Deserialize("cookie-token")) + .Returns(context.TestTokenSet.OldCookieToken); + context.TokenSerializer + .Setup(o => o.Deserialize("form-token")) + .Returns(context.TestTokenSet.FormToken); - context.TokenGenerator.Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken)) - .Throws(new InvalidOperationException("my-message")); + context.TokenGenerator + .Setup(o => o.ValidateTokens( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.FormToken)) + .Throws(new InvalidOperationException("my-message")); context.TokenStore = null; var antiforgery = GetAntiforgery(context); // Act & assert var exception = Assert.Throws( () => antiforgery.ValidateTokens( - context.HttpContext, + context.HttpContext, new AntiforgeryTokenSet("form-token", "cookie-token"))); Assert.Equal("my-message", exception.Message); } @@ -386,15 +416,19 @@ namespace Microsoft.AspNet.Antiforgery // Arrange var context = CreateMockContext(new AntiforgeryOptions()); - context.TokenSerializer.Setup(o => o.Deserialize("cookie-token")) - .Returns(context.TestTokenSet.OldCookieToken); - context.TokenSerializer.Setup(o => o.Deserialize("form-token")) - .Returns(context.TestTokenSet.FormToken); + context.TokenSerializer + .Setup(o => o.Deserialize("cookie-token")) + .Returns(context.TestTokenSet.OldCookieToken); + context.TokenSerializer + .Setup(o => o.Deserialize("form-token")) + .Returns(context.TestTokenSet.FormToken); - context.TokenGenerator.Setup(o => o.ValidateTokens( - context.HttpContext, - context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken)) - .Verifiable(); + context.TokenGenerator + .Setup(o => o.ValidateTokens( + context.HttpContext, + context.TestTokenSet.OldCookieToken, + context.TestTokenSet.FormToken)) + .Verifiable(); context.TokenStore = null; var antiforgery = GetAntiforgery(context); @@ -480,7 +514,7 @@ namespace Microsoft.AspNet.Antiforgery SuppressXFrameOptionsHeader = suppressXFrameOptions }; - // Genreate a new cookie. + // Generate a new cookie. var context = CreateMockContext(options, useOldCookie: false, isOldCookieValid: false); var antiforgery = GetAntiforgery(context); @@ -508,8 +542,7 @@ namespace Microsoft.AspNet.Antiforgery antiforgeryOptionsAccessor: optionsManager, tokenGenerator: tokenGenerator, tokenSerializer: tokenSerializer, - tokenStore: tokenStore, - htmlEncoder: new HtmlTestEncoder()); + tokenStore: tokenStore); } private HttpContext GetHttpContext() @@ -522,9 +555,9 @@ namespace Microsoft.AspNet.Antiforgery private DefaultAntiforgery GetAntiforgery(AntiforgeryMockContext context) { return GetAntiforgery( - context.Options, - context.TokenGenerator?.Object, - context.TokenSerializer?.Object, + context.Options, + context.TokenGenerator?.Object, + context.TokenSerializer?.Object, context.TokenStore?.Object); } @@ -538,7 +571,7 @@ namespace Microsoft.AspNet.Antiforgery var mockTokenStore = new Mock(MockBehavior.Strict); mockTokenStore.Setup(o => o.GetCookieToken(context)) .Returns(oldCookieToken); - + mockTokenStore.Setup(o => o.GetRequestTokensAsync(context)) .Returns(() => Task.FromResult(new AntiforgeryTokenSet( testTokenSet.FormTokenString, diff --git a/test/Microsoft.AspNet.Antiforgery.Test/project.json b/test/Microsoft.AspNet.Antiforgery.Test/project.json index 46fd83f562..a9bab13140 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/project.json +++ b/test/Microsoft.AspNet.Antiforgery.Test/project.json @@ -7,6 +7,7 @@ "Microsoft.AspNet.Antiforgery": "1.0.0-*", "Microsoft.AspNet.Http": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.0.0-*", + "Microsoft.Extensions.WebEncoders": "1.0.0-*", "xunit.runner.aspnet": "2.0.0-aspnet-*" }, "commands": {