Ajax call breaks Password Reset

This fix seems to work. Very odd that nobody found this problem before as it seems like a common use case.

If you don't want to patch your drupal core then you can do this in a form alter:

/**
 * Implements hook_form_alter() for user_profile_form.
 */
function custommodule_form_user_profile_form_alter(&$form, $form_state, $form_id) {
  global $user;
  $account = $form['#user'];
  // Fix the ajax call (http://drupal.org/node/1858486).
  // Code below is from user.module user_account_form().
  //
  // When Form is rebuild using AJAX, GET variables won't be set
  // so check if form had been built using pass reset token
  $pass_reset_token = isset($_GET['pass-reset-token']) ? $_GET['pass-reset-token'] : (isset($form_state['input']['pass_reset_token']) ? $form_state['input']['pass_reset_token'] : FALSE);
  // To skip the current password field, the user must have logged in via a
  // one-time link and have the token in the URL (or stored by form).
  $pass_reset = isset($_SESSION['pass_reset_' . $account->uid]) && $pass_reset_token && ($pass_reset_token == $_SESSION['pass_reset_' . $account->uid]);

  $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'] = $form['account']['mail']['#title'];
    $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));
  }
  else {
    // Store pass reset token in form as hidden value
    $form['pass_reset_token'] = array(
      '#type' => 'hidden',
      '#value' => $pass_reset_token,
    );
  }

  // The user must enter their current password to change to a new one.
  if ($user->uid == $account->uid) {
    $form['account']['current_pass_required_values']['#value'] = $protected_values;
    $form['account']['current_pass']['#access'] = !empty($protected_values);
    $form['account']['current_pass']['#description'] = $current_pass_description;
  }
}

This is reproducible in Drupal 8 also.

Maybe it would be better to store the token in $form_state directly the first time it's found, rather than passing it back and forth to the browser?

Something like the attached, maybe... Seems to work based on some quick testing.

It is surprising that this bug with Drupal 7 core has not been reported more. I suppose not many people use AJAX on the user profile / registration forms, but it is completely site breaking for one of my sites. Nobody can register at all.

I fixed this simply by using the nocurrent_pass module but really I think this should be fixed in 7 as well as 8.

Yeah, I'm a bit surprised too.

Once the above patch is reviewed and committed to Drupal 8, it can be backported to Drupal 7 and fixed there also.

Hi there, I am experiencing this issue with my registration form since I am using an extra image field with upload Ajax button. Are any of this two patches working with drupal core 7.20? tried #1 but failed when submitting form SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry
thanks

Issues are fixed in d8 first

and what about D7 commit?

It gets committed once it is fixed in D8.

Here's a re-rolled patch for D7 that fixes some code and comment style.

• Updated the assignment to $form_state to use the FormStateInterface.
• Replaced the $pass_reset variable with the latest version in HEAD

Would it be possible to add a test here?

The fix itself looks solid to me, although I haven't confirmed the fix manually.

I've updated the issue summary to include a beta phase evaluation.

I added a test and updated the code slightly.

Thanks for the test! I think this is good to go.

Committed 29f441d and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

Ported the patch to D7.

I ran into this same issue and thankfully found this patch. While this patch got rid of the ugly error for me, it didn't completely resolve my issue. On my user profile form, I have a multi value text field that accepts unlimited values. This initially presents itself as one text field with an "Add Another Item" button. Now, with this patch in place, when I click the "Add Another Item" no second text field appears -- I get a 500 Internal Server Error in my console log.

If additional information is needed from my end, please request :)

Cheers, Leah

Thank's for backport to D7.
Didn't test it with multi value text field but worked fine for "Address Field".

+1

I'm excited to see this actually get released.

I reviewed the patch in #25.
It basically does one simple thing.
It stores the check

isset($_SESSION['pass_reset_' . $account->uid]) && isset($_GET['pass-reset-token']) && ($_GET['pass-reset-token'] == $_SESSION['pass_reset_' . $account->uid]);

in $form_state['user_pass_reset'] instead of using $pass_reset variable and by doing that it prevents the fail after form submission which is caused by any AJAX related form submit like user photo upload etc.

RTBC and would be nice to get it in the next D7 release too.

At first sight this looks good -- the D7 and D8 code are identical and the tests look OK as well.

The tests look good to me also. The original patch was mine, so ideally someone else would commit it.

However, has anyone looked into #27? We need to make sure there isn't a regression there.

One other small thing (could be fixed on commit):

+    // Make sure the ajax request from uploading a file does not invalidate the
+    // reset token.

Should be "Ajax".

Thanks for all the work on this, but given #27 I think this needs more review (and time).

Also while I understand that it can be a nuisance and is for sure a bug, it is rightly only normal priority as it is not that likely that a user first uploads a new file before proceeding to reset their password.

Obviously for password confirmation, this is way more likely.

Therefore setting back to needs review and not getting this in for 7.51 (but hopefully for 7.52).

--

Actionable item:

Try to re-create #27 on vanilla Drupal.

We run into this issue therefore having address field (Company select) on profile forms.

I've tried to reproduce #27 based on the infos we had, but everything worked fine.

We had this problem with D7 as we attached a form (field_attach_form) with an address field to the user form. The patch #25 solved the problem and we noticed no other problems with this patch.

D7 solution is identical, ready.

Fabianx can this get into this month's release?

Seems like with the additional testing that happened above, this is probably ready to go in.

#27 really doesn't sound related, and actually reading that again I'm not sure if it's even implying that this patch caused the problem; maybe it was a preexisting problem on that site that this patch just didn't happen to solve?

I'd say this is ready to go, all reports after #27 are saying this is good.

and the fact that it's already in D8 verbatim.

here's the patch again from 25, just for running the tests.

Looks good.

Committed and pushed to 7.x, thanks!