Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

FormTagHelper does not apply on Razor Pages unless asp-antiforgery is explicitly set to true #6006

Closed
DamianEdwards opened this issue Mar 22, 2017 · 7 comments

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Mar 22, 2017

The FormTagHelper only targets <form> elements if various attributes are present, e.g. asp-controller. The default behavior of it rendering anti-forgery token fields thus only applies when these attributes are present. When used in Razor Pages however, the normal attributes are not used (as we're posting to another page typically), so the anti-forgery fields are not rendered unless the asp-antiforgery attribute is explicitly set, which is far less than ideal.

<form method="post" class="form-horizontal" asp-antiforgery="true">
    <h4>Use a local account to log in.</h4>
    <hr />
    <div asp-validation-summary="All" class="text-danger"></div>
    ...
</form>

We want this to just work in Razor Pages, and it will be much more common there to post back to the current page, meaning no form action will need to be configured.

Shall we just make the FormTagHelper target all <form> elements by default now?

@rynowak @Eilon

@Eilon
Copy link
Member

Eilon commented Mar 23, 2017

Let's change the FormTagHelper to target all <form> elements without restrictions on which attributes it has.

@dougbu
Copy link
Member

dougbu commented Apr 9, 2017

The other option is to move antiforgery support into RenderAtEndOfFormTagHelper. That one already targets all <form> elements and already helps with antiforgery.

@rynowak
Copy link
Member

rynowak commented Apr 10, 2017

@Eilon says: We think that moving this feature (asp-antiforgery) would be a bigger breaking change than just running form tag helper all of the time

@rynowak
Copy link
Member

rynowak commented Apr 17, 2017

Load balancing

NTaylorMullen added a commit that referenced this issue Apr 18, 2017
- 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
@rynowak rynowak moved this from Ready to Working in Razor Pages 2.0.0-preview1 Apr 19, 2017
NTaylorMullen added a commit that referenced this issue Apr 19, 2017
- 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
NTaylorMullen added a commit that referenced this issue Apr 21, 2017
- 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
NTaylorMullen added a commit that referenced this issue Apr 21, 2017
- 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
NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- 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
NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- 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
@NTaylorMullen
Copy link
Member

NTaylorMullen commented Apr 25, 2017

5c8a161
ef0333a
f165914

@NTaylorMullen
Copy link
Member

@Eilon Are we announcing all MVC breaking changes together or do we want to do them as one-offs?

@rynowak rynowak moved this from Working to Done in Razor Pages 2.0.0-preview1 Apr 25, 2017
@Eilon
Copy link
Member

Eilon commented Apr 25, 2017

Generally using one-offs, unless there are closely-related items.

NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- Added a test case to the HtmlGeneration functional test (the one verifying complex FormTagHelpers).
- Added unit test verifying antiforgery behavior when it's the only provided parameter.

#6006
NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- Added functional test to verify `asp-*` attributes on form taghelpers work as expected.
- Added a unit test to validate `FormTagHelper` behaves as expected.
- Moved `Method == "get"` checks into appropriate code paths. These include the one where a user specifies an empty or non-existent `action` attribute and where a user doesn't utilize any `asp-*` attributes. In the later, we default `Method` to `"get"` if it's not provided.

#6006
NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- Added functional test to verify `asp-*` attributes on form taghelpers work as expected.
- Added a unit test to validate `FormTagHelper` behaves as expected.
- Moved `Method == "get"` checks into appropriate code paths. These include the one where a user specifies an empty or non-existent `action` attribute and where a user doesn't utilize any `asp-*` attributes. In the later, we default `Method` to `"get"` if it's not provided.

#6006
NTaylorMullen added a commit that referenced this issue Apr 25, 2017
- Added functional test to verify `asp-*` attributes on form taghelpers work as expected.
- Added a unit test to validate `FormTagHelper` behaves as expected.
- Moved `Method == "get"` checks into appropriate code paths. These include the one where a user specifies an empty or non-existent `action` attribute and where a user doesn't utilize any `asp-*` attributes. In the later, we default `Method` to `"get"` if it's not provided.

#6006
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

6 participants