cmandib opened a new pull request, #6670: URL: https://github.com/apache/fineract/pull/6670
## Description Fixes [FINERACT-2956](https://issues.apache.org/jira/browse/FINERACT-2956). The root cause and repro are in the ticket. Before this change, the password reuse check compared password hashes with `equals()`. That never works with bcrypt, so reused passwords were always accepted. Now the check uses `PasswordEncoder.matches()`, which is the correct way to compare a password with a bcrypt hash. I added a `matches()` method to `PlatformPasswordEncoder` so the service can call it. Requests that send `passwordEncoded` (a password the client already hashed) never send the plain text, so `matches()` can't be used for them and they still use the old exact string comparison. This also rejects the user's current password when the reuse check is on, since the current password is not in the history table and would otherwise still be accepted. I'm happy to drop this if reviewers prefer to keep the change limited to the history check. The old unit test did not catch this bug because it used a mocked encoder. The updated tests use the real one. I added unit tests to `AppUserWritePlatformServiceJpaRepositoryImplTest` and also added 3 integration tests to `UserAdministrationTest`. They check that a reused password is rejected when changing a password, when changing to the current password, and when updating a user. In each case the API returns a 400 error with `error.msg.password.already.used`. All unit tests pass, and the integration tests pass against a local Docker build of this branch. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions). - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes - [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [x] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
