craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.22k stars 626 forks source link

[5.x]: actionUrl('users/send-password-reset-email') returns 404 #15596

Closed masiorama closed 1 week ago

masiorama commented 2 weeks ago

What happened?

Cannot make recovery password form with dedicated craft endpoint work with actionUrl while it does work with actionInput.

Steps to reproduce

  1. Install craft cms via shell, following instructions: composer create-project craftcms/craft my-craft-project
  2. On the template/index page add the form below, which does work (submit form with empty text leads to error message, which is fine):
<form method="post" accept-charset="UTF-8">
  {{ csrfInput() }}
  {{ actionInput('users/send-password-reset-email') }}

  <label for="loginName">Username or email</label>
  {{ input('text', 'loginName', loginName ?? craft.app.user.rememberedUsername, {
    id: 'loginName',
  }) }}

  {% if errors is defined %}
    {{ ul(errors, {class: 'errors'}) }}
  {% endif %}

  <button>Submit</button>
</form>

Remove actionInput for actionUrl, as follows.

<form method="post" accept-charset="UTF-8" action="{{ actionUrl('users/send-password-reset-email') }}">
  {{ csrfInput() }}

  <label for="loginName">Username or email</label>
  {{ input('text', 'loginName', loginName ?? craft.app.user.rememberedUsername, {
    id: 'loginName',
  }) }}

  {% if errors is defined %}
    {{ ul(errors, {class: 'errors'}) }}
  {% endif %}

  <button>Submit</button>
</form>

The latter doesn't work (submit form with empty text leads to 404), or I am missing something huge here.

Expected behavior

The two form above should be equivalent.

Actual behavior

They are not.

Craft CMS version

5

PHP version

8,2

Operating system and version

No response

Database type and version

mysql 8

Image driver and version

No response

Installed plugins and versions

-

brandonkelly commented 2 weeks ago

Guessing that the action is resolving correctly, but Craft just doesn’t know where to redirect the request afterward (which defaults to the requested URL).

Try adding this to the form:

{{ redirectInput('some/path') }}
masiorama commented 2 weeks ago

Hi @brandonkelly thanks for your support. I added the {{ redirectInput('some/path') }} and get 2 different behaviors depending on whether the input field nickname is compiled or not.

  1. the input field is filled with any char -> the form works and the user is redirected to the some/path page.
  2. the input field is blank -> the form keeps returning the same 404 error.

In the second case I would expect to be redirected to the form page with the "input cannot be black" user message .

brandonkelly commented 2 weeks ago

The way this typically works is:

  1. Form is submitted from /some/url and has an empty action="" attribute.
  2. Controller action fails (e.g. due to validation) and sets the invalid user to a route variable. Returns null so Craft knows to continue routing the request based on the requested URL.
  3. The original template is re-rendered (via the requested URL) and passed in the invalid user model, to surface any validation errors on it.

When you submit the form with a different action attribute, there is no normal template route to fall back to when the controller fails. Even if we were to allow you to pass an alternate redirect URL in the case of failure, it still wouldn’t be perfect because you’d end up losing access to the invalid user model in the process.

So in hindsight, setting the controller action via the action form attribute was not a good suggestion on my part (#15041), at least for actions that need to do validation.

Maybe a better solution to #15041 would be to set the action input to the current page’s URL, and still set the action input within the form like you normally would:

<form method="post" accept-charset="UTF-8" action="{{ craft.app.request.absoluteUrl }}">
  {{ csrfInput() }}
  {{ actionInput('users/send-password-reset-email') }}
  ...
</form>
masiorama commented 1 week ago

Hello @brandonkelly thanks, this way it seems it works, at least in the simple scenario I set. I'm gonna try and change, in my project, all forms (user recovery password, user registration, cart form, checkout form, address forms, ...) and see if it works, and also implementing GA4 with Consent Mode V2 which on the first hand is giving troubles due to the fact that it messes up with forms' action fields. It's since June that I have to delivery this project and it was all set and ready to go live, but this problem with forms was a total bummer, so thanks for the support. I'll keep you posted.

brandonkelly commented 1 week ago

@masiorama Thanks for verifying it works! I’ve updated #15041 and the Stack Exchange post with that suggestion.

masiorama commented 1 week ago

@brandonkelly sorry to bother, but I only checked that the form was working, and it does now but, at soon as I applied again Google Analytics tracking with Google Consent Mode V2 and iubenda tracking (which I removed to simplify things for this issue), the issue with form is back again. I guess I will have to dig more. Btw, 404 in this issue scenario is solved.

Edit to clarify: this solution with just GA GCMv2 tracking works (since the tracking is accepted), while the issue is still persistent when Iubenda is installed too and cookies are rejected.

brandonkelly commented 1 week ago

What exactly are you seeing?

masiorama commented 1 week ago

@brandonkelly what you can test here: https://sale.colombomilano1911.com/shop/customer/forgot-password Reject cookies ("Rifiuta cookies") on the banner then submit the form with empty value, still the infamous 403 we already mentioned, with the url broken as DOM object is attached to the url, like this https://sale.colombomilano1911.com/shop/customer/[object%20HTMLInputElement]?_gl=1*1rat8o4*_up*MQ..*_ga*NDU0MTU0NzE3LjE3MjUzNzA4Mjc.*_ga_MH6GBKQWM8*MTcyNTM3MDgyNy4xLjAuMTcyNTM3MDgyNy4wLjAuMA.. ... please note the [object%20HTMLInputElement].

edit: I prepared an .md file to explain to Iubenda a way to replicate the error on dev env, which involves also the installing of the cookies. Hope they will follow.

brandonkelly commented 1 week ago

Very weird. Could this be some sort of caching bug? I’m not sure where they would even be getting that https://sale.colombomilano1911.com/shop/customer/ URL they are appending the [object… bit to. The page source has zero instances of that URL on its own, without either forgot-password or sign-in appended to it.

masiorama commented 1 week ago

I don't think caching is involved, since I can replicate the same issue on my dev env which doesn't involve any type of it.

I read here https://support.google.com/google-ads/thread/173640313/url-passthrough-gclid-is-breaking-form-actions?hl=en that it could be the way rejected GA cookies could break forms actions, but still cannot go deeper than that.

edit: I can replicate also from a basic craft installation with just a few steps. If I can send this to you/your team to investigate that would be awesome.

brandonkelly commented 1 week ago

If you send that over we can throw an extra pair of eyes on it, but no promises since it sounds like a bug with the JS script.

masiorama commented 1 week ago

@brandonkelly email just sent to support. I know that this is a bit outside of your scope of intervention but I am glad of any help, even know that it's 100% not craft cms fault would help to pinpoint the issue with iubenda tech guys. Knowing that the scenario described, how to replicate it and that there are no errors from craft point of view is still critical to me, and would help me with my client. Thanks again for your support!