Support for Password Reset Landing Page (PRLP)

This will be a good feature to add. The patch is straightforward but I think it is problematic. It is module-specific integration code which--as with all module-specific integration code--increases the complexity of the module and will make it harder to ensure it is fully working.

If someone reads the code for the first time, they will wonder "Why are we modifying the password field on the user_pass_reset form when there is no password field on that form?" We could add a comment to explain that it is for PRLP, or they could figure it out through the Git log, etc., but this is still adding a special case which increases the complexity of the code.

Over time, how will we know this code still works for the user_pass_reset form, unless we have an automated test that tests the integration with the PRLP module?

I think a better approach might be a general one that checks all forms for the presence of the user password fields and always customizes them, instead of hard-coding all the specific form IDs where they might possibly appear. I am not sure if it can be done robustly or performantly.

Much like kyuubi's hook_form_alter, here's how we integrated the two on one project:

/**
 * Implements hook_form_FORM_ID_alter().
 *
 * Alters the password reset form to provide prlp functionality.
 *
 * The password policy module doesn't integrate with prlp module so we're
 * integrating it here.
 *
 * @param array $form
 *   The form to modify.
 * @param array $form_state
 *   The form state array.
 */
function mycustommodule_form_user_pass_reset_alter(&$form, $form_state) {
  // Avoid fatal errors is password policy module gets uninstalled.
  if (module_exists('password_policy')) {
    // Modify the elements.
    drupal_add_library('system', 'drupal.form');
    drupal_add_js(drupal_get_path('module', 'password_policy') . '/password_policy.js', array(
      'weight' => 20,
    ));
    password_policy_password_element_alter($form['account']['pass'], $form['#user']);
    // Add the new validate and submit handlers.
    $form['#validate'][] = 'password_policy_user_profile_form_validate';
    $form['#submit'][] = 'password_policy_password_submit';
  }
}

Putting the code here in case it helps anyone coming across this issue.

One solution that could be beneficial would be a simple UI where the user can add himself additional form keys to be checked.

This is a possibility but I think it would be better if the modules just worked together without any manual configuration by the administrator.

Attached is a patch that removes the hard-coding of form IDs. A sufficient check for the password field seems to be just:

isset($form['account']) && isset($form['account']['pass'])

It is more complex than the original code though because it was necessary to force the hook_form_alter() to execute after the hook_form_FORM_ID_alter() of Password Reset Landing Page using hook_module_implements_alter(). Otherwise Password Policy does not get the password field in the $form object, because PRLP uses hook_form_FORM_ID_alter() to inject it. The patch in #1 does not seem to work due to this call ordering, and I think the excerpt in #3 only works if the module is properly weighted or you get lucky. Please correct me if wrong.

The JavaScript constraint checking does not fully work with this patch, because $user is the anonymous user in password_policy_ajax_check() instead of the user for whom the password is being changed. (This is also a problem with the patch in #1 and the excerpt in #3). For instance, if only the username constraint is enabled, and user 'foo' tries to set their password to 'foo', the JavaScript check will tell the user that all constraints are satisfied, but when they submit, validation will fail.

This is my patch for this issue with the 7.x-1.dev release. I'll look at testing patch #5 with 7.x-2.dev when I get a chance.

Switching issue status to Needs review for review, testing and reporting of latest patch (see #7).
It would be great if Automated Testing could be enabled in module's tracker.
Cheers!

Correction on previous comment: Automated Testing is enabled. This issue just never got in Needs review status.
Forgot to relate it with Password Reset Landing Page (PRLP)'s #2294381: No minimal password length?, thus another edit.
Cheers!

Thanks for the 7.x-1.x patch in #7, but it has the same problem as mentioned in #2. Maybe #5 could be backported.

The automated tests have probably failed for #7 because the version of this issue is currently set to 7.x-2.x-dev. New features should go in to 2.x first (per project page).

The JavaScript constraint checking does not fully work with this patch, because $user is the anonymous user in password_policy_ajax_check() instead of the user for whom the password is being changed. (This is also a problem with the patch in #1 and the excerpt in #3). For instance, if only the username constraint is enabled, and user 'foo' tries to set their password to 'foo', the JavaScript check will tell the user that all constraints are satisfied, but when they submit, validation will fail.

One solution to this would be to pass the user ID as a POST parameter, so password_policy_ajax_check() can load the appropriate user object. This would introduce a security flaw, though, because if the history constraint were enabled, AJAX requests could then be used to brute force previous user passwords for arbitrary users.

A possible way to the prevent brute force attacks, or at least slow them down, would be to use Drupal's flood control. Every time password_policy_ajax_check() is called an event is registered with flood_register_event() [1] and then a check is made to see if there have been too many connection attempts with flood_is_allowed() [2].

I'm happy to look into putting a patch together for this if it is thought to be a good solution.

[1] https://api.drupal.org/api/drupal/includes!common.inc/function/flood_register_event/7
[2] https://api.drupal.org/api/drupal/includes!common.inc/function/flood_is_allowed/7

#7 works in my case for 7.x-1.x
Thanks stephen-cox for your efforts.

Hi,

I have the exact same problem, but after reading all this I still don't know how to solve it.
Anyone can help me please?

Stefan97, I do not think there is a fully working solution for 7.x-2.x currently. The 7.x-2.x patch in #5 seems to have problems. The 7.x-1.x patch in #7 may work, though.

Edit: Updating this comment as I misread the patch in #7 before. It looks to not have the problem I thought it did.

I have the objections in #2 to patch in #7: We should not be hard-coding user_pass_reset in the code because it will not be clear to someone reading the code why that is there, and it would not be covered by tests. A better approach would seem to be to check for the presence of the password fields and modify those forms. If the password and confirm password fields are present, you know it is a form on which the password can be changed.

How about if the code in password_policy_form_alter() were placed into a callable function so that other modules could call it?

How about:

function _password_policy_is_covered_form($form_id) {
  $covered_form_ids = array(
    'user_profile_form',
    'user_register_form',
  );
  // Let other modules add to the list.
  drupal_alter('password_policy_is_covered_form', $covered_form_ids);
  return in_array($form_id, $covered_form_ids);
}

In my opinion, the proper solution would be for PRLP and Password Policy to work together without knowing about each other. The suggestion in #19 is good, but would require PRLP to know about Password Policy (i.e., have code in its module specifically to integrate with Password Policy).

Agree with @NancyDru. It will be useful. Have provided patch for it https://www.drupal.org/node/2562481

Using patch #5

1. +++ b/password_policy.module
   @@ -244,16 +244,28 @@ function password_policy_js_alter(&$javascript) {
   +    $form['#submit'][] = 'password_policy_password_submit';
   

   On applying this patch.
   with 7.2.x branch.
   Call to undefined function password_policy_password_submit();
   Need to remove this and it's working fine.

Just tried PRLP with the latest 7.x-2.x release (7.x-2.0-alpha6), and policies seem to be applied to the password field at user/%uid/edit upon login.

emag, could you post which Password Policy version number you are experiencing the problem with? Did you change any of the default PRLP settings?

emag, thanks, I was able to reproduce the problem following your steps in #25 on a fresh Drupal site.

The cause of the problem is that Password Policy checks for the password element in hook_form_alter(), and modifies it as necessary. However, PRLP adds the password element to the user_reset_form using hook_form_FORM_ID_alter(), which is called after hook_form_alter(). So Password Policy does not see the added password element.

Per https://api.drupal.org/api/drupal/modules!system!system.api.php/function... :

Within each module, form alter hooks are called in the following order: first, hook_form_alter(); second, hook_form_BASE_FORM_ID_alter(); third, hook_form_FORM_ID_alter(). So, for each module, the more general hooks are called first followed by the more specific.

I am not sure of a good solution to this problem. Any suggestions? I would not want Password Policy to have any specific knowledge of PRLP (i.e., no "if PRLP..." logic).

Posted PRLP patch which seems to fix the incompatibility with latest Password Policy 7.x-2.x, in this comment:

#2294381-5: No minimal password length?

If either module is going to have knowledge of the other, it may make more sense for PRLP to know about Password Policy, than vice versa, since PRLP is (currently) the less popular module (in terms of site installations).

3. Went to email and clicked the one time use link.
4. Am brought to the user/reset/uid url. I'm forced to change the password in compliance with password policy. AWESOME!
5. I'm brought to the user/uid/edit screen. The first two messages are fine: 'you have successfully validated your email address' and 'your new password has been saved'. HOWEVER, the third message says 'you have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.'

Cannot reproduce this. I followed steps 3 and 4, and am redirected to the user/%uid/edit page, but no messages are displayed.

I am using a fresh Drupal site. I just enabled the default, example password policy and left PRLP set to its defaults. Maybe you could try reproducing this problem from a fresh Drupal site? Perhaps there is something else involved here.

Could someone clarify for me . . . ? It sounds like one of the solutions listed above works, but I'm not clear which one. Is the fix now in the Dev version of this module? It also sounds like one also needs to be using the Dev version of PRLP as well.

I answered my own question by installing Dev on both this and the PRLP module. The policy compliance is now enforced using PRLP. There still remains the java script issue described in #15 but at least the module works otherwise. Thanks for everyone's work on this issue.

there's a patch, so should be NW or NR

Was this issue only fixed for the 7.x-2 branch? I am using the latest password_policy 7.x-1.x-dev from today with PRLP 7.x-1.13, where the patch from #28 should be commited. Unfortunately on the password reset page Drupal's default password hints are shown instead of my password policy settings and it is even possible to submit the form with a one-digit-password …

Thanks for a quick reply!

ABaier, I am guessing the problem with PRLP and Password Policy 7.x-1.x is actually #2562481: Apply password policies to account password elements on custom forms.

Password Policy 7.x-2.x can apply password policies to password elements on custom forms, whereas 7.x-1.x only applies password policies to a few specific, hard-coded forms. I cannot remember, but I suspect PRLP provides a custom form to which only 7.x-2.x applies password policies.

It would be useful to know if one of the 7.x-1.x patches in that issue solves this problem for you.

Thanks for clarifying this. I switched to the 7.x-2.x branch after posting this and got it working now. Just hesitated to use it in alpha-state first.
I did not investigate the patches for 1.x any further, sorry.

Just want to share this in case others find it helpful.

I have a site which is using the Password Hustle module to provide a Password Reset form from a one-time login link. I just tried uninstalling Password Policy 1.x and installed 2.x but the rules I setup were still not enforced when using the form provided by Password Hustle.

I have reverted to Password Policy 1.x and am successfully enforcing its rules on this form having added the Form ID to the list in password_policy_form_alter() as follows:

function password_policy_form_alter(&$form, $form_state, $form_id) {
  global $user;

  switch ($form_id) {
    case 'user_profile_form':
    case 'user_register_form':
    case 'password_policy_password_tab':
    case 'password_hustle_user_pass_reset':
    case 'another_password_form_to_enforce':

[...]

I've already spent too much time trying to get this working to begin troubleshooting the 2.x version at this stage. It would be great if it supported the forms created by Password Hustle and other modules but I see no documentation on this available, so this hack to the 1.x module will have to suffice for now.

nerdcore,

7.x-2.x will apply policies to any form that has an $form['account']['pass'] element of type 'password_confirm'. A number of contributed modules with custom password forms use that structure. I'd guess that Password Hustle is using something atypical. Will investigate.

7.x-1.x hardcodes specific form IDs to apply password policies to. I was hoping to change to the approach used by 7.x-2.x.

There are many modules with custom password forms so we need some general approach that does not require continually adding form IDs for every module that crops up.

nerdcore, here's a Password Hustle patch that should make it work with Password Policy 7.x-2.x. It just makes the Password Hustle form more consistent with other forms that have an account password. Note that for the dynamic, JavaScript list of unmet requirements to work, the policy will need to also apply to the 'anonymous user' role.

Password Hustle might work with 7.x-1.x if this patch and the patch in #2562481-19: Apply password policies to account password elements on custom forms are applied.

This Integration is needed for Drupal 8 too.
#2917622: Password expires (Password Reset Days) does not work

1. Introduction, Threat Models
at 18:20
https://youtu.be/GqmQg-cszw4?t=18m20s

#2924009: Apply password policies to password elements on custom forms

is there a patch version of this that works for drupal 8?

Here is the patch for Drupal 8 for password policy rules on PRLP form

Started implementing my compatibility fix for the prlp module in a new contrib module: https://www.drupal.org/project/password_policy_extras