Opened 3 years ago
Last modified 3 years ago
#54481 new defect (bug)
User list doesn't honor allow_password_reset filter
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | minor | Version: | 5.8.2 |
Component: | Users | Keywords: | |
Focuses: | administration | Cc: |
Description
In WP 5.7 or so, the user list (Users - All Users in the admin section) began to display a "Send password reset" link for users (other than yourself). On many of my sites, I have a filter like this:
add_filter('allow_password_reset', '__return_false');
(I use an outside authentication system, so the WP-local passwords aren't used for anything, and most users can't get to WordPress' own login screen anyway.)
I expected the above filter also to suppress the links in the user list, but this appears not to be the case.
The patch for this is simple enough (a few lines of new code around line 279 in wp-admin/includes/class-wp-users-list-table.php), but before I write and test it, is this the best approach to fix this bug? i.e. is there anyone expecting that the allow_password_reset filter only applies to users doing self-service password resets, as opposed to admins sending resets on behalf of other users? If that's the case, this probably should be a separate filter or define of some sort.
Attachments (1)
Change History (6)
#2
follow-up:
↓ 4
@
3 years ago
As a newbie question, what is the correct way to handle the bulk dropdown? (Assuming allow_password_reset is a per-user flag, which is implied by the current filter, if it's in the bulk dropdown it would succeed for some users but fail for others.)
Since allow_password_reset can take a user ID, does that imply that there are cases (now or in the future) where a given user may be able to reset some other users' passwords, but not those of other users?
#3
@
3 years ago
As another newbie question, where would a hypothetical is_password_reset_allowed() function go? My first guess would be wp-includes/user.php, probably near the existing get_password_reset_key(), but I've been burned before by not understanding which of the many many WP files are which.
#4
in reply to:
↑ 2
@
3 years ago
Replying to desmith:
As a newbie question, what is the correct way to handle the bulk dropdown? (Assuming allow_password_reset is a per-user flag, which is implied by the current filter, if it's in the bulk dropdown it would succeed for some users but fail for others.)
Ah, good point. We might want to leave the bulk actions dropdown as is then.
Since allow_password_reset can take a user ID, does that imply that there are cases (now or in the future) where a given user may be able to reset some other users' passwords, but not those of other users?
Yes, I think that would be currently possible by returning true
or false
depending on the current user and the passed user ID value.
As another newbie question, where would a hypothetical is_password_reset_allowed() function go? My first guess would be wp-includes/user.php, probably near the existing get_password_reset_key(), but I've been burned before by not understanding which of the many many WP files are which.
My suggestion would be directly above get_password_reset_key()
.
Hi there, welcome back to WordPress Trac!
Indeed, I think it makes for these links to be hidden if the
allow_password_reset
filter returnsfalse
.Just noting that there are several instances of these links:
Send password reset
in the bulk actions dropdown.Send password reset
in action links.Send Reset Link
on Edit User screen.To make the check more reusable, perhaps this code from
get_password_reset_key()
should be separated into its own function, e.g.is_password_reset_allowed()
: