Change `FormTagHelper` to apply to all form tags.

- Added functional test to validate that non-attributed form tags have an antiforgery input generated. Re-generated baseline to reflect changes.
- Added a unit test to validate that parameterless `FormTagHelper`s behave as expected.

#6006
This commit is contained in:
N. Taylor Mullen 2017-04-17 17:25:44 -07:00
parent ccdaa5a729
commit 5c8a161ace
6 changed files with 218 additions and 32 deletions

View File

@ -4,6 +4,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.ComponentModel; using System.ComponentModel;
using Microsoft.AspNetCore.Html;
using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.Rendering;
using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Razor.TagHelpers; using Microsoft.AspNetCore.Razor.TagHelpers;
@ -14,15 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
/// <summary> /// <summary>
/// <see cref="ITagHelper"/> implementation targeting &lt;form&gt; elements. /// <see cref="ITagHelper"/> implementation targeting &lt;form&gt; elements.
/// </summary> /// </summary>
[HtmlTargetElement("form", Attributes = ActionAttributeName)] [HtmlTargetElement("form")]
[HtmlTargetElement("form", Attributes = AntiforgeryAttributeName)]
[HtmlTargetElement("form", Attributes = AreaAttributeName)]
[HtmlTargetElement("form", Attributes = PageAttributeName)]
[HtmlTargetElement("form", Attributes = FragmentAttributeName)]
[HtmlTargetElement("form", Attributes = ControllerAttributeName)]
[HtmlTargetElement("form", Attributes = RouteAttributeName)]
[HtmlTargetElement("form", Attributes = RouteValuesDictionaryName)]
[HtmlTargetElement("form", Attributes = RouteValuesPrefix + "*")]
public class FormTagHelper : TagHelper public class FormTagHelper : TagHelper
{ {
private const string ActionAttributeName = "asp-action"; private const string ActionAttributeName = "asp-action";
@ -152,15 +145,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{ {
throw new ArgumentNullException(nameof(output)); throw new ArgumentNullException(nameof(output));
} }
if (Method != null) if (Method != null)
{ {
output.CopyHtmlAttribute(nameof(Method), context); output.CopyHtmlAttribute(nameof(Method), context);
} }
else
{
Method = "get";
}
var antiforgeryDefault = true; var antiforgeryDefault = true;
// If "action" is already set, it means the user is attempting to use a normal <form>. // If "action" is already set, it means the user is attempting to use a normal <form>.
if (output.Attributes.ContainsName(HtmlActionAttributeName)) if (output.Attributes.TryGetAttribute(HtmlActionAttributeName, out var actionAttribute))
{ {
if (Action != null || if (Action != null ||
Controller != null || Controller != null ||
@ -184,9 +182,29 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
PageAttributeName)); PageAttributeName));
} }
// User is using the FormTagHelper like a normal <form> tag. Antiforgery default should be false to string attributeValue = null;
// not force the antiforgery token on the user. switch (actionAttribute.Value)
antiforgeryDefault = false; {
case HtmlString htmlString:
attributeValue = htmlString.ToString();
break;
case string stringValue:
attributeValue = stringValue;
break;
}
if (string.IsNullOrEmpty(attributeValue))
{
// User is using the FormTagHelper like a normal <form> tag that has an empty or complex IHtmlContent action attribute.
// e.g. <form action="" method="post"> or <form action="@CustomUrlIHtmlContent" method="post">
// Antiforgery default is already set to true
}
else
{
// User is likely using the <form> element to submit to another site. Do not send an antiforgery token to unknown sites.
antiforgeryDefault = false;
}
} }
else else
{ {
@ -223,8 +241,12 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
routeValues["area"] = Area; routeValues["area"] = Area;
} }
TagBuilder tagBuilder; TagBuilder tagBuilder = null;
if (pageLink) if (Action == null && Controller == null && Route == null && _routeValues == null && Fragment == null && Area == null && Page == null)
{
// Empty form tag such as <form></form>. Let it flow to the output as-is and only handle anti-forgery.
}
else if (pageLink)
{ {
tagBuilder = Generator.GeneratePageForm( tagBuilder = Generator.GeneratePageForm(
ViewContext, ViewContext,
@ -256,13 +278,19 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
htmlAttributes: null); htmlAttributes: null);
} }
output.MergeAttributes(tagBuilder); if (tagBuilder != null)
if (tagBuilder.HasInnerHtml)
{ {
output.PostContent.AppendHtml(tagBuilder.InnerHtml); output.MergeAttributes(tagBuilder);
if (tagBuilder.HasInnerHtml)
{
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
}
} }
}
antiforgeryDefault = !string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase); if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase))
{
antiforgeryDefault = false;
} }
if (Antiforgery ?? antiforgeryDefault) if (Antiforgery ?? antiforgeryDefault)

View File

@ -19,21 +19,15 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
foreach (var form in htmlDocument.Descendants("form")) foreach (var form in htmlDocument.Descendants("form"))
{ {
foreach (var attribute in form.Attributes()) foreach (var input in form.Descendants("input"))
{ {
if (string.Equals(attribute.Name.LocalName, "action", StringComparison.OrdinalIgnoreCase)) if (input.Attribute("name") != null &&
input.Attribute("type") != null &&
input.Attribute("type").Value == "hidden" &&
(input.Attribute("name").Value == "__RequestVerificationToken" ||
input.Attribute("name").Value == "HtmlEncode[[__RequestVerificationToken]]"))
{ {
foreach (var input in form.Descendants("input")) return input.Attributes("value").First().Value;
{
if (input.Attribute("name") != null &&
input.Attribute("type") != null &&
input.Attribute("type").Value == "hidden" &&
(input.Attribute("name").Value == "__RequestVerificationToken" ||
input.Attribute("name").Value == "HtmlEncode[[__RequestVerificationToken]]"))
{
return input.Attributes("value").First().Value;
}
}
} }
} }
} }

View File

@ -6,6 +6,8 @@ using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Net; using System.Net;
using System.Net.Http; using System.Net.Http;
using System.Net.Http.Headers;
using System.Reflection;
using System.Threading.Tasks; using System.Threading.Tasks;
using Xunit; using Xunit;
@ -13,6 +15,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
{ {
public class RazorPagesTest : IClassFixture<MvcTestFixture<RazorPagesWebSite.Startup>> public class RazorPagesTest : IClassFixture<MvcTestFixture<RazorPagesWebSite.Startup>>
{ {
private static readonly Assembly _resourcesAssembly = typeof(RazorPagesTest).GetTypeInfo().Assembly;
public RazorPagesTest(MvcTestFixture<RazorPagesWebSite.Startup> fixture) public RazorPagesTest(MvcTestFixture<RazorPagesWebSite.Startup> fixture)
{ {
Client = fixture.Client; Client = fixture.Client;
@ -20,6 +24,35 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
public HttpClient Client { get; } public HttpClient Client { get; }
[Fact]
public async Task Page_SimpleForms_RenderAntiforgery()
{
// Arrange
var expectedMediaType = MediaTypeHeaderValue.Parse("text/html; charset=utf-8");
var outputFile = "compiler/resources/RazorPagesWebSite.SimpleForms.html";
var expectedContent = await ResourceFile.ReadResourceAsync(_resourcesAssembly, outputFile, sourceFile: false);
// Act
var response = await Client.GetAsync("http://localhost/SimpleForms");
var responseContent = await response.Content.ReadAsStringAsync();
// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal(expectedMediaType, response.Content.Headers.ContentType);
responseContent = responseContent.Trim();
var forgeryToken = AntiforgeryTestHelper.RetrieveAntiforgeryToken(responseContent, "SimpleForms");
#if GENERATE_BASELINES
// Reverse usual substitution and insert a format item into the new file content.
responseContent = responseContent.Replace(forgeryToken, "{0}");
ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent);
#else
expectedContent = string.Format(expectedContent, forgeryToken);
Assert.Equal(expectedContent.Trim(), responseContent, ignoreLineEndingDifferences: true);
#endif
}
[Fact] [Fact]
public async Task Page_Handler_HandlerFromQueryString() public async Task Page_Handler_HandlerFromQueryString()
{ {

View File

@ -0,0 +1,6 @@
<form></form>
<form method="get"></form>
<form method="post"><input name="__RequestVerificationToken" type="hidden" value="{0}" /></form>
<form action="" method="post"><input name="__RequestVerificationToken" type="hidden" value="{0}" /></form>
<form action="/Foo/Bar/Baz.html" method="get"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>

View File

@ -25,6 +25,122 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{ {
public class FormTagHelperTest public class FormTagHelperTest
{ {
[Fact]
public async Task ProcessAsync_EmptyHtmlStringActionGeneratesAntiforgery()
{
// Arrange
var expectedTagName = "form";
var metadataProvider = new TestModelMetadataProvider();
var tagHelperContext = new TagHelperContext(
tagName: "form",
allAttributes: new TagHelperAttributeList()
{
{ "method", new HtmlString("post") }
},
items: new Dictionary<object, object>(),
uniqueId: "test");
var output = new TagHelperOutput(
expectedTagName,
attributes: new TagHelperAttributeList()
{
{ "action", HtmlString.Empty },
},
getChildContentAsync: (useCachedResult, encoder) =>
{
var tagHelperContent = new DefaultTagHelperContent();
tagHelperContent.SetContent("Something");
return Task.FromResult<TagHelperContent>(tagHelperContent);
});
var urlHelper = new Mock<IUrlHelper>();
urlHelper
.Setup(mock => mock.Action(It.IsAny<UrlActionContext>())).Returns("home/index");
var htmlGenerator = new TestableHtmlGenerator(metadataProvider, urlHelper.Object);
var viewContext = TestableHtmlGenerator.GetViewContext(
model: null,
htmlGenerator: htmlGenerator,
metadataProvider: metadataProvider);
var expectedPostContent = HtmlContentUtilities.HtmlContentToString(
htmlGenerator.GenerateAntiforgery(viewContext),
HtmlEncoder.Default);
var formTagHelper = new FormTagHelper(htmlGenerator)
{
ViewContext = viewContext,
Method = "post",
};
// Act
await formTagHelper.ProcessAsync(tagHelperContext, output);
// Assert
var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("action"));
Assert.Equal(HtmlString.Empty, attribute.Value);
Assert.Empty(output.PreElement.GetContent());
Assert.Empty(output.PreContent.GetContent());
Assert.Empty(output.Content.GetContent());
Assert.Equal(expectedPostContent, output.PostContent.GetContent());
Assert.Empty(output.PostElement.GetContent());
Assert.Equal(expectedTagName, output.TagName);
}
[Fact]
public async Task ProcessAsync_EmptyStringActionGeneratesAntiforgery()
{
// Arrange
var expectedTagName = "form";
var metadataProvider = new TestModelMetadataProvider();
var tagHelperContext = new TagHelperContext(
tagName: "form",
allAttributes: new TagHelperAttributeList()
{
{ "method", new HtmlString("post") }
},
items: new Dictionary<object, object>(),
uniqueId: "test");
var output = new TagHelperOutput(
expectedTagName,
attributes: new TagHelperAttributeList()
{
{ "action", string.Empty },
},
getChildContentAsync: (useCachedResult, encoder) =>
{
var tagHelperContent = new DefaultTagHelperContent();
tagHelperContent.SetContent("Something");
return Task.FromResult<TagHelperContent>(tagHelperContent);
});
var urlHelper = new Mock<IUrlHelper>();
urlHelper
.Setup(mock => mock.Action(It.IsAny<UrlActionContext>())).Returns("home/index");
var htmlGenerator = new TestableHtmlGenerator(metadataProvider, urlHelper.Object);
var viewContext = TestableHtmlGenerator.GetViewContext(
model: null,
htmlGenerator: htmlGenerator,
metadataProvider: metadataProvider);
var expectedPostContent = HtmlContentUtilities.HtmlContentToString(
htmlGenerator.GenerateAntiforgery(viewContext),
HtmlEncoder.Default);
var formTagHelper = new FormTagHelper(htmlGenerator)
{
ViewContext = viewContext,
Method = "post",
};
// Act
await formTagHelper.ProcessAsync(tagHelperContext, output);
// Assert
var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("action"));
Assert.Equal(string.Empty, attribute.Value);
Assert.Empty(output.PreElement.GetContent());
Assert.Empty(output.PreContent.GetContent());
Assert.Empty(output.Content.GetContent());
Assert.Equal(expectedPostContent, output.PostContent.GetContent());
Assert.Empty(output.PostElement.GetContent());
Assert.Equal(expectedTagName, output.TagName);
}
[Fact] [Fact]
public async Task ProcessAsync_GeneratesExpectedOutput() public async Task ProcessAsync_GeneratesExpectedOutput()
{ {

View File

@ -0,0 +1,9 @@
@page
@addTagHelper "*, Microsoft.AspNetCore.Mvc.TagHelpers"
<form></form>
<form method="get"></form>
<form method="post"></form>
<form action="" method="post"></form>
<form action="/Foo/Bar/Baz.html" method="get"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>