Make WordPress Core

Opened 9 years ago

Last modified 5 years ago

#33209 new enhancement

Inviting a new user to Multisite results in password being emailed

Reported by: ipstenu's profile Ipstenu Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch dev-feedback
Focuses: multisite Cc:

Description

If you add a new user from a site users page (NOT the network one, /sitename/wp-admin/user-new.php ) the flow results in a password being emailed in plaintext.

1) Add new user

2) New user gets email to activate

3) Activate link (ex. example.com/sitename/wp-activate.php?key=5324e8cf2cef143b ) shows the new password

4) The following email is sent:

Howdy anotherstenu,

Your new account is set up.

You can log in with the following information:
Username: anotherstenu
Password: 78HoBi6oFSf9
http://local.multisite-pre.dev/blarg/wp-login.php

Thanks!

--The Team @ Multisite Naked Sites

Whoops.

It looks like this can be fixed for new sites by updating wp-includes/ms-functions.php, however this is set in the database on Network Activation, which means even changing core doesn't update the myriad sites who are merrily emailing out passwords because this is set (wp admin -> Network settings -> Welcome User Email)

Howdy USERNAME,

Your new account is set up.

You can log in with the following information:
Username: USERNAME
Password: PASSWORD
LOGINLINK

Thanks!

--The Team @ SITE_NAME

The attached patch addresses new setups and doesn't break existing ones since I'm really not sure what's best here. I want to say we should edit everyone's DB and change the above block to this:

Howdy USERNAME,

Your new account is set up.

Username: USERNAME

To set your password, visit the following address:

<RESETLINK>

Thanks!

--The Team @ SITE_NAME

However there are myriad people who have customized that simply because they can, and I fear the damage of breaking them.

Attachments (5)

33209.diff (2.1 KB) - added by Ipstenu 9 years ago.
Change default Multisite NOT to email passwords
33209.2.diff (3.8 KB) - added by thomaswm 8 years ago.
Backwards-compatible fix
33209-3.diff (5.9 KB) - added by BjornW 6 years ago.
Updated patch, using apply_deprecated_filters to deprecate password token
BjornW-mu-no-plain-password-bba70b4db848.zip (5.1 KB) - added by BjornW 6 years ago.
mu-plugin which automagically replaces the PASSWORD token for RESET_LINK (and thus replaced with the password re(set) link) including admin_notice for super_admins
BjornW-mu-no-plain-password-069688dc398c.zip (6.1 KB) - added by BjornW 6 years ago.
Updated mu-no-plain-password plugin: hides (cannot remove due to lack of filters/hooks in wp-activate) username and password upon activation of user/blog

Download all attachments as: .zip

Change History (29)

@Ipstenu
9 years ago

Change default Multisite NOT to email passwords

This ticket was mentioned in Slack in #core-multisite by ipstenu. View the logs.


9 years ago

#3 follow-up: @johnbillion
9 years ago

Rather than overwriting the entire welcome_user_email option, we could replace the Password: PASSWORD string which is quite likely to appear in the message even if it's been modified.

#4 @johnbillion
9 years ago

  • Keywords has-patch dev-feedback added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

#5 in reply to: ↑ 3 ; follow-up: @SergeyBiryukov
9 years ago

Replying to johnbillion:

Rather than overwriting the entire welcome_user_email option, we could replace the Password: PASSWORD string which is quite likely to appear in the message even if it's been modified.

How would that work for translations?

#6 @Ipstenu
9 years ago

I was trying to balance out old and new and make it match what we have in single site. By replacing but leaving the extant search/replace checks, it would not break current Multisite but still get new people 'right'.

Also that way people don't go "This doesn't match what I have in my network admin!"

It's probably a bad idea to search the DB for a perfect match to the old one and replace that, huh?

#7 in reply to: ↑ 5 ; follow-up: @johnbillion
9 years ago

Replying to SergeyBiryukov:

How would that work for translations?

It wouldn't, unless we introduced Password: PASSWORD as a new translatable string as used that for the replacement.

Another option would be to change the behaviour of the PASSWORD token so it outputs the password reset link, but I suspect that's asking for trouble.

#8 in reply to: ↑ 7 @Ipstenu
9 years ago

Replying to johnbillion:

Another option would be to change the behaviour of the PASSWORD token so it outputs the password reset link, but I suspect that's asking for trouble.

That could wreak havoc with anyone who customized their email. And there's no way to tell who did and didn't. I'm on that knife edge between "For god's sake you're doing it all wrong!" and "I shouldn't break things for people."

#9 follow-up: @Justin_K
9 years ago

I was just testing my plugins against WP4.3, and discovered that while I could previously notify only admins of new user registrations by calling wp_new_user_notification() (with no 2nd parameter), in 4.3 the behavior is changed: now it seems to always notify both admins and users (there is no longer a 2nd parameter that can be left blank to suppress emails to users).

How can I get back the behavior of WP prior to 4.3: send new user notifications to the admin *only*?

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


9 years ago

#11 in reply to: ↑ 9 @johnbillion
9 years ago

Replying to Justin_K:

How can I get back the behavior of WP prior to 4.3: send new user notifications to the admin *only*?

Sorry you didn't get a more timely response to your comment. This issue has been addressed in #33654, and will be included in 4.3.1.

@thomaswm
8 years ago

Backwards-compatible fix

#12 @thomaswm
8 years ago

33209.2.diff is an attempt to fix both this ticket and #33186 in a backwards-compatible way. It does the following things:

  • Changes the default text for notification emails so that they contain a placeholder RESETPWLINK instead of PASSWORD.
  • Performs a simple false !== strpos( $welcome_email, 'PASSWORD' ) check in wp_welcome_notification/wp_welcome_user_notification to see if the email text in the welcome_email/welcome_user_email option contains the placeholder for the password.
    • If it does, then the password is sent to the user in plain text (in order to maintain backwards compatibility).
    • Otherwise, a password reset link is sent instead.
Last edited 8 years ago by thomaswm (previous) (diff)

#13 @Mista-Flo
8 years ago

+1 for this ticket that needs attention.

Last edited 8 years ago by Mista-Flo (previous) (diff)

This ticket was mentioned in Slack in #core-multisite by bjornw. View the logs.


6 years ago

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


6 years ago

@BjornW
6 years ago

Updated patch, using apply_deprecated_filters to deprecate password token

#17 @BjornW
6 years ago

A bit more info about this patch:

In short:

This patch will make a new installation of default WordPress Mu installation safer by removing the plain-text passwords from the welcome_email and welcome_user_email emails. It respects existing installations by not changing their settings (yet), but it will warn them that the PASSWORD token is deprecated.

Details:

  • It replaces the PASSWORD token from the default 'Welcome Email' and 'Welcome User Email' template texts with a new token RESETLINK in the code. It does *NOT* change settings in the database to preserve backwards-compatibility.

In a future WordPress version we should remove the PASSWORD token completely and replace it with the RESETLINK token automagically. However doing this now, might be to abrupt for users. Therefor I assume we want to deprecate and warn people first.

  • It refactors the PASSWORD token replacement functionality into using a new filter called 'wpmu_replace_password_token'. This filter is being called using apply_filters_deprecated to immediately deprecate the function so we can set a notice warning about NOT using the PASSWORD token anymore.

It might even be extended into using an admin notice in the wp-admin for users with super_admin role, to make sure they are aware of this upcoming change

  • The RESETLINK token functionality uses a new filter called 'wpmu_replace_resetlink_token' to replace the RESETLINK token for a re(set) url.

To discuss:

  1. Is this the proper way to deprecate the usage of the PASSWORD token?
  2. Should we warn users with super_admin role about this change using an admin notice?
  3. Should we respect the existing settings or replace them automagically with the re(set) functionality now without even warning them?

@BjornW
6 years ago

mu-plugin which automagically replaces the PASSWORD token for RESET_LINK (and thus replaced with the password re(set) link) including admin_notice for super_admins

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


6 years ago

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


6 years ago

@BjornW
6 years ago

Updated mu-no-plain-password plugin: hides (cannot remove due to lack of filters/hooks in wp-activate) username and password upon activation of user/blog

#20 @Mista-Flo
5 years ago

Hey, @BjornW Thanks for your work, your patch is pretty solid.

All: Is it possible to move forward and get this ticket handled soon ? We should avoid this lack of consistency between Multisite and non Multisite mode.

Cheers,

#21 @sebastienserre
5 years ago

agree with @Mista-Flo ! This ticket should be in 5.3

#22 @BjornW
5 years ago

@Mista-Flo & @sebastienserre Thank you!

I'd like to see my patch added to WordPress, but as you can see there was no response for roughly 7 months...I tried to get a Core committer interested in this by discussing it on Slack, but apparently my timing was not right. It seems WordPress Core is focused on other issues and considers this not a priority now.

Frankly I don't know how to proceed with tickets anymore. It is demoralizing to see (my) contributions lay dormant for so long due to a lack of time/interest from Core committers.

Unless someone with Core commit rights is interested in helping this get into Core, I (as a self-employed developer) find it harder and harder to invest any more time in this (and other WordPress tickets).

Maybe one of you are able to gain the interest of someone with commit rights?


#23 follow-up: @Mista-Flo
5 years ago

Well, I have the same experience than you, a lot of tickets take a very long time being reviewed.

But we should thank core committers, they are doing an amazing job, and it's a voluntary one.

#24 in reply to: ↑ 23 @BjornW
5 years ago

Replying to Mista-Flo:

Well, I have the same experience than you, a lot of tickets take a very long time being reviewed.

But we should thank core committers, they are doing an amazing job, and it's a voluntary one.

Oh, I'm thankful, but also painfully aware that this is a problem which has been going on for quite some time while those able to do something about this ignoring the issue at hand.

Note: See TracTickets for help on using tickets.