Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
The module by default saving passwords for all users into 'password_policy_history' table, which is not required. I think, it should save passwords only for users have role added to password policy.
Could you please confirm, is that a bug or something you don't support ?
Thanks,
Balraj
Comments
Comment #1
BalrajB CreditAttribution: BalrajB commentedIf this is bug, need to add this patch (file attached)
Comment #2
BalrajB CreditAttribution: BalrajB commentedNeed to review above patch.
Comment #4
BalrajB CreditAttribution: BalrajB commentedThanks for quick reply. that was my fault, did not upload correct patch. Correct one is here, lets see if still fail.
Comment #5
BalrajB CreditAttribution: BalrajB commentedComment #6
erikwebb CreditAttribution: erikwebb commentedThe balance here is that if you need to add any new roles with history constraints, they won't be usable at first.
I'm not opposed to this change, but I'd want to make sure we are gaining something here.
Comment #7
BalrajB CreditAttribution: BalrajB commentedThe main advantage of above patch is:
on hight traffic site, the 'password_policy_history' table would become massive ( because this would add password history for all users) which is not good. and by above solution you can still save password history once you add a role to same or new policy.
Is that answer your question ?
Comment #8
erikwebb CreditAttribution: erikwebb commentedI guess my thought was really why you would use a password policy for only a minority of your users. That would be the use case for this patch. If a majority of users are already using a policy, then the overhead would be minimal in the event you wanted to add a policy to those unrestricted roles.
Comment #9
AohRveTPV CreditAttribution: AohRveTPV commentedIn my opinion it would be better to not store old password hashes at all by default. Password hashes are sensitive data and add security risk. Worse, administrators who do not use the history constraint may not expect or be aware that they are being stored. So I think it would also be good to store hashes only for users to whom a history constraint applies, and store them only once the constraint is enabled.
If administrators want to store old passwords in case they later decide to enable a history constraint, there could be a setting to enable storage of old password hashes for all users.
The benefit of storing old password hashes by default without administrator knowledge is limited anyway, because they are only stored since the module was enabled and not before. I am guessing the typical use case for Password Policy is to install it, immediately configure policies, and enable them. It is probably not so much to install Password Policy, wait for a password history to accumulate, then configure and enable password policies.
However, changing the default behavior of password history in 7.x-1.x/6.x-1.x could cause confusion and risks causing problems in a stable release. I think more would need to be changed than in the patch (e.g., README.txt and a configuration setting). So I currently would be in favor of implementing this change by default in 7.x-2.x and leaving the 7.x-1.x/6.x-1.x behavior as it is. Maybe it could be implemented in 7.x-1.x/6.x-1.x later as a non-default option.
Comment #10
AohRveTPV CreditAttribution: AohRveTPV commentedComment #11
lisotton CreditAttribution: lisotton at European Commission and European Union Institutions, Agencies and Bodies commentedI tried to implement something more generic, which will not store the history for users that don't match any of the configured policy.
We have a case where 99% of the users authenticate with CAS module and the CAS module generate a new password every time a user authenticate, so a new record is created in the "password_policy_history" for every authentication.
Comment #12
lisotton CreditAttribution: lisotton at European Commission and European Union Institutions, Agencies and Bodies commentedFixing patch.