WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 3 months ago

Last modified 8 days ago

#16470 closed enhancement (fixed)

Require confirmation on email change

Reported by: linuxologos Owned by: johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.0
Component: Users Keywords:
Focuses: administration Cc:

Description

When a new user is registered for a site, the e-mail he provides gets easily confirmed. But immediately after that, the new member can visit his profile and is able to change his e-mail to anything. Regardless of whether it is done on purpose or the user enters a wrong e-mail by mistake, the admin cannot contact the member, should he has to for any reason. The e-mail address is of great importance in such cases and I don't think that's a rare need!

I've had the impression that WP was not offering this feature, but then I realised that the code lies in core, though restricted to multisite installations. I find it quite difficult to understand why.

There might seem to be a relation to #13717, but what I propose hereby is just giving the admin of a single-site installation the option to activate e-mail change confirmation.

I think the implementation would only require a few changes in wp-admin/user-edit.php, making send_confirmation_on_profile_email() available outside of wp-admin/includes/ms.php and adding an option in Settings.

Why would we have to hack the core or consider a plugin for something almost already offered in core? That's why I describe the ticket as "enhancement", not "feature request".

Attachments (4)

16470.patch (11.0 KB) - added by rodrigosprimo 15 months ago.
Require confirmation on email change on single site installs
16470.2.patch (11.1 KB) - added by johnbillion 11 months ago.
16470.diff (13.8 KB) - added by tharsheblows 11 months ago.
adds unit tests
16470.3.patch (14.4 KB) - added by johnbillion 3 months ago.

Download all attachments as: .zip

Change History (29)

#1 @linuxologos
7 years ago

  • Version set to 3.0

#2 @c3mdigital
4 years ago

  • Resolution set to invalid
  • Status changed from new to closed

This would be a great plugin. Don't think it's needed in core.

#3 @johnbillion
4 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I know there's been no traction since this ticket was opened, but I think this would actually be a neat feature for single site installations.

Note that this functionality exists as described in the ticket when you're using Multisite. A change of either a user profile email address or the admin email address will trigger a confirmation email with a link which needs to be clicked in order to confirm the change.

I'll patch this up and then we can discuss.

#4 @chriscct7
2 years ago

  • Keywords needs-patch added

#5 @swissspidy
2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from reopened to closed

Duplicate of #32430.

Since WordPress 4.3 email notifications will be sent out in the event that an email or password is changed.

#6 @johnbillion
2 years ago

  • Keywords 2nd-opinion added
  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

This isn't really a dupe of #32430. This ticket is concerned with the confirmation before changing the address, not the notification afterwards.

The confirmation request should also be sent when changing the site admin email, same as multisite.

One complication is sites that cannot send emails, which I presume is why this is limited to multisite currently (less likely to not have outgoing email working).

#7 @knutsp
2 years ago

Should we have a constant like WP_NO_EMAILS or an option, so that when not true, such suggested featured could be implemented?

@rodrigosprimo
15 months ago

Require confirmation on email change on single site installs

#8 @rodrigosprimo
15 months ago

  • Keywords has-patch added; needs-patch removed

To make confirmation mandatory on email change on single site installs all I had to do was move a few functions from multi site specific files to generic files and change two checks on src/wp-admin/user-edit.php. As far as I could test this patch is working but it is good if someone else could test it as well.

#9 @johnbillion
11 months ago

  • Focuses administration added
  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 4.8
  • Owner set to johnbillion
  • Status changed from reopened to reviewing

#10 @tharsheblows
11 months ago

One issue with requiring confirmation from the old email address is that sometimes people are changing it because it's incorrect or they no longer have access to it. But it's already this way on multisite so hmmm, I guess there aren't any problems there. However, I know I would have problems with it. I mean my users would have problems with it.

So it would be good to be able to not require the confirmation. I'm not sure what should be used to do this - a constant, settings option or simply a filter.

#11 @johnbillion
11 months ago

The confirmation email is sent to the new address, not the old one. Its intent is to prevent a user from changing their email address to one which they have no access.

#12 @tharsheblows
11 months ago

Oh ha! Go me but I make this type of mistake often, I'm pretty used to it.

I'd still want to be able to disable the emails. I've disabled wp_new_user_notification() so getting an email check here would feel strange. I think the main things are that I want control over the emails the site sends and not have any basic functionality rely on a person receiving an email and clicking a link in it because sometimes site emails get caught in spam or go undelivered for whatever reason.

But I can just remove the action, can't I? I'm sorry, I'm typing this without properly testing. I'll do some unit tests, feel free to ignore until then. :)

@tharsheblows
11 months ago

adds unit tests

#13 @tharsheblows
11 months ago

Sorry I wasn't paying attention to the filename, can redo if you'd like.

So that was right, disabling it all was as easy as removing the action. I'm not sure how to test the final bit in src/wp-admin/user-edit.php but have left a note it should be in there.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#16 @jbpaul17
5 months ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 per today's bug scrub.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#19 @jbpaul17
3 months ago

  • Milestone changed from 4.8.1 to 4.9

Per today's bug scrub, we'll punt this as the focus for 4.8.1 is regressions only.

#20 @johnbillion
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41163:

Users: Require a confirmation link in an email to be clicked when a user attempts to change their email address.

This adds this previously Multisite-only functionality to single site installations too. This change prevents accidental or erroneous email address changes from potentially locking users out of their account.

Props rodrigosprimo, tharsheblows, johnbillion

Fixes #16470

#21 @johnbillion
3 months ago

  • Keywords needs-dev-note added; has-patch removed

#22 @johnbillion
3 months ago

In 41165:

Users: Re-add entity decoding to the site name before it's used in the email address change confirmation email.

This was accidentally removed in [41163].

See #40015, #16470

#23 @johnbillion
3 months ago

In 41171:

Users: Further fixes to entitiy decoding in the user email address change confirmation email, and the corresponding tests.

See #16470, #40015

#24 @johnbillion
2 months ago

In 41255:

Options, Meta APIs: Update the multisite unit tests after [41254], [41164], and [41163].

This moves some more previously Multisite-only tests into the main test suite, and makes small adjustments to their assertions.

See #39118, #16470, #39117

#25 @johnbillion
8 days ago

  • Keywords needs-dev-note removed
Note: See TracTickets for help on using tickets.