Skip to content

[6.0] Trim email before password reset request #42893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 6.0-dev
Choose a base branch
from

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Feb 26, 2024

Pull Request for Issue #42374.

Summary of Changes

When typing in the email adress and accidentally adding a space at the front, the process fails. See the original issue linked above.

I read the original issue and I'm not sure if this is a bug or a feature or if we even want to interfer this way with this, but I decided to create a PR anyway, since it is easier to discuss with code directly instead of hypotheticals.

Testing Instructions

Type in your email adress with a space at the beginning.

Actual result BEFORE applying this Pull Request

Password reset fails.

Expected result AFTER applying this Pull Request

Password reset works.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomdonation
Copy link
Contributor

I remember I looked at this issue at PBF but could not replicate it for some reasons. If we want to trim space like this, I think we should add filter="TRIM" to the field, before validate="email" https://github.com/joomla/joomla-cms/blob/4.4-dev/components/com_users/forms/reset_request.xml#L10 instead

@brianteeman
Copy link
Contributor

Personaly this is expected and intended behaviour and should not be accepted

@HLeithner
Copy link
Member

would move it to 5.2, btw. if you already trim the email you could lowercase it too in php but I think it doesn't has an performance impact in sql

@MacJoom MacJoom self-assigned this May 17, 2024
@Hackwar Hackwar changed the base branch from 4.4-dev to 5.2-dev August 15, 2024 13:17
@Hackwar Hackwar changed the title [4.4] Trim email before password reset request [5.2] Trim email before password reset request Aug 15, 2024
@Quy Quy removed the PR-4.4-dev label Aug 15, 2024
@coolcat-creations
Copy link
Contributor

that a great improvement for User Experience, thank you. I tested it but it does not work

grafik

@crommie
Copy link

crommie commented Aug 24, 2024

I have tested this item 🔴 unsuccessfully on 95be583

Firefox doesn't see it as an email address when I type in a space first.
Google Chrome does, and before AND after applying the patch, I get a message that the email is sent.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@muhme
Copy link
Contributor

muhme commented Aug 24, 2024

I have tested this item ✅ successfully on 95be583

Tested successfully with 5.2-dev

  • The user used could not have the superuser role, as no password reset mails are sent to them
  • It was already working before the patch as ¿all? modern browsers remove leading spaces for email fields (verified with Chrome developer tools inspecting network request)
  • Used one and multiple spaces before email address
  • Tested with 'Forgot your password?' and 'Forgot your username?'
  • Using emails with spaces before is still working after applying the PR
  • The interesting question is the used browser in the issue?
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@muhme
Copy link
Contributor

muhme commented Aug 24, 2024

@crommie I can confirm that Firefox is not accepting email address with leading space, but there is a clear error message Please enter an email address. In my view, this is valid browser behaviour and cannot and need not be influenced by this PR.

@crommie
Copy link

crommie commented Aug 24, 2024

Email was sent before and after applying patch - in theory, because I tested on a PBF server site that didn't send emails at all. So maybe the unsuccessful test should be removed.

@ghost
Copy link

ghost commented Aug 24, 2024

So maybe the unsuccessful test should be removed

mark "not tested" at issue trackerand "submit test result".

@crommie
Copy link

crommie commented Aug 24, 2024

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@crommie
Copy link

crommie commented Aug 24, 2024

Yayyy, that worked, thanks.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@HLeithner HLeithner changed the base branch from 5.2-dev to 5.3-dev September 2, 2024 08:52
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.3-dev.

@HLeithner HLeithner changed the title [5.2] Trim email before password reset request [5.3] Trim email before password reset request Sep 2, 2024
@Hackwar Hackwar removed the PR-5.2-dev label Sep 3, 2024
@MacJoom MacJoom added the PBF Pizza, Bugs and Fun label Feb 18, 2025
@Pritams43
Copy link

I have tested this item ✅ successfully on 95be583

I have tested this item successfully and it is working as expected


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@crimle
Copy link

crimle commented Feb 22, 2025

I have tested this item 🔴 unsuccessfully on 95be583

Using Firefox, password reset fails in any case, with or without patch. Firefox displays a little message «Please enter an email address» in either case. Maybe this issue is related to a certain brwoser?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@MacJoom
Copy link
Contributor

MacJoom commented Feb 22, 2025

With Chrome the problem cannot be reproduced, email is sent - no issue. Patch not needed or already fixed?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@MacJoom
Copy link
Contributor

MacJoom commented Feb 22, 2025

I have tested this item 🔴 unsuccessfully on 95be583


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@ghost
Copy link

ghost commented Feb 22, 2025

@MacJoom Can you please provide the reasons for the test result?

@webfeuerflo
Copy link

Without the patch the issue occurs in firefox, in chrome the password reset email is being sent, in firefox I see the message mentioned above. applying the patch doesn't change anything

@MacJoom
Copy link
Contributor

MacJoom commented Feb 22, 2025

@MacJoom Can you please provide the reasons for the test result?

See above

@ghost
Copy link

ghost commented Feb 22, 2025

@MacJoom Thanks for the answer. I wasn't sure because of "Patch not needed or already fixed?".

@ceus1984
Copy link

I have tested this item 🔴 unsuccessfully on 95be583

Errormessage: The email address you entered is invalid.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@cybersalt
Copy link

I tested this in Chrome - entering an email with a space at the beginning worked with and without the patch. As for if this is needed or not, I am all for it since I cannot think of any circumstance where a person would intentionally put a space at the beginning of entering an email, so why not just save the frustration of it failing. Also, why not set things to remove a blank space at the end of an email as well, since sometimes double-clicking to select an email for cut and paste might accidentally grab a space that may be after it. Of course it is not needed if a space at the end doesn't break anything.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@cybersalt
Copy link

I have tested this item 🔴 unsuccessfully on 95be583

Sorry for the double post. I missed using the "Submit Test Result" feature - #noob!

I tested this in Chrome - entering an email with a space at the beginning worked with and without the patch. As for if this is needed or not, I am all for it since I cannot think of any circumstance where a person would intentionally put a space at the beginning of entering an email, so why not just save the frustration of it failing. Also, why not set things to remove a blank space at the end of an email as well, since sometimes double-clicking to select an email for cut and paste might accidentally grab a space that may be after it. Of course it is not needed if a space at the end doesn't break anything.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

@HLeithner HLeithner changed the base branch from 5.3-dev to 6.0-dev March 4, 2025 17:21
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 6.0-dev.

@HLeithner HLeithner changed the title [5.3] Trim email before password reset request [6.0] Trim email before password reset request Mar 4, 2025
@rdeutz rdeutz removed the PR-5.3-dev label Mar 5, 2025
@rbuelund
Copy link

I have tested this item 🔴 unsuccessfully on ae0c40c

In Chrome browser the "Forgot your password?" works both with space before e-mail and space after - the request is send.
In Firefox browser the request is blocked by the browser if adding a space in fornt or after the e-mail.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42893.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature PBF Pizza, Bugs and Fun PR-6.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.