From f0777b95a84583477bc53426e438116f0a0673e3 Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Thu, 14 Jan 2016 16:21:16 -0800 Subject: [PATCH] [Fixes #3868] Exclude Antiforgery token in form with method Get --- .../FormTagHelper.cs | 20 +- .../Rendering/HtmlHelperFormExtensions.cs | 78 +++--- .../Rendering/IHtmlHelper.cs | 19 +- .../ViewFeatures/HtmlHelper.cs | 34 ++- ...e.HtmlGeneration_Home.Product.Encoded.html | 2 +- .../FormTagHelperTest.cs | 17 +- .../Rendering/HtmlHelperFormExtensionsTest.cs | 114 +++++--- .../Rendering/HtmlHelperFormTest.cs | 264 +++++++++++++++++- .../DefaultEditorTemplatesTest.cs | 4 +- .../Views/RemoteAttribute_Home/Create.cshtml | 2 +- .../Views/RemoteAttribute_Home/Create.cshtml | 2 +- .../Views/FlushPoint/PageWithoutLayout.cshtml | 2 +- 12 files changed, 442 insertions(+), 116 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/FormTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/FormTagHelper.cs index 82c51527a9..431020ae0c 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/FormTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/FormTagHelper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Mvc.ViewFeatures; using Microsoft.AspNet.Razor.TagHelpers; @@ -68,7 +69,8 @@ namespace Microsoft.AspNet.Mvc.TagHelpers /// /// Whether the antiforgery token should be generated. /// - /// Defaults to false if user provides an action attribute; true otherwise. + /// Defaults to false if user provides an action attribute + /// or if the method is ; true otherwise. [HtmlAttributeName(AntiforgeryAttributeName)] public bool? Antiforgery { get; set; } @@ -81,6 +83,13 @@ namespace Microsoft.AspNet.Mvc.TagHelpers [HtmlAttributeName(RouteAttributeName)] public string Route { get; set; } + /// + /// The HTTP method to use. + /// + /// Passed through to the generated HTML in all cases. + [EditorBrowsable(EditorBrowsableState.Never)] + public string Method { get; set; } + /// /// Additional parameters for the route. /// @@ -122,6 +131,10 @@ namespace Microsoft.AspNet.Mvc.TagHelpers { throw new ArgumentNullException(nameof(output)); } + if (Method != null) + { + output.CopyHtmlAttribute(nameof(Method), context); + } var antiforgeryDefault = true; @@ -195,6 +208,11 @@ namespace Microsoft.AspNet.Mvc.TagHelpers output.MergeAttributes(tagBuilder); output.PostContent.AppendHtml(tagBuilder.InnerHtml); } + + if (string.Equals(Method, FormMethod.Get.ToString(), StringComparison.OrdinalIgnoreCase)) + { + antiforgeryDefault = false; + } } if (Antiforgery ?? antiforgeryDefault) diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperFormExtensions.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperFormExtensions.cs index 34d85ff34d..a0cdd2ae82 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperFormExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperFormExtensions.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -43,9 +43,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// match the current request. /// /// The instance this method extends. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An instance which renders the </form> end tag when disposed. @@ -53,7 +55,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// In this context, "renders" means the method writes its output using . /// - public static MvcForm BeginForm(this IHtmlHelper htmlHelper, bool suppressAntiforgery) + public static MvcForm BeginForm(this IHtmlHelper htmlHelper, bool? antiforgery) { if (htmlHelper == null) { @@ -66,7 +68,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: suppressAntiforgery, + antiforgery: antiforgery, htmlAttributes: null); } @@ -94,7 +96,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -130,7 +132,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: htmlAttributes); } @@ -140,9 +142,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// The instance this method extends. /// The HTTP method for processing the form, either GET or POST. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An that contains the HTML attributes for the element. Alternatively, an @@ -158,7 +162,7 @@ namespace Microsoft.AspNet.Mvc.Rendering public static MvcForm BeginForm( this IHtmlHelper htmlHelper, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes) { if (htmlHelper == null) @@ -171,7 +175,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: method, - suppressAntiforgery: suppressAntiforgery, + antiforgery: antiforgery, htmlAttributes: htmlAttributes); } @@ -205,7 +209,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: routeValues, method: FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -237,7 +241,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -277,7 +281,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues, FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -311,7 +315,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -353,7 +357,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues, method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -393,7 +397,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: htmlAttributes); } @@ -426,7 +430,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName: null, routeValues: routeValues, method: FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -442,9 +446,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// instance containing the route /// parameters. /// - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An instance which renders the </form> end tag when disposed. @@ -452,7 +458,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// In this context, "renders" means the method writes its output using . /// - public static MvcForm BeginRouteForm(this IHtmlHelper htmlHelper, object routeValues, bool suppressAntiforgery) + public static MvcForm BeginRouteForm(this IHtmlHelper htmlHelper, object routeValues, bool? antiforgery) { if (htmlHelper == null) { @@ -463,7 +469,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName: null, routeValues: routeValues, method: FormMethod.Post, - suppressAntiforgery: suppressAntiforgery, + antiforgery: antiforgery, htmlAttributes: null); } @@ -490,7 +496,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -500,9 +506,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// The instance this method extends. /// The name of the route. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An instance which renders the </form> end tag when disposed. @@ -510,7 +518,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// In this context, "renders" means the method writes its output using . /// - public static MvcForm BeginRouteForm(this IHtmlHelper htmlHelper, string routeName, bool suppressAntiforgery) + public static MvcForm BeginRouteForm(this IHtmlHelper htmlHelper, string routeName, bool? antiforgery) { if (htmlHelper == null) { @@ -521,7 +529,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: suppressAntiforgery, + antiforgery: antiforgery, htmlAttributes: null); } @@ -558,7 +566,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues, FormMethod.Post, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -589,7 +597,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -628,7 +636,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues, method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: null); } @@ -665,7 +673,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: null, htmlAttributes: htmlAttributes); } } diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs index babb080eaa..bdcde2e7a9 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs @@ -114,9 +114,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// instance containing the route parameters. /// /// The HTTP method for processing the form, either GET or POST. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An that contains the HTML attributes for the element. Alternatively, an @@ -133,7 +135,7 @@ namespace Microsoft.AspNet.Mvc.Rendering string controllerName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes); /// @@ -148,8 +150,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// instance containing the route parameters. /// /// The HTTP method for processing the form, either GET or POST. - /// - /// Determines whether or not to include an <input> of type "hidden" with an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An that contains the HTML attributes for the element. Alternatively, an @@ -165,7 +170,7 @@ namespace Microsoft.AspNet.Mvc.Rendering string routeName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes); /// diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs index 7d0ce01fa7..29f283453b 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs @@ -288,7 +288,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures string controllerName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes) { // Push the new FormContext; MvcForm.GenerateEndForm() does the corresponding pop. @@ -297,7 +297,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures CanRenderAtEndOfForm = true }; - return GenerateForm(actionName, controllerName, routeValues, method, suppressAntiforgery, htmlAttributes); + return GenerateForm(actionName, controllerName, routeValues, method, antiforgery, htmlAttributes); } /// @@ -305,7 +305,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures string routeName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes) { // Push the new FormContext; MvcForm.GenerateEndForm() does the corresponding pop. @@ -314,7 +314,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures CanRenderAtEndOfForm = true }; - return GenerateRouteForm(routeName, routeValues, method, suppressAntiforgery, htmlAttributes); + return GenerateRouteForm(routeName, routeValues, method, antiforgery, htmlAttributes); } /// @@ -871,9 +871,11 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures /// instance containing the route parameters. /// /// The HTTP method for processing the form, either GET or POST. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An that contains the HTML attributes for the element. Alternatively, an @@ -890,7 +892,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures string controllerName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes) { var tagBuilder = _htmlGenerator.GenerateForm( @@ -906,7 +908,8 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); } - if (!suppressAntiforgery) + var shouldGenerateAntiforgery = antiforgery.HasValue ? antiforgery.Value : method != FormMethod.Get; + if (shouldGenerateAntiforgery) { ViewContext.FormContext.EndOfFormContent.Add(_htmlGenerator.GenerateAntiforgery(ViewContext)); ViewContext.FormContext.HasAntiforgeryToken = true; @@ -927,9 +930,11 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures /// instance containing the route parameters. /// /// The HTTP method for processing the form, either GET or POST. - /// - /// If true, suppresses the generation an <input> of type "hidden" with an antiforgery token. By - /// default <form> elements will automatically include an antiforgery token. + /// + /// If true, <form> elements will include an antiforgery token. + /// If false, suppresses the generation an <input> of type "hidden" with an antiforgery token. + /// If null, <form> elements will include an antiforgery token only if + /// is not . /// /// /// An that contains the HTML attributes for the element. Alternatively, an @@ -945,7 +950,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures string routeName, object routeValues, FormMethod method, - bool suppressAntiforgery, + bool? antiforgery, object htmlAttributes) { var tagBuilder = _htmlGenerator.GenerateRouteForm( @@ -960,7 +965,8 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); } - if (!suppressAntiforgery) + var shouldGenerateAntiforgery = antiforgery.HasValue ? antiforgery.Value : method != FormMethod.Get; + if (shouldGenerateAntiforgery) { ViewContext.FormContext.EndOfFormContent.Add(_htmlGenerator.GenerateAntiforgery(ViewContext)); ViewContext.FormContext.HasAntiforgeryToken = true; diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html index 8e76207964..d40a62e45e 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html @@ -4,7 +4,7 @@ -
+
diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/FormTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/FormTagHelperTest.cs index 32c8f44c4c..c5f70a818c 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/FormTagHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/FormTagHelperTest.cs @@ -99,18 +99,23 @@ namespace Microsoft.AspNet.Mvc.TagHelpers } [Theory] - [InlineData(null, "")] - [InlineData(true, "")] - [InlineData(false, "")] + [InlineData(null, FormMethod.Post, "")] + [InlineData(true, FormMethod.Post, "")] + [InlineData(false, FormMethod.Post, "")] + [InlineData(null, FormMethod.Get, "")] + [InlineData(true, FormMethod.Get, "")] + [InlineData(false, FormMethod.Get, "")] public async Task ProcessAsync_GeneratesAntiforgeryCorrectly( bool? antiforgery, + FormMethod method, string expectedPostContent) { // Arrange var viewContext = CreateViewContext(); + var expectedAttribute = new TagHelperAttribute("method", method.ToString().ToLowerInvariant()); var context = new TagHelperContext( allAttributes: new ReadOnlyTagHelperAttributeList( - Enumerable.Empty()), + new ReadOnlyTagHelperAttributeList(new List { expectedAttribute })), items: new Dictionary(), uniqueId: "test"); var output = new TagHelperOutput( @@ -140,6 +145,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers Action = "Index", Antiforgery = antiforgery, ViewContext = viewContext, + Method = method.ToString().ToLowerInvariant() }; // Act @@ -148,7 +154,8 @@ namespace Microsoft.AspNet.Mvc.TagHelpers // Assert Assert.Equal("form", output.TagName); Assert.Equal(TagMode.StartTagAndEndTag, output.TagMode); - Assert.Empty(output.Attributes); + var attribute = Assert.Single(output.Attributes); + Assert.Equal(expectedAttribute, attribute); Assert.Empty(output.PreContent.GetContent()); Assert.True(output.Content.IsEmpty); Assert.Equal(expectedPostContent, output.PostContent.GetContent()); diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormExtensionsTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormExtensionsTest.cs index 508eae10d6..1bbf49370a 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormExtensionsTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormExtensionsTest.cs @@ -338,7 +338,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginForm(suppressAntiforgery: false); + var mvcForm = htmlHelper.BeginForm(antiforgery: true); // Assert Assert.NotNull(mvcForm); @@ -371,7 +371,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginForm(suppressAntiforgery: true); + var mvcForm = htmlHelper.BeginForm(antiforgery: false); // Assert Assert.NotNull(mvcForm); @@ -397,10 +397,14 @@ namespace Microsoft.AspNet.Mvc.Rendering null)) // htmlAttributes .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -437,10 +441,14 @@ namespace Microsoft.AspNet.Mvc.Rendering htmlAttributes)) .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -489,7 +497,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginForm(method, suppressAntiforgery: false, htmlAttributes: htmlAttributes); + var mvcForm = htmlHelper.BeginForm(method, antiforgery: true, htmlAttributes: htmlAttributes); // Assert Assert.NotNull(mvcForm); @@ -525,7 +533,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginForm(method, suppressAntiforgery: true, htmlAttributes: htmlAttributes); + var mvcForm = htmlHelper.BeginForm(method, antiforgery: false, htmlAttributes: htmlAttributes); // Assert Assert.NotNull(mvcForm); @@ -673,10 +681,14 @@ namespace Microsoft.AspNet.Mvc.Rendering null)) // htmlAttributes .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -715,10 +727,14 @@ namespace Microsoft.AspNet.Mvc.Rendering null)) // htmlAttributes .Returns(tagBuilder) .Verifiable(); - htmlGenerator + + if (method != FormMethod.Get) + { + htmlGenerator .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) .Returns(HtmlString.Empty) .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -757,10 +773,14 @@ namespace Microsoft.AspNet.Mvc.Rendering htmlAttributes)) .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -816,7 +836,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: true, htmlAttributes: htmlAttributes); // Assert @@ -860,7 +880,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues: null, method: method, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: htmlAttributes); // Assert @@ -936,7 +956,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginRouteForm(routeValues, suppressAntiforgery: false); + var mvcForm = htmlHelper.BeginRouteForm(routeValues, antiforgery: true); // Assert Assert.NotNull(mvcForm); @@ -970,7 +990,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginRouteForm(routeValues, suppressAntiforgery: true); + var mvcForm = htmlHelper.BeginRouteForm(routeValues, antiforgery: false); // Assert Assert.NotNull(mvcForm); @@ -1045,7 +1065,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginRouteForm(routeName, suppressAntiforgery: false); + var mvcForm = htmlHelper.BeginRouteForm(routeName, antiforgery: true); // Assert Assert.NotNull(mvcForm); @@ -1079,7 +1099,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(builder); // Act - var mvcForm = htmlHelper.BeginRouteForm(routeName, suppressAntiforgery: true); + var mvcForm = htmlHelper.BeginRouteForm(routeName, antiforgery: false); // Assert Assert.NotNull(mvcForm); @@ -1145,10 +1165,14 @@ namespace Microsoft.AspNet.Mvc.Rendering null)) // htmlAttributes .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -1185,10 +1209,14 @@ namespace Microsoft.AspNet.Mvc.Rendering null)) // htmlAttributes .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -1225,10 +1253,14 @@ namespace Microsoft.AspNet.Mvc.Rendering htmlAttributes)) .Returns(tagBuilder) .Verifiable(); - htmlGenerator - .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) - .Returns(HtmlString.Empty) - .Verifiable(); + + if (method != FormMethod.Get) + { + htmlGenerator + .Setup(g => g.GenerateAntiforgery(htmlHelper.ViewContext)) + .Returns(HtmlString.Empty) + .Verifiable(); + } // Guards Assert.NotNull(htmlHelper.ViewContext); @@ -1281,7 +1313,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: method, - suppressAntiforgery: false, + antiforgery: true, htmlAttributes: htmlAttributes); // Assert @@ -1322,7 +1354,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues: null, method: method, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: htmlAttributes); // Assert diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormTest.cs index 0c1134afea..fbfea95fd8 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperFormTest.cs @@ -151,7 +151,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: null); // Assert @@ -201,7 +201,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName: null, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: htmlAttributes); // Assert @@ -254,7 +254,7 @@ namespace Microsoft.AspNet.Mvc.Rendering controllerName, routeValues, method, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: htmlAttributes); // Assert @@ -302,7 +302,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName, routeValues, method, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: htmlAttributes); // Assert @@ -404,6 +404,45 @@ namespace Microsoft.AspNet.Mvc.Rendering writer.GetStringBuilder().ToString()); } + [Fact] + public void BeginForm_EndForm_RendersAntiforgeryTokenWhenMethodIsPost() + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginForm(FormMethod.Post, antiforgery: null, htmlAttributes: null)) + { + Assert.True(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "", + writer.GetStringBuilder().ToString()); + } + // This is an integration for the implicit antiforgery token added by BeginForm. [Fact] public void BeginForm_EndForm_RendersAntiforgeryToken_WithExplicitCallToAntiforgery() @@ -477,7 +516,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(writer); // Act & Assert - using (var form = htmlHelper.BeginForm(FormMethod.Post, suppressAntiforgery: true, htmlAttributes: null)) + using (var form = htmlHelper.BeginForm(FormMethod.Post, antiforgery: false, htmlAttributes: null)) { Assert.False(viewContext.FormContext.HasAntiforgeryToken); } @@ -487,6 +526,86 @@ namespace Microsoft.AspNet.Mvc.Rendering writer.GetStringBuilder().ToString()); } + [Fact] + public void BeginForm_EndForm_SuppressAntiforgeryTokenWhenMethodIsGet() + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginForm(FormMethod.Get, antiforgery: null, htmlAttributes: null)) + { + Assert.False(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "
", + writer.GetStringBuilder().ToString()); + } + + [Theory] + [InlineData(FormMethod.Get)] + [InlineData(FormMethod.Post)] + public void BeginForm_EndForm_DoesNotSuppressAntiforgeryTokenWhenAntiforgeryIsTrue(FormMethod method) + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginForm(method, antiforgery: true, htmlAttributes: null)) + { + Assert.True(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "
", + writer.GetStringBuilder().ToString()); + } + // This is an integration for suppressing implicit antiforgery token added by BeginForm. [Fact] public void BeginForm_EndForm_SuppressAntiforgeryToken_WithExplicitCallToAntiforgery() @@ -517,7 +636,7 @@ namespace Microsoft.AspNet.Mvc.Rendering Assert.NotNull(writer); // Act & Assert - using (var form = htmlHelper.BeginForm(FormMethod.Post, suppressAntiforgery: true, htmlAttributes: null)) + using (var form = htmlHelper.BeginForm(FormMethod.Post, antiforgery: false, htmlAttributes: null)) { Assert.False(viewContext.FormContext.HasAntiforgeryToken); @@ -569,6 +688,49 @@ namespace Microsoft.AspNet.Mvc.Rendering writer.GetStringBuilder().ToString()); } + [Fact] + public void BeginRouteForm_EndForm_RendersAntiforgeryTokenWhenMethodIsPost() + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateRouteForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginRouteForm( + routeName: null, + routeValues: null, + method: FormMethod.Post, + antiforgery: null, + htmlAttributes: null)) + { + Assert.True(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "
", + writer.GetStringBuilder().ToString()); + } + // This is an integration for suppressing implicit antiforgery token added by BeginRouteForm. [Fact] public void BeginRouteForm_EndForm_SuppressAntiforgeryToken() @@ -602,7 +764,7 @@ namespace Microsoft.AspNet.Mvc.Rendering routeName: null, routeValues: null, method: FormMethod.Post, - suppressAntiforgery: true, + antiforgery: false, htmlAttributes: null)) { Assert.False(viewContext.FormContext.HasAntiforgeryToken); @@ -613,6 +775,94 @@ namespace Microsoft.AspNet.Mvc.Rendering writer.GetStringBuilder().ToString()); } + [Fact] + public void BeginRouteForm_EndForm_SuppressAntiforgeryTokenWhenMethodIsGet() + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateRouteForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginRouteForm( + routeName: null, + routeValues: null, + method: FormMethod.Get, + antiforgery: null, + htmlAttributes: null)) + { + Assert.False(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "
", + writer.GetStringBuilder().ToString()); + } + + [Theory] + [InlineData(FormMethod.Get)] + [InlineData(FormMethod.Post)] + public void BeginRouteForm_EndForm_DoesNotSuppressAntiforgeryTokenWhenAntiforgeryIsTrue(FormMethod method) + { + // Arrange + var htmlGenerator = new Mock(MockBehavior.Strict); + htmlGenerator + .Setup(g => g.GenerateRouteForm( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(new TagBuilder("form")); + + htmlGenerator + .Setup(g => g.GenerateAntiforgery(It.IsAny())) + .Returns(new TagBuilder("antiforgery")); + + var htmlHelper = DefaultTemplatesUtilities.GetHtmlHelper(htmlGenerator.Object); + var serviceProvider = new Mock(); + serviceProvider.Setup(s => s.GetService(typeof(HtmlEncoder))).Returns(new HtmlTestEncoder()); + var viewContext = htmlHelper.ViewContext; + viewContext.HttpContext.RequestServices = serviceProvider.Object; + + var writer = viewContext.Writer as StringWriter; + Assert.NotNull(writer); + + // Act & Assert + using (var form = htmlHelper.BeginRouteForm( + routeName: null, + routeValues: null, + method: method, + antiforgery: true, + htmlAttributes: null)) + { + Assert.True(viewContext.FormContext.HasAntiforgeryToken); + } + + Assert.Equal( + "
", + writer.GetStringBuilder().ToString()); + } + private string GetHtmlAttributesAsString(object htmlAttributes) { var dictionary = HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes); diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/DefaultEditorTemplatesTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/DefaultEditorTemplatesTest.cs index b47ac79700..2a4f21a689 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/DefaultEditorTemplatesTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ViewFeatures/DefaultEditorTemplatesTest.cs @@ -1002,7 +1002,7 @@ Environment.NewLine; string controllerName, object routeValues, FormMethod method, - bool antiforgery, + bool? antiforgery, object htmlAttributes) { throw new NotImplementedException(); @@ -1012,7 +1012,7 @@ Environment.NewLine; string routeName, object routeValues, FormMethod method, - bool antiforgery, + bool? antiforgery, object htmlAttributes) { throw new NotImplementedException(); diff --git a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml index 0ffd8d7c8b..7367c6d941 100644 --- a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml +++ b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml @@ -8,7 +8,7 @@ }

@ViewBag.Title

-@using (Html.BeginForm(FormMethod.Post, htmlAttributes: null, suppressAntiforgery: true)) +@using (Html.BeginForm(FormMethod.Post, htmlAttributes: null, antiforgery: false)) {

Person

diff --git a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml index 0ffd8d7c8b..7367c6d941 100644 --- a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml +++ b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml @@ -8,7 +8,7 @@ }

@ViewBag.Title

-@using (Html.BeginForm(FormMethod.Post, htmlAttributes: null, suppressAntiforgery: true)) +@using (Html.BeginForm(FormMethod.Post, htmlAttributes: null, antiforgery: false)) {

Person

diff --git a/test/WebSites/RazorWebSite/Views/FlushPoint/PageWithoutLayout.cshtml b/test/WebSites/RazorWebSite/Views/FlushPoint/PageWithoutLayout.cshtml index 05cfe01c41..da35bbecaa 100644 --- a/test/WebSites/RazorWebSite/Views/FlushPoint/PageWithoutLayout.cshtml +++ b/test/WebSites/RazorWebSite/Views/FlushPoint/PageWithoutLayout.cshtml @@ -5,7 +5,7 @@ Secondary content @{ await Html.RenderPartialAsync("_PartialWithFlush"); } -@using (Html.BeginForm(method: FormMethod.Post, suppressAntiforgery: true, htmlAttributes: null)) +@using (Html.BeginForm(method: FormMethod.Post, antiforgery: false, htmlAttributes: null)) { @Html.TextBox("Name1") @await FlushAsync()