Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comments
Comment #1
erikwebb CreditAttribution: erikwebb commentedI 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.
Comment #2
erikwebb CreditAttribution: erikwebb commentedWhere 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.
Comment #3
dman CreditAttribution: dman commentedGood idea. This should address a few edge cases for us.
Comment #4
erikwebb CreditAttribution: erikwebb commentedImplemented and committed.
http://drupalcode.org/project/password_policy.git/commit/b08f139b8602bbc...
Comment #5
erikwebb CreditAttribution: erikwebb commentedComment #6
rooby CreditAttribution: rooby commentedHere is a patch for D7.
Comment #8
rooby CreditAttribution: rooby commentedOops, I was playing around with something else before that somehow got its way into that patch.
Here is a better one.
Comment #10
erikwebb CreditAttribution: erikwebb commentedComment #11
erikwebb CreditAttribution: erikwebb commented#8: password_policy-exclude_pages-1332000-8.patch queued for re-testing.
Comment #13
erikwebb CreditAttribution: erikwebb commentedFor brand new features, let's focus those on 7.x-2.x to start.
Comment #14
AohRveTPV CreditAttribution: AohRveTPV commentedI 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.
Comment #15
AohRveTPV CreditAttribution: AohRveTPV commentedLooked 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:
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.)Comment #16
AohRveTPV CreditAttribution: AohRveTPV commentedThis 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 allowuser/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.
Comment #17
AohRveTPV CreditAttribution: AohRveTPV commented- Update to apply on latest code.
- Document return value data types.
Comment #18
AohRveTPV CreditAttribution: AohRveTPV commentedAdd test.
To-do: Add update code.
Comment #19
AohRveTPV CreditAttribution: AohRveTPV commentedUpdating the D6
password_policy_exclude_pages
variable (i.e., migrating its contents topassword_policy_extra_allowed_paths
) seems difficult, because paths may have changed between D6 and D7. A pathfoo/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.Comment #20
hanoii#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.Comment #22
AohRveTPV CreditAttribution: AohRveTPV commentedThanks 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.
Comment #24
AohRveTPV CreditAttribution: AohRveTPV commentedComment #25
AohRveTPV CreditAttribution: AohRveTPV commented#2481101: Add file/ajax/* to the extra allowed paths default value
Comment #26
DCLoganGuy CreditAttribution: DCLoganGuy commentedGreetings,
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
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.)