Force password change on first-time login requires password to change

This is strange because we do not add back in this field. It appears core has forgotten to remove the old password field. Do you have any other password-related modules installed?

According to the password reset link documentation, this should be handled via a session variable within Drupal core.

I confirm this bug.

When adding a new user with the checkbox to force password change, upon login you get to password change screen and there is the "Current Password" field. Weird thing is that the SESSION variable is set (checked on DB).

I'm debugging.

It seems the SESSION is not the only thing:

<?php
    // To skip the current password field, the user must have logged in via a
    // one-time link and have the token in the URL.
    $pass_reset = isset($_SESSION['pass_reset_' . $account->uid]) && isset($_GET['pass-reset-token']) && ($_GET['pass-reset-token'] == $_SESSION['pass_reset_' . $account->uid]);
?>

This is in line 1047 of user.module

Last test was to manually place the GET parameter back. The current_password field is gone. I'll make a patch here.

Thanks for calling out my ignorance. I look forward to your patch.

Tested and this patch worked.

I think we need to cover this in a test.

After taking a look at your patch, I have another thought. I'm thinking if this is a one-time login link, should we just not perform a redirection at all and let Drupal core take care of it? That seems like a safer way to handle this now.

I thought about this, but that will open the whole site, if the user navigates away, isn't it?

Fair enough, I guess Drupal doesn't stick them to the login form. Does this patch prevent the navigation away from the password page more than once? It doesn't appear so to me, so we might need to store the reset token in the session after all.

Well, I think core compares the value in $_GET and the one in SESSION for security reasons, otherwise someone who can steal the cookie from the one-time login can reset the password without having the original reset link. Navigating away from the change password screen on Drupal also cause this issue, but then user can again request a new password. With password_policy, that's not possible, so maybe this is what we need to adjust for this feature to be usable yet safe.

Summarizing:

1 - User login through link from e-mail.
2 - Is redirected to change password.
3 - Tries to navigate away.
4 - Is redirected to change password, but with query value gone, needs existing password.
5 - Bellow "existing password" there is the link to request the new password.
6 - User gets the new e-mail and go back to 1.

I'm worried about altering the security of how user.module usually handles this problem. I know this is an important bug, but I'm going to have to postpone it temporarily while I consult the Security Team.

someone who can steal the cookie from the one-time login

If someone can steal your cookie then you are already owned.

I think it makes sense for password policy to just try to follow the core behavior as close as possible and not do any additional features.

Note: This issue is blocking #1682928: Password reset with password change attempts

following

Not really sure if it is related

I applied the patch password_policy-update_password_change_tab_d7-1330502-23.patch first and then password_policy-incorrect_redirect_after_password_reset-1680146-1.patch to stable version 7.x-1.1

I get the error Notice : Undefined index: pass in password_policy_password_tab() (line 41 in ....mysite/sites/all/modules/password_policy/contrib/password_tab/password_policy_password_tab.pages.inc). When I get to the change password form after getting a link by email. The old password field does not appear.

When I validate the form it seems that there is still a need for the old password, which I am not supposed to have because the user requests to reset his password.

I am using Drupal core 7.15

I solved my issue (above at #18 ) by doing this in password_policy_password_tab.pages.inc (see pluses and minuses... sorry it's not a real patch).
It works fine now, hope it helps.

function password_policy_password_tab($form, &$form_state, $account) {
  global $user;

  // During initial form build, add the entity to the form state for use during
  // form building and processing. During a rebuild, use what is in the form
  // state.
  if (!isset($form_state['user'])) {
    $form_state['user'] = $account;
  }
  else {
    $account = $form_state['user'];
  }

  if ($user->uid == $account->uid) {
    // To skip the current password field, the user must have logged in via a
    // one-time link and have the token in the URL.
    $pass_reset = isset($_SESSION['pass_reset_' . $account->uid]) && isset($_GET['pass-reset-token']) && ($_GET['pass-reset-token'] == $_SESSION['pass_reset_' . $account->uid]);
   
    + if (!$pass_reset) {
    $protected_values = array();
    
    - $current_pass_description = '';
    // The user may only change their own password without their current
    // password if they logged in via a one-time login link.
    - if (!$pass_reset) {
      $protected_values['mail'] = t('E-mail address');
      $protected_values['pass'] = t('Password');
      $request_new = l(t('Request new password'), 'user/password', array('attributes' => array('title' => t('Request new password via e-mail.'))));
      $current_pass_description = t('Enter your current password to change the %mail or %pass. !request_new.', array('%mail' => $protected_values['mail'], '%pass' => $protected_values['pass'], '!request_new' => $request_new));
    -}
    // The user must enter their current password to change to a new one.
    $form['current_pass_required_values'] = array(
      '#type' => 'value',
      '#value' => array('pass' => $protected_values['pass']),
    );
    $form['account']['current_pass'] = array(
      '#type' => 'password',
      '#title' => t('Current password'),
      '#size' => 25,
      '#access' => !empty($protected_values),
      '#description' => $current_pass_description,
      '#weight' => -5,
      '#attributes' => array('autocomplete' => 'off'),
    );

    $form['#validate'][] = 'user_validate_current_pass';

    +}
  }

  $form['pass'] = array(
    '#type' => 'password_confirm',
    '#size' => 25,
    '#required' => TRUE,
    '#description' => t('To change the current user password, enter the new password in both fields.'),
  );

  // @TODO Remove this as it supports a D6-style of interacting with a user form
  // In the future, this data should be stored in $form_state.
  $form['#uid'] = $account->uid;
  $form['#user'] = $account;
  $form['_account'] = array('#type' => 'value', '#value' => $account);

  $form['actions'] = array('#type' => 'actions');
  $form['actions']['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Save'),
  );

  return $form;
}

I have a problem even if the user does NOT navigate away but simply just tries to change/edit/add any user information while they're inserting and confirming their address, e.g. phone number, address, etc.

As long as the user only inserts password and confirmed password, then saves, they are fine. Otherwise, old password filed pops up after trying to save new password and any other data.

This issue pretty much ruins this modules purpose!

#21, error. I didn't realize this was for a module, thought it was core. I'm not using module.

This doesn't appear to be a security issue. We'll resolve this by checking only against the session variable, rather than the URL (as core does both).

@mloigeret - Would you mind submitting a patch for your changes?

following and checking if my solution is the as as mloigerert's solution

I think you only need to do the following is my solution change the code around line 28 in the password_policy_password_tab.pages.inc, from this
$protected_values = array();
$current_pass_description = '';
// The user may only change their own password without their current
// password if they logged in via a one-time login link.
if (!$pass_reset) {
$protected_values['mail'] = t('E-mail address');
$protected_values['pass'] = t('Password');
$request_new = l(t('Request new password'), 'user/password', array('attributes' => array('title' => t('Request new password via e-mail.'))));
$current_pass_description = t('Enter your current password to change the %mail or %pass. !request_new.', array('%mail' => $protected_values['mail'], '%pass' => $protected_values['pass'], '!request_new' => $request_new));
} //remove this line
// The user must enter their current password to change to a new one.
$form['current_pass_required_values'] = array(
'#type' => 'value',
'#value' => array('pass' => $protected_values['pass']),
);
}

to

$protected_values = array();
$current_pass_description = '';
// The user may only change their own password without their current
// password if they logged in via a one-time login link.
if (!$pass_reset) {
$protected_values['mail'] = t('E-mail address');
$protected_values['pass'] = t('Password');
$request_new = l(t('Request new password'), 'user/password', array('attributes' => array('title' => t('Request new password via e-mail.'))));
$current_pass_description = t('Enter your current password to change the %mail or %pass. !request_new.', array('%mail' => $protected_values['mail'], '%pass' => $protected_values['pass'], '!request_new' => $request_new));

// The user must enter their current password to change to a new one.
$form['current_pass_required_values'] = array(
'#type' => 'value',
'#value' => array('pass' => $protected_values['pass']),
);
} //and move it to here.

the idea is to move the current password form element in to the password reset test ( if (!$pass_reset) ) so it doesn't get rendered in the form if the process is a password reset. but does display if $pass_reset is empty

Also both solution produce this error

Warning: Invalid argument supplied for foreach() in user_validate_current_pass() (line 1190 of /modules/user/user.module).

the error isn't critical but is untidy

FYI, The current password field is not required (for me) when...

- the nocurrent_pass module is intalled
- the password_policy tab module is NOT installed

The nocurrent_pass module seems to work for the EDIT tab, but not for the PASSWORD tab.

This may be common knowledge here, but I didn't see mention of it on any comments, so for what it's worth, am sharing this here.

Hello,

I believe that in order for a forgotten password to be reset there are two steps. I mention them here https://drupal.org/node/1802978#comment-6806236.

Note: - the nocurrent_pass module is intalled
- the password_policy tab module IS installed

I just bumped into this, and against my better judgement, have modified the contrib to make this work.

Because I don't really know how to contribute to the module, I'll just post here:

The module is forwarding the one-time-login link to the wrong place. To fix:
File: password_policy_password_tab.module:80
Change:$path .= '/password';
To: $path=str_replace("/edit","/password",$path);

Then, the change-password form is sending too many (or the wrong) form elements:
File: password_policy_password_tab.pages.inc:39
Change:

$form['current_pass_required_values'] = array(
      '#type' => 'value',
      '#value' => array('pass' => $protected_values['pass']),
    );
    $form['account']['current_pass'] = array(
      '#type' => 'password',
      '#title' => t('Current password'),
      '#size' => 25,
      '#access' => !empty($protected_values),
      '#description' => $current_pass_description,
      '#weight' => -5,
      '#attributes' => array('autocomplete' => 'off'),
    );

    $form['#validate'][] = 'user_validate_current_pass';

To:

if(isset($protected_values['pass'])){
      $form['current_pass_required_values'] = array(
        '#type' => 'value',
        '#value' => array('pass' => $protected_values['pass']),
      );
      $form['account']['current_pass'] = array(
        '#type' => 'password',
        '#title' => t('Current password'),
        '#size' => 25,
        '#access' => !empty($protected_values),
        '#description' => $current_pass_description,
        '#weight' => -5,
        '#attributes' => array('autocomplete' => 'off'),
      );
  
      $form['#validate'][] = 'user_validate_current_pass';
    }

These changes made it work for me. Hope this helps someone.

I think this issue has diverged into two separate discussions -

1. Storing the one-time login token in the SESSION in case the page is loaded more than once (or redirected to)
2. Fixing Password Tab to support the current password field

Let's move the Password Tab discussion over to #1802978: Password tab URL not correct when using password reset

Unfortunate that this issue has taken so long, but I think this patch should fix this problem. Please test.

Adjusting tests.

Works for me, thanks!

EDIT: Seemed to work in Dev but not our Prod, looking into it.

Thanks Jim, please update when you confirm.

Not sure why this was still assigned to me...

#37 Works fine for me.

http://drupalcode.org/project/password_policy.git/commit/73a251f

Does anyone know if this is an issue for 6.x too?

#37: password_policy-force_change_requires_password-1596960-37.patch queued for re-testing.

Possible solution http://drupal.org/project/password_hustle

@Road Kill - This problem is actually solved in the 7.x-1.x branch (no release yet). It's a shame that module couldn't provide that functionality here to consolidate.

No worries erikwebb just thought I would share, but you right it is a shame could have saved me and others a lot of time.

A quick fix in d7.. in the password_policy_init function replace the following lines

        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()));

with the following

$destination = drupal_get_destination();
      if (strpos($destination['destination'], 'pass-reset-token') === FALSE) {
        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()));
      }

I am actually getting this problem on Drupal.org. I created an account and it is forcing me to enter the Current Password (which doesnt exist) to change the password. Seems like a core issue considering its on drupal.org itself.

Unable to reproduce in D6:
1. Enabled password_policy.
2. Checked "Force password change on first-time login" at admin/settings/password_policy/password_change.
3. Created new user "foo" using admin/user/user/create.
4. Logged out.
5. Used one-time login link to log in as user "foo".
6. Was prompted to change password as expected, without field to provide a current password.
7. Attempted to navigate away from page, but was returned to same page, again prompting to change password, without field to provide a current password.

Re #50: Indeed, if you register an account on Drupal.org, follow the one-time login link, browse away from the initial password-change page, then return to it, you are prompted for a nonexistent current password. However, you can recover from the situation by requesting a new password (via e-mail).

So it is unclear to me why this was a change for Password Policy to make. I guess the intent was to do it better than core?

The issue does not seem to occur in D6 so there does not need to be a D6 patch.