Skip to content
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

Fixed bug in login procedure #1139

Open
wants to merge 2 commits into
base: 18.x
Choose a base branch
from

Conversation

STProgrammer
Copy link

@STProgrammer STProgrammer commented Oct 11, 2023

Fix the bug in the login procedure:
Every time users log in, the password changes automatically, causing the user to not be able to log in 2nd time.

In the original code, this is what happens:
When user logs in, the updatePwdHashIfNeeded function in LoginFormsProcess is called.
That function then calls pwdNeedsRehash function to check if the password needs rehash, and if so, receives
new password hash.
That password hash then is sent to changePassword function as an argument in the UserCoreModel.

The changePassword function then takes the password hash as an argument instead of the password itself and runs this code:
$rStmt->bindValue(':newPassword', Security::hashPwd($sNewPassword), PDO::PARAM_STR);, which decrypts the already password hash instead of decrypting the password itself.

There is nothing wrong with changePassword. But the changePassword shouldn't get password hash, but the password itself is an argument. Which in current code wasn't the case, which caused the password to be changed without any intention. So, I made some changes to fix this bug.

To fix this, I changed the following:

  • pwdNeedsRehash function on Security.class.php changed: instead of returning the password hash, it just returns TRUE or FALSE.
  • updatePwdHashIfNeeded function on LoginFormProcess changed: instead of sending the password hash as arugment, it sends the password itself as argument to the changePassword function. I changed the "if" statement as there is no need to save TRUE or FALSE value returned from pwdNeedsRehash function on Security.class.php.

I controlled where the pwdNeedsRehash function is used, and my changes don't break any other parts of the project.

…ash function on Security.class.php and updatePwdHashIfNeeded function in LoginFormProcessing files of Affiliates, users and admin.
@STProgrammer STProgrammer changed the title Bug fix login Fixed bug in login procedure Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant