password_tab should redirect elsewhere after password change.

Patch attached. This is just redirecting to user view page. Should be applied within password_policy/contrib/password_tab/.

Attachment	Size
password_tab_redirect.patch	638 bytes

I had a specific requirement to separate password change form from the profile page. So I would prefer having a configuration option instead of just defaulting to the profile.

@miglius:
Thanks for your quick response.

>I had a specific requirement to separate password change form from the profile page.
Yes, you did a great job, and it works great. I really like it - otherwise I would not enable it for few of my sites.

>So I would prefer having a configuration option instead of just defaulting to the profile.
Do you want me to add configuration option or would you implement it? I could do that if you want.

It would be quicker if you attached a patch.

Here you go - amended patch, now with configuration form and variable. You almost made me sweat ;) Now you can't back out from committing it.

Attachment	Size
password_policy_tab.patch	4.6 KB

Even better patch - this includes call to _user_password_dynamic_validation(), which makes password tab behave in the same way as the drupal core user/%/edit page.

Attachment	Size
password_policy_tab.3.patch	4.99 KB

After applying the patch the page 'admin/user/settings' does not open:

You're trying to use array union operator '+' on the non array $form['login'] ($form['login'] is undefined). You've created fieldset $form['password_tab'], but later in the code use the $form['login'] instead?

Instead of altering 'user_admin_settings' form I would prefer using a dedicated settings page at 'admin/settongs/password_change_tab', otherwise the setting page and place can be overlooked.

I deliberately don't use _user_password_dynamic_validation() javascript because password_policy module provides it's own javascript based on enabled and configured password policy constraints which replaces drupal's default javascript.

Also keep in mind that a user might be sent (redirected) to the password change page and it is expected that the user will be redirected back after the password is changed. It is working like this in current implementation and should work after the patch is applied. This is used for example with the openid provider module. When user logs in using openid, he is sent to his openid provider drupal instance and on that instance if the user's password has expired, he is redirected to the password change page and after the password is changed the user is redirected back to openid provider page to continue openid login process.

Thanks for the review! Sorry I missed 'login' that should be 'password_tab' (perils of copy-paste code in the middle of the night. It worked because there is another module in my sandbox that creates 'login' fieldset - the one I copied from). Attached is the corrected patch.

>Instead of altering 'user_admin_settings' form I would prefer using a dedicated settings page at 'admin/settongs/password_change_tab', otherwise the setting page and place can be overlooked.

Please, feel free to do as you wish. That's why I did the form in a function _password_policy_password_tab_admin_settings(). As for me, for a single field it seems too much to create a separate settings page. It also grows admin_menu needlessly.

>I deliberately don't use _user_password_dynamic_validation() javascript because password_policy module provides it's own javascript based on enabled and configured password policy constraints which replaces drupal's default javascript.

If policy replace the default - good, but if they don't - then default kicks in. Or am I missing something? Without it on my testbed there is no policy javascript at all. Feel free to drop that line out.

>Also keep in mind that a user might be sent (redirected) to the password change page and it is expected that the user will be redirected back after the password is changed. It is working like this in current implementation and should work after the patch is applied. This is used for example with the openid provider module. When user logs in using openid, he is sent to his openid provider drupal instance and on that instance if the user's password has expired, he is redirected to the password change page and after the password is changed the user is redirected back to openid provider page to continue openid login process.

Sorry, I have no way to set it up for test. Can you write a quick spec or a testcase for this? Maybe you can just improve that code by strategically placed "if (...)"?

Attachment	Size
password_policy_tab.4.patch	5 KB

nag nag

I have committed a slightly changed code to the cvs. The redirection won't be triggered if the destination is already set in the query string.

Automatically closed -- issue fixed for 2 weeks with no activity.