Problem/Motivation
When you have the force password change options set Password Policy forces the user to be directed to a page on which they can edit their password and sets the destination to the page the user was attempting to view.

Currently password policy has some checks to stop this redirect happening if the page the user is already attempting to view is a password edit page however these checks do not take into account that, ordinarily, both 'user/uid/edit' and 'user/uid/edit/account' both point to the same page. Therefore, if you try to visit 'user/uid/edit' when your password expires you will be redirected to 'user/uid/edit/account?destination=user/uid/edit'. This means that any redirection set on the password edit form will be ignored and the user will be forced to view the account edit page twice.

Below is a snippet of the offending code.

  // Check password reset status and force a reset if needed.
  $change_password_url = 'user/' . $account->uid . '/' . (module_exists('password_policy_password_tab') ? 'password' : 'edit/account');
  if ($account->force_password_change && $_GET['q'] != $change_password_url) {
    // let users log out
    if (current_path() != 'user/logout' && current_path() != 'system/ajax') {
      drupal_set_message(t('Your password has expired. You must change your password to proceed on the site.'), 'error', FALSE);
      drupal_goto($change_password_url, array('query' => drupal_get_destination()));
    }
  }

Solution
Edit the comparison of $_GET['q'] to check against an array of possible paths.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rlmumford’s picture

Status: Needs work » Needs review
FileSize
1.12 KB

Here is a patch with the bear minimum for the fix.

Status: Needs review » Needs work

The last submitted patch, 1: 2170225-1.patch, failed testing.

rlmumford’s picture

FileSize
1.52 KB
1.53 KB

Here is a second patch. That cleans up some more of the existing code.

rlmumford’s picture

Status: Needs work » Needs review

The last submitted patch, 3: 2170225-2.patch, failed testing.

rlmumford’s picture

FileSize
1.65 KB
653 bytes

Heres a patch that actually works.

The last submitted patch, 6: 2170225-3.patch, failed testing.

The last submitted patch, 6: 2170225-3.patch, failed testing.

rlmumford’s picture

6: 2170225-3.patch queued for re-testing.

The last submitted patch, 6: 2170225-3.patch, failed testing.

rlmumford’s picture

FileSize
1.65 KB
731 bytes

Fixes a wierdness with array merging.

erikwebb’s picture

Status: Needs review » Closed (duplicate)
rlmumford’s picture

Status: Closed (duplicate) » Needs review

That issue is marked as a new feature, and being built for 7.x-2.x branch. This is a bug i.e broken behaviour in the current 7.x-1.x. This patch is not a new feature.

AohRveTPV’s picture

Seems reasonable to me to go ahead and fix the bug. The configurable exclude pages feature could be ported to 7.x-1.x later.

rlmumford, could you please give steps to reproduce the problem so we can test?

Updated patch:
- Changed "url" in variable names to "path". These are paths, not URLs: https://en.wikipedia.org/wiki/URI_scheme
- Added functions to make code more readable. (Abstracts away implementation details in long previous hook_init() implementation.)

  • Commit 0ba6dab on 7.x-1.x by AohRveTPV:
    Issue #2170225 by rlmumford, AohRveTPV: Password Policy force reset...
AohRveTPV’s picture

Status: Needs review » Patch (to be ported)

Refined patch in #14 to remove redundant code and committed. Now needs porting to 6.x and possibly 7.x-2.x.

  • Commit 0a07459 on 6.x-1.x by AohRveTPV:
    Issue #2170225 by rlmumford, AohRveTPV: Password Policy force reset...
AohRveTPV’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

Backported fix to 6.x-1.x. Rewrote hook_init() implementation there to make code more consistent with 7.x-1.x.

Now needs porting to 7.x-2.x.

AohRveTPV’s picture

Priority: Major » Normal
Status: Patch (to be ported) » Postponed

user/$uid/edit/account could be added to 7.x-2.x in password_policy_password_policy_expire_url_exclude() to address this, but doing so now would probably just add confusion to the code. In my opinion, the expire plugin in 7.x-2.x generally needs rework first.

  • AohRveTPV committed 0ba6dab on 8.x-1.x
    Issue #2170225 by rlmumford, AohRveTPV: Password Policy force reset...
AohRveTPV’s picture

rlmumford, sorry I didn't give proper credit for this commit. At the time I did not realize you had to use the --author parameter.