WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15725 closed defect (bug) (fixed)

"Skip Confirmation Email" checkbox value is ignored

Reported by: ocean90 Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Users Keywords:
Focuses: Cc:

Description (last modified by duck_)

Multisite: Go to wp-admin/user-new.php and add a new user but check the no confirmation box. Result:

Invitation email sent to new user. A confirmation link must be clicked before their account is created.

Attachments (2)

15725.diff (917 bytes) - added by duck_ 3 years ago.
15725.patch (1.5 KB) - added by ocean90 3 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 duck_3 years ago

  • Description modified (diff)

NB: this is Multisite only, since the checkbox on single site is TO "Send this password to the new user by email." instead of "Add the user without sending them a confirmation email."

Could be the reason for the problem (not looked at the source at all yet).

duck_3 years ago

comment:2 duck_3 years ago

  • Keywords has-patch added; needs-patch removed
  • Severity changed from normal to major

Not a multisite expert, but this looks like it's much worse than just the wrong confirmation text, wpmu_activate_signup is never called when creating a new user without sending them a confirmation email. This means that the key is not activated and without the email you would have to go to the database to get the key and activate manually.

Looks like the check for adduser in $_POST was added in [16527] to paper over a notice (see #15456).

Note that I cannot actually reproduce the notice described, I believe it's Notice: Undefined property: stdClass::$blogid in wp-includes/ms-functions.php:1390 (add a user without sending mail) after applying patch above, but I do now receive the notice if I go to the DB to retrieve the activation key to manually activate a user because of this bug.

comment:3 nacin3 years ago

  • Severity changed from major to blocker

Blocker pending further investigation and much-needed sleep.

comment:4 follow-up: ocean903 years ago

duck_: Thx for your investigation. Revert the part from [16527] and use the right var will fix the problem.

comment:5 in reply to: ↑ 4 duck_3 years ago

Replying to ocean90:

duck_: Thx for your investigation. Revert the part from [16527] and use the right var will fix the problem.

Are you sure $current_site->id is what we want there, not $current_site->blog_id?

comment:6 follow-up: wpdavis3 years ago

  • Cc will@… added

The isset( $_POST[ 'adduser' ] ) check is needed to make sure the right form is being submitted. In MS, there are two forms on that page that use the same names for fields — without that check you spawn a ton of errors because WordPress tries to submit two forms at once.

I'll take a look into the problem.

comment:7 in reply to: ↑ 6 ; follow-up: duck_3 years ago

Replying to wpdavis:

The isset( $_POST[ 'adduser' ] ) check is needed to make sure the right form is being submitted. In MS, there are two forms on that page that use the same names for fields — without that check you spawn a ton of errors because WordPress tries to submit two forms at once.

I'll take a look into the problem.

In that case I believe we would want $_POST['createuser'] on line 111 and I suppose $_POST['adduser'] above on line 65 (not looked into beyond checking the line numbers).

comment:8 wpdavis3 years ago

Sounds about right — I'll try it and see.

comment:9 in reply to: ↑ 7 duck_3 years ago

Replying to duck_:

Replying to wpdavis:

The isset( $_POST[ 'adduser' ] ) check is needed to make sure the right form is being submitted. In MS, there are two forms on that page that use the same names for fields — without that check you spawn a ton of errors because WordPress tries to submit two forms at once.

I'll take a look into the problem.

In that case I believe we would want $_POST['createuser'] on line 111 and I suppose $_POST['adduser'] above on line 65 (not looked into beyond checking the line numbers).

P.S. I cannot reproduce "a ton of errors" or the double form submit before or after removing isset($_POST['adduser']), shouldn't the checks on $_REQUEST['action'] be doing this anyway (lines 34 and 78)?

ocean903 years ago

comment:10 ocean903 years ago

duck_: You are right, fat finger or so, patch refreshed.

comment:11 ryan3 years ago

(In [16949]) Skip confirmation email fixes. Props ocean90, duck_. see #15725

comment:12 ryan3 years ago

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

comment:13 sillybean3 years ago

  • Cc steph@… added
  • Resolution fixed deleted
  • Severity changed from blocker to normal
  • Status changed from closed to reopened
  • Version changed from 3.1 to 3.1.3

The emails are still being sent in 3.1.3.

comment:14 ryan3 years ago

Skip confirmation seems to work for me on trunk for both "Add Existing User" and "Add New User". Note that adding a new user will still send a notification to the admin, but this is not the same as the confirmation email sent to the user being added.

comment:15 ryan3 years ago

  • Keywords reporter-feedback added; has-patch removed

comment:16 sillybean3 years ago

  • Keywords reporter-feedback removed

I just tested it again in 3.1.3 MS. (Can't upgrade this site to 3.2 just yet.)

Adding an existing user, skip email box checked: no email. All good.

Adding a new user, skip email box checked: email. I added myself under an alias, and got both the admin notification and the unwanted user email ("New WordPress MU User, your new account is set up...").

comment:17 nacin3 years ago

I can never remember the different forms or how they're processed, but I just skimmed the code and see why the email is being sent:

When checking that box, wpmu_signup_user_notification() gets blocked. But that email is coming from perhaps the welcome email -- the only string with "Your new account is set up." is that one (in 3.1 or trunk), and that isn't even a direct match.

Which form/URL are you adding the new user from?

comment:18 miklb3 years ago

The label is bit of a misnomer. An email is still sent regardless, but what it does is bypass the confirmation link that is sent. Meaning, if you check that box, the user is immediately added to the network/site, you can edit their profile immediately. Otherwise, you have to wait until they confirm.

comment:19 follow-up: sillybean3 years ago

I was adding the user in one of the subsites, from User -> Add New.

comment:20 in reply to: ↑ 19 nacin3 years ago

Replying to sillybean:

I was adding the user in one of the subsites, from User -> Add New.

There's two forms there, right? Add Existing User and Add New User. Only the first one has the box, and it only works for that.

comment:21 sillybean3 years ago

The box appears in both forms. I was adding a new user, and that's where it didn't work. There was no email sent when adding an existing user to this site.

comment:22 miklb3 years ago

I can not speak for sillybean, but my experience with other admins in that environment is that when they read "skip confirmation email", they assumed no email of any kind would be sent. Scenario would be developing a site in multi-site environment, and have a desire to create a user account to attribute content to, but not want to expose development site/URL to that user.

I've not looked into whether that function could be hooked into to also bypass a welcome message via a plugin or not.

comment:23 nacin3 years ago

if ( isset( $_POST[ 'noconfirmation' ] ) && is_super_admin() ) {
	add_filter( 'wpmu_signup_user_notification', '__return_false' ); // Disable confirmation email
}

That code should prevent wpmu_signup_user_notification() from firing off an email. I'm looking at 3.1 code for this.

comment:24 sillybean3 years ago

miklb -- right. My scenario is that our school is using central authentication (wpCAS; could be LDAP in the future). Since the WP password is never used, we don't want it sent to the users.

comment:25 nacin3 years ago

Closing as fixed. Sounds like this deserves a new ticket > 3.2.

comment:26 nacin3 years ago

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

comment:27 SergeyBiryukov3 years ago

  • Version changed from 3.1.3 to 3.1

Restoring the initial reported version.

Note: See TracTickets for help on using tickets.