User with empty password can't reset their password

Description

This is perhaps a bit insane of a scenario which came about when testing in a dev environment, where I do things like create users with empty passwords which I would never do on a production system. Still, it potentially reveals some odd corner cases, so I wanted to capture it and let the developer decide what to do with it (with myself having a wide open mind to wontfix perhaps being the most appropriate handling).

To reproduce:

Create a new user with an empty password from the admin page.

Select Reset password for the user from the admin page.

Login as the user and verify that you are being forced to change the password.

Attempt to change the password, which will result in:

The user is not logged out and presented with an authentication dialog, which seems to be the typical response on a successful password change. Subsequent attempts to change the password have the same result, except the Thank you notice goes away after the first attempt.

This ties in to the need to have a minimum allowed password length. Is that something the AccountManagerPlugin should eventually support, or should the store be enforcing a minimum length?

If BasicCheck is not enabled though, the user probably shouldn't blocked from changing their password, even if the old password is empty.

I've just checked that it works from me (well I bet, you know, why I had to confirm that immediately) - it does. Glad, you already confirmed that too.

Yes, your suggestion looks right. Still watch out for the side-effect of later registration check configuration changes, as hinted on IRC. You may still shoot yourself in the foot after fixing the issue here, if you enable 'BasicCheck' just later on without fixing empty passwd accounts before.

I see two possible, related enhancements, that may require a dedicated ticket, if they remain valid after discussion and thinking it through a bit longer:

Make AccountManager jump at admins, if checks included in current configuration are disabled, including the default, implicit configuration.

Make AccountManager scan for insane accounts (empty password is just one option), possibly as an added countermeasure against undesired duplicate account entries in the same and/or another authentication store, or even recommend the check somehow while 'BasicCheck' is enabled.

Keeping 'no empty password' for user education, but only showing it on password
validation failure. But the story has only just started:

Additionally AccountModule did not check for password change success on
forced password change, unconditionally removing permission lock-down and
reporting password update even on failure. Thanks to Ryan J. Ollos for testing
and providing a detailed description of this behavior.

Closing yet another way to bypass a forced password change by just repeating
the old password as new password, what isn't accepted anymore now.

Admin users may continue to create password-less accounts. Still successful
authentication with acct_mgr.LoginModule required another change to prevent
unconditionally failed logins because of the empty password.

Otherwise admin users couldn't create new user accounts with this check
enabled for the new user registration procedure.
Background: There's no way to pass that check there, because the hidden field
isn't supplied to user admin panel, but to the registration form exclusively.

Ryan, as you see, this was far from becoming a wont-fix as is seemed initially.

I confess, that I underestimated this ticket too, or I had hold-off yesterdays acct_mgr-0.4.2 release a bit to include a least these fixes. Well, it was before looking closer at the code tonight. Now I'm exceptionally glad about you taking your time to report the initial issue here.

Changes look good to me, and following the procedure described in comment:description after updating past r12491 of AccountManager plugin trunk, I am no longer experiencing the issues reported here. I believe this ticket is can be resolved when 0.4.3 of AccountManager is released.

Changes look good to me, and ... I am no longer experiencing the issues reported here. I believe this ticket is can be resolved when 0.4.3 of AccountManager is released.

Thanks for this positive confirmation. I've been testing this too before, of course. As for the release you may notice from my silent planning in changelog that I don't intend to do another maintenance release, rather try to push all further changes towards acct_mgr-0.5, because there are quite a few new features and more invasive fixes, pretty much to justify a new "major" release.

This is another update for current stable acct_mgr-0.4 to immediatly push
the fix against trac.ini corruption and other recent corrections.

Note though that an unnecessary msgid change from [12490] has been excluded
and - as exception to the rule - there is a solution rated as 'feature' too.

Add Comment

This ticket has been modified since you started editing. You should review the
other modifications which have been appended above,
and any conflicts shown in the preview below.
You can nevertheless proceed and submit your changes if you wish so.