WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 7 months ago

#20140 assigned feature request

Ask old password to change user password

Reported by: nprasath002 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch commit 2nd-opinion
Focuses: Cc:

Description

I have experienced this in various sites and i think
it adds extra security.
We must ask for the old password when the user tries to change the password

Attachments (3)

20140.diff (3.7 KB) - added by bootsz 20 months ago.
Adds "Current Password" field to the profile & user-edit pages.
20140.2.diff (3.0 KB) - added by iandunn 8 months ago.
20140.3.diff (3.1 KB) - added by nacin 7 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 tman450621 months ago

  • Owner set to tman4506
  • Status changed from new to reviewing

comment:2 tman450621 months ago

  • Component changed from Administration to Security

comment:3 in reply to: ↑ description tman450621 months ago

Replying to nprasath002:

I have experienced this in various sites and i think
it adds extra security.
We must ask for the old password when the user tries to change the

I I think it's a great idea. I'll work on it no guarantees though

comment:4 tman450621 months ago

  • Status changed from reviewing to accepted

comment:6 bootsz20 months ago

  • Cc sbutze@… added
  • Keywords has-patch dev-feedback added

bootsz20 months ago

Adds "Current Password" field to the profile & user-edit pages.

comment:7 iandunn8 months ago

  • Cc ian.dunn@… added
  • Keywords needs-refresh added

+1

This was initially rejected in #4444, but I still think it's a good idea.

Most authentication systems employ this feature. Otherwise, an attacker could just walk up to a laptop while the owner isn't looking and change the password. It's easy to implement and doesn't place an unreasonable burden on the user.

One of of the reasons it was rejected was that "Someone with such access could install a backdoor, create a new user, or do any number of other things to engineer future access", but that assumes the current user is an Admin. They could be an Editor or other role.

Looks like the patch needs to be refreshed (and generated from / instead of /wp-admin).

comment:8 azaozz8 months ago

Thinking more about this: when users change their own passwords typing the old password is more or less expected and adds some security.

That's not the case when an admin is changing another user's password. The current patch also requires the admin to know the user's current password to be able to change it. That's not acceptable.

comment:9 iandunn8 months ago

Yup, I just noticed that too. I'm working on a refresh that allows admins to change it without knowing the current one.

Last edited 8 months ago by iandunn (previous) (diff)

iandunn8 months ago

comment:10 follow-up: iandunn8 months ago

  • Keywords needs-refresh removed

20140.2.diff refreshes the original patch, and allows admins to change other users' passwords without having to know what the current one is.

comment:11 SergeyBiryukov8 months ago

  • Milestone changed from Awaiting Review to 3.7
  • Owner tman4506 deleted
  • Status changed from accepted to assigned

comment:12 in reply to: ↑ 10 DrewAPicture8 months ago

Replying to iandunn:

20140.2.diff refreshes the original patch, and allows admins to change other users' passwords without having to know what the current one is.

Shouldn't we check whether that user can be edited by the current user rather than saying only administrators can change a user's password?

Last edited 8 months ago by DrewAPicture (previous) (diff)

comment:13 iandunn8 months ago

Shouldn't we check whether that user can be edited by the current user rather than saying only administrators can change a user's password?

The patch doesn't actually change any of the behavior related to capabilities; I just wasn't being precise when I said "administrators". The patch uses IS_PROFILE_PAGE to determine whether or not the extra field should be generated and validated, so if there's a custom role that can edit users then it shouldn't be affected at all.

nacin7 months ago

comment:14 nacin7 months ago

  • Keywords commit 2nd-opinion added; dev-feedback removed

I could go for this. 20140.3.diff is a clean-up.

I guarantee it'll take only a few days for us to receive a security report that states an administrator can simply create a new user, then log in as that user to change the first administrator's password. At the same time, though, that obviously could already happen.

This is designed for better user security for sub-administrator roles. Which is good, because they're commonly being used in attacks.

I'm going to put this on the agenda for Wednesday's meeting. Marking for commit (dev-wise) pending feedback as to whether we want this.

comment:16 iandunn7 months ago

xref: http://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-09-25&sort=asc#m696129

TL;DR: Instead of putting it in 3.7 as-in, we should look at expanding the scope and adding some extra protections for 3.8.

comment:17 nacin7 months ago

  • Milestone changed from 3.7 to Future Release
Note: See TracTickets for help on using tickets.