Here's a patch to allow the admin to specify pages to exclude pages from the force password change redirect.

I didn't make the patch from git, so it may not apply cleanly. I'll make a git patch if you want.

Files: 
CommentFileSizeAuthor
#18 password_policy-force_change_extra_allowed_paths-1332000-18.patch9.01 KBAohRveTPV
PASSED: [[SimpleTest]]: [MySQL] 847 pass(es).
[ View ]
#8 password_policy-exclude_pages-1332000-8.patch2.93 KBrooby
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-exclude_pages-1332000-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 password_policy-exclude_pages-1332000-6.patch3.02 KBrooby
FAILED: [[SimpleTest]]: [MySQL] 327 pass(es), 35 fail(s), and 150 exception(s).
[ View ]
#4 password_policy-feature_exclude_pages-1332000-4.patch3.05 KBerikwebb
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-feature_exclude_pages-1332000-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 password_policy-exclude_pages_from_force_change-1332000-2.patch2.49 KBerikwebb
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-exclude_pages_from_force_change-1332000-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
password_policy_exclude_pages.patch2.61 KBslip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy_exclude_pages.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

erikwebb’s picture

I like the idea. Looks clean and works for me. Because it's a new feature, I'd like another set of eyes on it before I add this.

And yes, a Git patch would be nice, but don't worry about changing this one.

erikwebb’s picture

StatusFileSize
new2.49 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-exclude_pages_from_force_change-1332000-2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Where is password_policy_profile_page coming from in your patch? Is that something else you've added?

Other than that, it looks good to me. I've attached a proper Git patch.

dman’s picture

Good idea. This should address a few edge cases for us.

erikwebb’s picture

Status:Needs review» Patch (to be ported)
StatusFileSize
new3.05 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-feature_exclude_pages-1332000-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
erikwebb’s picture

Version:6.x-1.x-dev» 7.x-1.x-dev
rooby’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new3.02 KB
FAILED: [[SimpleTest]]: [MySQL] 327 pass(es), 35 fail(s), and 150 exception(s).
[ View ]

Here is a patch for D7.

Status:Needs review» Needs work

The last submitted patch, password_policy-exclude_pages-1332000-6.patch, failed testing.

rooby’s picture

Status:Needs work» Needs review
StatusFileSize
new2.93 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch password_policy-exclude_pages-1332000-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Oops, I was playing around with something else before that somehow got its way into that patch.
Here is a better one.

Status:Needs review» Needs work

The last submitted patch, password_policy-exclude_pages-1332000-8.patch, failed testing.

erikwebb’s picture

Status:Needs work» Needs review
erikwebb’s picture

Status:Needs review» Needs work

The last submitted patch, password_policy-exclude_pages-1332000-8.patch, failed testing.

erikwebb’s picture

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

For brand new features, let's focus those on 7.x-2.x to start.

AohRveTPV’s picture

I implemented this same feature (not yet committed) in the context of a bug report: #2372935: Multiple forms when resetting password in admin theme. It is probably more appropriate as a feature request though because the module is working as intended. Will plan to compare the current implementation with the old implementations here, and re-implement for 2.x first.

AohRveTPV’s picture

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

Looked into implementing this in 2.x, but came away feeling it might actually be better to implement in 7.x-1.x first, for several reasons:

  1. 2.x seems to already provide a mechanism for administrators to exclude pages from the forced-password-change redirect: A module can implement hook_password_policy_expire_url_exclude() and return a non-empty value if the current path should be excluded. (Granted, this would only be useful administrators who know how to write a module.)
  2. The expiration plug-in code for 2.x looks to have several problems. I think it would be easier to fix the problems before extending the plug-in with this feature.
  3. The lack of this feature in 7.x-1.x causes problem for a number of users. It is maybe not a bug with this module, but it can be a bug for their sites.
  4. The feature already exists in 1.x (6.x-1.x). Ideally users should be able to upgrade from 6.x-1.x to 7.x-1.x without losing functionality.
AohRveTPV’s picture

This patch adds this feature on latest 7.x-1.x.

A couple select differences versus #8:
1. user/logout is not configurable. system/ajax is the only path allowed by default that is configurable. Is there any good reason someone might want to not allow user/logout when a password change is forced?
2. Setting is called "Extra allowed paths" instead of "Page exclusion list". I do not think the term "exclude" would be clear to an administrator. Excluded from what? I think it may be clearer to explain these paths as paths that are "allowed" when a password change is forced.

Not sure what the #wysiwyg key in #8 is for? I do not see it in the Form API.

To-do:
1. Add test(s).
2. Add code to update this setting from 6.x-1.x to 7.x-1.x.

AohRveTPV’s picture

StatusFileSize
new5.56 KB

- Update to apply on latest code.
- Document return value data types.

AohRveTPV’s picture

StatusFileSize
new9.01 KB
PASSED: [[SimpleTest]]: [MySQL] 847 pass(es).
[ View ]

Add test.

To-do: Add update code.

AohRveTPV’s picture

Status:Needs work» Needs review

Updating the D6 password_policy_exclude_pages variable (i.e., migrating its contents to password_policy_extra_allowed_paths) seems difficult, because paths may have changed between D6 and D7. A path foo/bar may be something the administrator wishes to allow access to in D6, but does not wish to allow access to in D7. Therefore, the more secure default seems like to not update the variable.

A possible solution would be to provisionally update the variable, and let the administrator decide whether to keep the updated paths or discard them. However, I do not know an easy way to implement this.

For now we can just leave the old variable around (i.e., don't variable_del()), for possible future migration, and so there is no loss of settings information.