From 1184d738cc9ba814f27b8e34be0f55b1a95d531e Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Mon, 18 Jul 2016 16:25:13 -0700 Subject: [PATCH] Always render antiforgery tokens if `!CanRenderAtEndOfForm` - #5005 - also add `FormContext` doc comments --- .../ViewFeatures/DefaultHtmlGenerator.cs | 18 +++--- .../ViewFeatures/FormContext.cs | 62 ++++++++++++++++++- .../ViewFeatures/DefaultHtmlGeneratorTest.cs | 21 +++++++ 3 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs index 9feec71ef2..1b6588784a 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs @@ -156,15 +156,19 @@ public virtual IHtmlContent GenerateAntiforgery(ViewContext viewContext) throw new ArgumentNullException(nameof(viewContext)); } - // If we're inside a BeginForm/BeginRouteForm, the antiforgery token might have already been - // created and appended to the 'end form' content OR the form tag helper might have already generated - // an antiforgery token. - if (viewContext.FormContext.HasAntiforgeryToken) + var formContext = viewContext.FormContext; + if (formContext.CanRenderAtEndOfForm) { - return HtmlString.Empty; - } + // Inside a BeginForm/BeginRouteForm or a
tag helper. So, the antiforgery token might have + // already been created and appended to the 'end form' content (the AntiForgeryToken HTML helper does + // this) OR the tag helper might have already generated an antiforgery token. + if (formContext.HasAntiforgeryToken) + { + return HtmlString.Empty; + } - viewContext.FormContext.HasAntiforgeryToken = true; + formContext.HasAntiforgeryToken = true; + } return _antiforgery.GetHtml(viewContext.HttpContext); } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/FormContext.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/FormContext.cs index 2a7460f6fb..42b928c710 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/FormContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/FormContext.cs @@ -7,6 +7,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures { + /// + /// Collection of information about the current <form>. + /// + /// + /// Literal <form> elements in a view will share the default instance unless tag + /// helpers are enabled. + /// public class FormContext { private Dictionary _renderedFields; @@ -14,7 +21,7 @@ public class FormContext private IList _endOfFormContent; /// - /// Property bag for any information you wish to associate with a <form/> in an + /// Gets a property bag for any information you wish to associate with a <form/> in an /// implementation or extension method. /// public IDictionary FormData @@ -30,12 +37,36 @@ public IDictionary FormData } } + /// + /// Gets or sets an indication the current <form> element contains an antiforgery token. Do not use + /// unless is true. + /// + /// + /// true if the current <form> element contains an antiforgery token; false otherwise. + /// public bool HasAntiforgeryToken { get; set; } + /// + /// Gets an indication the property bag has been used and likely contains entries. + /// + /// + /// true if the backing field for is non-null; false otherwise. + /// public bool HasFormData => _formData != null; + /// + /// Gets an indication the collection has been used and likely contains entries. + /// + /// + /// true if the backing field for is non-null; false + /// otherwise. + /// public bool HasEndOfFormContent => _endOfFormContent != null; + /// + /// Gets an collection that should be rendered just prior to the next </form> + /// end tag. Do not use unless is true. + /// public IList EndOfFormContent { get @@ -49,8 +80,23 @@ public IList EndOfFormContent } } + /// + /// Gets or sets an indication the current <form> is associated with a tag helper or will be generated by + /// an HTML helper. In particular, will be rendered just prior to the next + /// </form> end tag. + /// + /// + /// true if the framework will render ; false otherwise. + /// + /// + /// true for all framework-created instances except the default. + /// public bool CanRenderAtEndOfForm { get; set; } + /// + /// Gets a dictionary mapping full HTML field names to indications that the named field has been rendered in + /// this <form>. + /// private Dictionary RenderedFields { get @@ -64,6 +110,14 @@ private Dictionary RenderedFields } } + /// + /// Returns an indication based on that the given has + /// been rendered in this <form>. + /// + /// The full HTML name of a field that may have been rendered. + /// + /// true if the given has been rendered; false otherwise. + /// public bool RenderedField(string fieldName) { if (fieldName == null) @@ -77,6 +131,12 @@ public bool RenderedField(string fieldName) return result; } + /// + /// Updates to indicate has been rendered in this + /// <form>. + /// + /// The full HTML name of a field that may have been rendered. + /// If true, the given has been rendered. public void RenderedField(string fieldName, bool value) { if (fieldName == null) diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/DefaultHtmlGeneratorTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/DefaultHtmlGeneratorTest.cs index 933623f246..b415925b0f 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/DefaultHtmlGeneratorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/DefaultHtmlGeneratorTest.cs @@ -641,6 +641,7 @@ public void GenerateAntiforgery_GeneratesAntiforgeryFieldsOnlyIfRequired( var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var htmlGenerator = GetGenerator(metadataProvider); var viewContext = GetViewContext(model: null, metadataProvider: metadataProvider); + viewContext.FormContext.CanRenderAtEndOfForm = true; viewContext.FormContext.HasAntiforgeryToken = hasAntiforgeryToken; // Act @@ -651,6 +652,26 @@ public void GenerateAntiforgery_GeneratesAntiforgeryFieldsOnlyIfRequired( Assert.Equal(expectedAntiforgeryHtmlField, antiforgeryField); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void GenerateAntiforgery_AlwaysGeneratesAntiforgeryToken_IfCannotRenderAtEnd(bool hasAntiforgeryToken) + { + // Arrange + var expected = ""; + var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var htmlGenerator = GetGenerator(metadataProvider); + var viewContext = GetViewContext(model: null, metadataProvider: metadataProvider); + viewContext.FormContext.HasAntiforgeryToken = hasAntiforgeryToken; + + // Act + var result = htmlGenerator.GenerateAntiforgery(viewContext); + + // Assert + var antiforgeryField = HtmlContentUtilities.HtmlContentToString(result, HtmlEncoder.Default); + Assert.Equal(expected, antiforgeryField); + } + // GetCurrentValues uses only the IModelMetadataProvider passed to the DefaultHtmlGenerator constructor. private static IHtmlGenerator GetGenerator(IModelMetadataProvider metadataProvider) {