Skip to content

[5.2] User: Don't reset newly set requireReset#44519

Merged
pe7er merged 1 commit intojoomla:5.2-devfrom
Hackwar:5.2-user-requirereset
Dec 2, 2024
Merged

[5.2] User: Don't reset newly set requireReset#44519
pe7er merged 1 commit intojoomla:5.2-devfrom
Hackwar:5.2-user-requirereset

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Nov 24, 2024

Pull Request for Issue #40480.

Summary of Changes

When an existing user is edited, the password is updated and the require password reset flag is set, the flag is directly unset again. When an admin in the backend for example resets an organisational account and resets the password to something like 123456 and wants the user to reset their password upon next login, this unintentionally clears that flag because the admin has just changed the password.

Testing Instructions

  1. Create a new user
  2. Edit that new user and change the password AND set the switch to require password reset
  3. Click on save

Actual result BEFORE applying this Pull Request

The page reloads, the password is set to the new value and the require reset flag is set to NO.

Expected result AFTER applying this Pull Request

The page reloads, the password is set to the new value and the require reset flag is still set to YES.

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

@Attila-SWE
Copy link
Contributor

Since require password reset is something we want the end-user to act upon I suggest removing the code that automatically removes require reset all together. If admin wants to change password and no longer wants user to change at next login then admin manually can deselect the switch for require reset?

@Hackwar
Copy link
Member Author

Hackwar commented Nov 25, 2024

That code is the code which resets the flag after the user changed their password. Removing that code would mean that a user has to change their password on every page load.

@ghost
Copy link

ghost commented Nov 25, 2024

I have tested this item ✅ successfully on dfbac92


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

@Attila-SWE
Copy link
Contributor

Attila-SWE commented Nov 25, 2024

Oh! I thought it was only for backend change. I checked file path now. Then of course it should not be removed. Is there a point in modifying backend functionality?

The way we use it is when users contact us and have issues with resetting/changing their password on their own. Then we change for them and mark as require reset at next login. This could also be the case when user already has require reset marked.

@Hackwar
Copy link
Member Author

Hackwar commented Nov 25, 2024

Right now it clears that flag when you change the password, so you have to change the password, open the user again and then set the flag and save it again. This PR fixes that.

@Attila-SWE
Copy link
Contributor

Attila-SWE commented Nov 25, 2024

I have now manually applied code change and can confirm it works as described in PR.

Would it be possible to change from

if ($this->requireReset) {

to something like

if ($this->requireReset && !BACKENDEDIT) {

Because then when, as admin, editing users in backend the flag would not be reset automatically. This would basically only be relevant when user already has reset required marked and admin changes temporary password again (for example on users request)

@Attila-SWE
Copy link
Contributor

I have tested this item ✅ successfully on dfbac92


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

@Quy
Copy link
Contributor

Quy commented Nov 25, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 25, 2024
@pe7er pe7er self-assigned this Nov 28, 2024
@pe7er pe7er merged commit 15bf3d1 into joomla:5.2-dev Dec 2, 2024
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 2, 2024
@pe7er
Copy link
Contributor

pe7er commented Dec 2, 2024

Thanks @Hackwar !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants