Migrate user passwords to hash-based encoding - #7188
Conversation
…upon next user login.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 75 |
| Duplication | 1 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
@thomaslow : Did I understand it correct that the LDAP entry is written the first time as the user logged in the first time and no LDAP entry exists until then? |
No. If no LDAP entry exists for a user, but an LDAP group is configured for the user, the login will fail with "bad credentials". An LDAP entry is only created if explicitly requested by clicking the button on the user edit page and then asking the user to log in with the existing database password. |
Maybe there is only a little misunderstanding. Even now the LDAP entry is not created until the administrator is clicking on the button to create the LDAP entry. After this the entry exists in the LDAP and can be used (f.e. using the login credentials on a WebDAV/Samba file access system). With your change the LDAP entry is delayed until the user did the first login, so until the user did not made his / her first login associated systems like WebDAV or Samba did not "work" for this users as no LDAP entry exists. Correct? I'm asking only for a right understanding of the change. |
Yes, with this PR there is a new delay introduced. A User needs to log in at least once after an admin has clicked the button before the LDAP entry is created and the user may use WebDav/Samba that is configured with this LDAP server. The problem is that when clicking the "write LDAP entry" button, with this PR, it will no longer be possible to retrieve the plaintext password of a user from the database. The following strategies would be possible:
|
|
@thomaslow Thank you for the clarification. I like your approach as this is much better then the current solution or many others except for the delay but this should be okay and can be communicated. |
This PR improves security by migrating user passwords from the old symmetric encryption strategy (
PBEWithMD5AndDES) to a modern hash-based password encoding strategy calledargon2id.Todos
Related Issues
Migration Notes
To keep everything as easy as possible in terms of migration, password encodings are replaced upon the next user login automatically via Spring's
DelegatingPasswordEncoder. This means the database will contain both old password encodings and new password encodings for some time. New password encodings are distinguished from old ones based on a prefix, e.g.{argon2}password. Old passwords do not have any prefix.Old password encodings are still considered valid for now. This should reduce any issues when, e.g., loading a database backup with old passwords. We should discuss when to remove support for old password encodings in the future. Because of that, the corresponding security warning "Use of a broken or risky cryptographic algorithm" will remain in effect.
LDAP Login Task
The change from a symmetric encryption strategy to a hash-based encoding strategy also means that plaintext passwords of users cannot be decrypted anymore. Currently, passwords are decrypted to register the plaintext password with LDAP when clicking the "Write LDAP configuration" button on the user edit page. This PR changes this behavior and introduces "login tasks". Instead of writing the plaintext password immediately upon clicking the button, a login task is created for that particular user. When this user logs in, the plaintext password is available again shortly during the login and used to write the LDAP entry, see
CustomLoginSuccessHandler. Any error messages are stored for the login task and shown on the user edit page. The process is as follows:User logs in
Successful execution is reported
When changing passwords via the "password change/reset dialog", the LDAP configuration is written immediately (as before).
@solth @henning-gerhardt Do you think this process is acceptable for administrators when synchronizing user passwords with LDAP? Do you see any issues? Please add anyone that you think might be interested/affected to this PR.
Future Ideas
The mechanism of "login tasks" may be extended in the future, e.g.: