Feature to exclude pages from force password change

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.

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.

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

Implemented and committed.

http://drupalcode.org/project/password_policy.git/commit/b08f139b8602bbc...

Here is a patch for D7.

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

#8: password_policy-exclude_pages-1332000-8.patch queued for re-testing.

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

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.

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.

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.

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

Add test.

To-do: Add update code.

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.

#18 works awesome.

One addition though, might be good to add file/ajax/* to the default value for the new configuration setting, as it can be present on the user profile if it uses the default user picture.

One addition though, might be good to add file/ajax/* to the default value for the new configuration setting, as it can be present on the user profile if it uses the default user picture.

Thanks for the review and suggestion. I worry that path is used for more than just the user pictures, and that it could give some access to a user who is not supposed to be able to access the site beyond the password change page.

Will plan to open a separate issue for this. I do not think it will be a problem if this path is not included by default with the initial commit. We could just add it later, right.

One addition though, might be good to add file/ajax/* to the default value for the new configuration setting, as it can be present on the user profile if it uses the default user picture.

Will plan to open a separate issue for this. I do not think it will be a problem if this path is not included by default with the initial commit. We could just add it later, right.

#2481101: Add file/ajax/* to the extra allowed paths default value

Greetings,
You mention in comment #15 that there is a hook for password policy 7.x-2.x which can be used to add additional paths to exclude. Since we are using a feature, I tried using hook _password_policy_expire_url_exclude(). When I added the hook to one of the files in the feature, it does not run. Since the feature is named us_password_policy I added a new file called us_password_policy_expired. In the file I call the hook with the code below. If I put a break point to the left of function, the code is not called. This feature has been working fine for several years. Other hooks like user_insert(&$edit, $account, $category) work without issue. We need to exclude entity_legal. I am aware the code below is not complete. I need to make sure the path is whatever the entity_legal path is calling. I just wanted to make sure the hook is called before I completed. These modules fight for control and then timeout with too many redirects. Any suggestions?
thanks

/**
 * Implements hook_password_policy_expire_url_exclude().
 */
function us_password_policy_expired_password_policy_expire_url_exclude($account) { 
if (module_exists('entity_legal') {
return FALSE;
}

7.x-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.)