With only the 'administrator' role, if you select it and force password change, administrators will be endlessly forced to reset their passwords.. the value in password_policy_force_change.force_change never changes from 1 back to 0, no matter how many times the administrator changes his password.

Test with a non-uid=1 administrator, because by default uid=1 is never forced to change his password.

Found this issue in 7.x-1.0-rc1 first and confirmed still present in 7.x-1.x-dev

Comments

erikwebb’s picture

Status: Active » Needs review
StatusFileSize
new2.51 KB
erikwebb’s picture

erikwebb’s picture

fragglerock’s picture

Status: Needs review » Needs work

This is not working 100%

At time user first logs in after password has been reset they are been prompted it change the password. directly after that the user is not been re-prompted to change it. but if the user logs out they are not able to log back in with the new password

erikwebb’s picture

Status: Needs work » Postponed (maintainer needs more info)

None of the Password Policy code checks for an administrative role when changing this value. Do you see the same behavior for non-admins?

matt v.’s picture

Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

The patch in comment #2 above fixed the issue for me. I wasn't able to recreate the problem fragglerock described in comment #4.

I tried to write a test for the issue, but I think it's going to be difficult to pinpoint just the right chain of events to recreate the problem in a simpletest. Based on the initial problem description, I was hoping that checking the "Force password change on next login" box would trigger the problem, but it didn't seem to.

I experienced the problem on a live site and was able to test the patch on a dev version of same site. The problem cropped up there due to password expiration rules.

If someone is able to pinpoint the steps to recreate the problem, I'd be glad to write a test for it.

erikwebb’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.47 KB

The previous patch no longer applies. Please test again with this re-rolled patch.

Status: Needs review » Needs work

The last submitted patch, password_policy-force_password_change-1452280-7.patch, failed testing.

matt v.’s picture

Status: Needs work » Needs review
StatusFileSize
new3.77 KB

Oops, I wasn't working with the latest dev release, when doing the testing mentioned in Comment #6. Sorry about that.

As seen in the test results in Comment #7, the newest patch causes one test failure. I'm attaching a new patch that addresses the failing test.

matt v.’s picture

Status: Needs review » Needs work

I encountered another issue when I did some more testing. After I unblock an account with the administrator role that has an expired password, when the user next tries to login, the following error is triggered:

Recoverable fatal error: Argument 2 passed to db_query() must be an array, integer given, called in /var/www/example.dev/sites/all/modules/contrib/password_policy/password_policy.module on line 1036 and defined in db_query() (line 2310 of /var/www/example.dev/includes/database/database.inc).

matt v.’s picture

Status: Needs work » Needs review
StatusFileSize
new3.79 KB

Here's an updated patch that addresses the issue mentioned in Comment #10. Actually, I accidentally included a half-backed fix in the patch included in comment #9. In an effort to show the progression of what I was running into, I inadvertently made the patch and comments harder to follow.

In any case, after addressing the error mentioned in Comment #10, I did some more testing. Now, when I try to unblock an administrator account (I haven't tested with a non-admin) that has been blocked due to an expired password, the account gets re-blocked the next time the user tries to log in.

This new issue appears to be caused by the "unblocked" field not getting set by the query towards the bottom of password_policy_user_update. It appears that by the time this line has run:

if (isset($edit['status']) && $edit['status'] != $account->status && $edit['status'] == 1) {

... $account->status has already been set to 1, so the condition doesn't evaluate to true.

matt v.’s picture

I did a bunch more digging trying to pinpoint the problem. One thing I didn't realize until today is that, at times, a user being blocked in terms of the Password Policy module doesn't necessarily coincide with the same user's Status (blocked vs. active) in core, despite the fact that they both use the term "blocked." It seems the two settings are linked at times, but not always. Also, using the "Unblock the selected users" option on the admin/people page or setting a user's Status to Active on the user//edit page does not necessarily unblock the user in terms of the Password Policy module. I didn't initially realize that the Password Policy block is intended to be cleared via the admin/people/expired page. I think this could be delineated more clearly in the documentation, README.txt, or help text.

Another thing I've noticed as I've been testing is that every password change results in two rows being added to the password_policy_history table with the same created timestamp but two different hashed passwords. I haven't yet tracked down why that's happening.

I think I'm close to having a simpletest that recreates the initial issue. I hope to have it ready to post soon.

erikwebb’s picture

Re: the block/unblock process - it is very confusing in the current implementation. We should add some help text to clarify this. To help advance the implementation, please feel free to post some ideas for the 7.x-2.x branch.

Re: the issue of multiple rows being added to {password_policy_history - that should be fixed by #1653242: Password update doesn't store new password history

mgifford’s picture

Status: Needs review » Needs work

The last submitted patch, password_policy-force_password_change-1452280-11.patch, failed testing.

erikwebb’s picture

Status: Needs work » Needs review
StatusFileSize
new3.07 KB

Sorry for the delays here. Just catching up with some old issues.

I've re-rolled the patch in #11. I haven't re-added the change to password_policy_form_alter() though, because it doesn't look accurate. You are setting the default value of the force_change field based purely on whether you are changing your own account or another user's. This seems misleading.

deekayen’s picture

Issue summary: View changes
Status: Needs review » Needs work

doesn't apply anymore

aohrvetpv’s picture

Looks like deekayen later got it to apply:
http://drupalcode.org/project/password_policy.git/commit/672e8e9

Unfortunately the commit breaks a test. Specifically, the following assertion fails:

$this->assertNoRaw(t('Your password has expired. You must change your password to proceed on the site.'), t('Admin not initially prompted to change password.'));

Maybe just a matter of changing the edit path to edit/account in the test? Will plan to look into later.

aohrvetpv’s picture

The commit with a message referencing this issue, which I linked in #18, looks to actually be unrelated to this issue. Most of the changes look like style fixes, other than the change within PasswordPolicyForcePasswordChangeTestCase which breaks an assertion. I think it may just have an errant commit message. Please correct me if wrong.

Will revert the commit entirely and then recommit any style changes that make sense.

This issue is still unresolved and the patch in #16 remains unapplied.

aohrvetpv’s picture

Status: Needs work » Needs review
StatusFileSize
new7.63 KB

wrong issue

aohrvetpv’s picture

Status: Needs review » Needs work

The last submitted patch, 20: password_policy-ui-tests-2145649-20.patch, failed testing.

aohrvetpv’s picture

Status: Needs work » Postponed (maintainer needs more info)

Unable to reproduce this bug:
1. Installed/enabled Password Policy.
2. Created role 'administrator'.
3. Selected 'administrator' in 'Administrator role' select list at admin/config/people/accounts.
4. Created user 'admin2' and assigned to role 'administrator'.
5. Checked "Force password change at next login" for admin2.
6. Logged out.
7. Logged in as admin2.
8. Observed password expiration message and was forced to change password. After changing password, taken to front page, with no further prompt to change password.

So I believe the bug has been fixed. Can someone who experienced it confirm it no longer occurs? If it does occur, could you please give specific steps to reproduce it?

aohrvetpv’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

A month has passed with no information on how to reproduce this. Please re-open if you still have this issue.