Skip to content

Additional password checks #1067

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 11 commits into
base: master
Choose a base branch
from

Conversation

qrainaud
Copy link

New password check option : check if new password contains either name or surname

  • Can be enabled/disabled with $pwd_diff_namesurname in config
  • Backend verification happens in htdocs/change.php after submit
  • /rest/v1/getnamesurnamefromlogin.php api endpoint for future frontend feedback
  • Frontend feedback will require ltb-project/ltb-common to be updated, as well as rest api to be enabled

qrainaud and others added 5 commits April 25, 2025 09:21
… function name was changed to detect_language
…r surname are inside the new password.

- It can be enabled/disabled in config ($pwd_diff_namesurname)
- It requires restapi to be enabled ($use_restapi)
- It uses the api endpoint /rest/v1/checknamesurname.php
- Both english and french languages are currently supported
…rted) for future backend verification (Ltb-common)
…e linked to a given login, to be used by the ltb-common library
@qrainaud
Copy link
Author

For frontend feedback : ltb-project/ltb-common#71

@coudot coudot added this to the Backlog milestone Apr 29, 2025
@coudot
Copy link
Member

coudot commented Apr 29, 2025

Hello,

why don't use $pwd_forbidden_ldap_fields ?
https://self-service-password.readthedocs.io/en/stable/config_ppolicy.html#forbidden-ldap-fields

@qrainaud
Copy link
Author

Hello,

I didn't notice this. The backend part of my fork is basically useless, but I believe the api as well as the frontend would still be useful as a lot of users tend to put their name or surname in their password, and the feedback message after submission isn't really clear.
And since the message is only shown after submission, they have to rewrite everything.

@qrainaud
Copy link
Author

qrainaud commented May 7, 2025

Hello, let me know what you think.

@coudot
Copy link
Member

coudot commented May 7, 2025

This issue is in backlog for now, it is too specific for the moment (checking only name and surname).

Using the current generic parameter $pwd_forbidden_ldap_fields is the correct way to do this. Maybe the improvement is only to dynamically check this parameter when user is typing its password.

@qrainaud
Copy link
Author

qrainaud commented May 7, 2025

I can change it so that it checks every forbidden ldap field dynamically when the user is typing its password, and display a feedback similar to "Your password may not contain personal informations". What do you think?

@coudot
Copy link
Member

coudot commented May 7, 2025

You can give a try.

I don't swear it will be included as it can be a security flaw: this would allow to discover informations from LDAP directory.

@qrainaud
Copy link
Author

qrainaud commented May 7, 2025

For the security, I will change the api endpoint so that it validates passwords instead of returning LDAP data.
On the api side : to avoid spamming the LDAP directory, I can make a cache system so that it only fetches informations from LDAP when the login changes.

qrainaud added 4 commits May 7, 2025 14:58
- takes a login and a password as x-www-form-urlencoded format parameters
- returns an "isValid" boolean
- uses the project root cache folder to store recent LDAP queries using symfony cache system

The "isValid" boolean returned is :
- true if the login doesn't exist in the LDAP
- true if the login exists and the password doesn't contain any forbidden ldap entry
- false if the login exists and the password contains a forbidden ldap entry
@qrainaud qrainaud force-pushed the additional-password-checks branch from c5dfb62 to 4ccf748 Compare May 7, 2025 13:25
qrainaud added 2 commits May 7, 2025 15:27
update "pwd_forbidden_ldap_fields" from pwd_policy_config array in index.php for future template display
@qrainaud
Copy link
Author

qrainaud commented May 7, 2025

Hello again, let me know what you think.

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.

2 participants