Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WordPress 4.3.1 changed wp_new_user_notification() again, conflicting s2member v150827 #732

Closed
bridgeport opened this issue Sep 18, 2015 · 15 comments
Assignees
Milestone

Comments

@bridgeport
Copy link

@bridgeport bridgeport commented Sep 18, 2015

The change in WordPress 4.3.1 wp_new_user_notification() pluggable function effectively undoes the fix in the latest s2member version, which was extensively discussed in issue #689

v150827
(s2Member/s2Member Pro) WordPress v4.3 Compat./Bug Fix This release of s2Member alters the way New User Notification Emails are sent, and in how they should be formatted in WordPress v4.3+.

The New User Notification Email is now sent (to a user) only if they did not set a Custom Password during their registration; i.e., only if they need this email to set their password for the first time. In short, s2Member now follows the same approach used by WordPress v4.3+.

Settings to reproduce:

General Options > Email Configuration > New User Email Configuration > No (default, use WordPress defaults)
General Options > Registration/Profile Fields & Options > Allow Custom Passwords During Registration > Yes

Following a checkout where a user chooses their own password, the following is emailed to the user:

Email subject:

[Site Name] Your username and password info

Email body:

Username: johndoe

To set your password, visit the following address:

https://example.com/wp-login.php?action=rp&key=zbbbtvqzynH4fgaqgy6m&login=johndoe

https://example.com/wp-login.php

If a user is setting their own password upon registration, the above obviously shouldn't be sent.

The pluggable wp_new_user_notification() function arguments changed again, as documented here:
Passwords: Deprecate second parameter of wp_new_user_notification().

Version 4.3 function with two arguments:

/**
 * Email login credentials to a newly-registered user.
 *
 * A new user registration notification is also sent to admin email.
 *
 * @since 2.0.0
 * @since 4.3.0 The `$plaintext_pass` parameter was changed to `$notify`.
 *
 * @param int    $user_id User ID.
 * @param string $notify  Whether admin and user should be notified ('both') or
 *                        only the admin ('admin' or empty).
 */
function wp_new_user_notification( $user_id, $notify = '' ) {}

Version 4.3.1 function with three arguments (middle argument deprecated):

/**
 * Email login credentials to a newly-registered user.
 *
 * A new user registration notification is also sent to admin email.
 *
 * @since 2.0.0
 * @since 4.3.0 The `$plaintext_pass` parameter was changed to `$notify`.
 * @since 4.3.1 The `$plaintext_pass` parameter was deprecated. `$notify` added as a third parameter.
 *
 * @global wpdb         $wpdb      WordPress database object for queries.
 * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance.
 *
 * @param int    $user_id    User ID.
 * @param null   $deprecated Not used (argument deprecated).
 * @param string $notify     Optional. Type of notification that should happen. Accepts 'admin' or an empty
 *                           string (admin only), or 'both' (admin and user). The empty string value was kept
 *                           for backward-compatibility purposes with the renamed parameter. Default empty.
 */
function wp_new_user_notification( $user_id, $deprecated = null, $notify = '' ) {}

I assume there's a possibility that WordPress will eventually remove the second $deprecated argument, which would reintroduce this issue, unless s2member were modified to expect the third $notify argument to always be the last argument.

Thanks.

@jaswrks

This comment has been minimized.

Copy link
Contributor

@jaswrks jaswrks commented Sep 18, 2015

Thanks for the very detailed report. Really appreciate it!

@robertkreuz

This comment has been minimized.

Copy link

@robertkreuz robertkreuz commented Sep 19, 2015

Hello, it seems the $deprecated argument is not the only issue.

For a couple of hours I checked different solutions - without luck.

I did reorder second and third argument in the pluggable.php - no succsess.
I take over old code of wp_new_user_notification from the last running backuped pluggable.php - no succsess.
I tried the code suggested in this post:
https://wordpress.org/support/topic/new-user-email-not-being-sent-2?replies=10
I tried this direct in pluggable.php and with a own php in my MUST USE-Plugin folder - no succsess.

And If S2 Member is working on that, please give back the posibility, that a wordpress generated password is send to a new user direct after registration! The new way with a link is a) not as save as before. B) It is definitly not user friendly and a mess if you want to earn money with your blog.

Let me say, that is very very annoying, if hyperactiv wordpress-developer force users into changing their stable running system. The ours of testing and bug isolation, will nobody pay.
I know thats happend all to make wordpress more safe. But the result is now:

I have to allowed New User that they can set their own password direct in the registration process and thats not more safe!

@raamdev

This comment has been minimized.

Copy link
Contributor

@raamdev raamdev commented Sep 19, 2015

I have successfully confirmed and reproduced @bridgeport's bug report as he described. Custom Passwords + WordPress defaults for New User Email Configuration = "Your username and password info" email being sent anyway with a "To set your password" link.

Tested with WordPress v4.3.1 + s2Member Pro v150827

@jaswrks jaswrks self-assigned this Sep 19, 2015
@raamdev raamdev added this to the Next Release milestone Sep 19, 2015
@robertkreuz

This comment has been minimized.

Copy link

@robertkreuz robertkreuz commented Sep 19, 2015

Thanks for working on it.

@kristineds

This comment has been minimized.

Copy link
Contributor

@kristineds kristineds commented Sep 22, 2015

@renzms

This comment has been minimized.

Copy link
Contributor

@renzms renzms commented Sep 22, 2015

@renzms

This comment has been minimized.

Copy link
Contributor

@renzms renzms commented Sep 22, 2015

@jaswrks

This comment has been minimized.

Copy link
Contributor

@jaswrks jaswrks commented Sep 23, 2015

Next Release Changelog:

  • (s2Member/s2Member Pro) WP v4.3.1 Compat. This release corrects a compatibility issue whenever you run s2Member together with WordPress v4.3.1+. Note that WordPress v4.3 made changes to the wp_new_user_notification() function in WordPress core. Then, a later release of WP v4.3.1 changed it again; breaking compatibility in both instances. This release brings s2Member up-to-date with WordPress v4.3.1 and preserves backward compatibility with WordPress v4.3, as well for versions prior. Props @bridgeport. See this GitHub issue if you'd like additional details.
@jaswrks

This comment has been minimized.

Copy link
Contributor

@jaswrks jaswrks commented Sep 23, 2015

This has been fixed in the development copy. No short-term patch available for this I'm afraid. However, we will push this out ASAP. Pending internal testing only. Thanks @bridgeport!

@jaswrks jaswrks closed this Sep 23, 2015
@renzms

This comment has been minimized.

Copy link
Contributor

@renzms renzms commented Sep 24, 2015

@raajtram

This comment has been minimized.

Copy link

@raajtram raajtram commented Oct 6, 2015

So, in v150925, will the users be allowed to set a password during registeration and then then receive that password in plain text as e-mail like before or not? Please confirm this since I'm confused whether this is fixed or now.

Thanks!

@jaswrks

This comment has been minimized.

Copy link
Contributor

@jaswrks jaswrks commented Oct 6, 2015

This was resolved in 150925, yes.

will the users be allowed to set a password during registeration and then then receive that password in plain text as e-mail like before or not?

No. If you enable Custom Passwords, the email is not sent, no matter what it contains.

@raajtram

This comment has been minimized.

Copy link

@raajtram raajtram commented Oct 6, 2015

No. If you enable Custom Passwords, the email is not sent, no matter what it contains - jaswsinc

Alright, so will the e-mail still contain a link to "Create a Password"? I'm sorry asking these questions rather than updating the plugin myself, but I want to make sure before I update.

@raamdev

This comment has been minimized.

Copy link
Contributor

@raamdev raamdev commented Oct 6, 2015

so will the e-mail still contain a link to "Create a Password"?

@raajtram If Custom Passwords are enabled (i.e., users set their password on the Pro-Form when registering), then no email to set the password is sent (i.e., the New User Email Message is not sent).

If Custom Passwords are disabled (i.e., users are not allowed to create a password on the Pro-Form when registering), then the standard WordPress email containing a link to set the password is sent after they register (or, if you've modified the New User Email Message in s2Member → General Options → Email Configuration, then that modified email will go out--just be sure to include %%wp_set_pass_url%% in your customized email so that users can set their password after registering).

So again, the behavior depends on the Custom Passwords setting. See s2Member → General Options → Registration/Profile Fields & Options → Allow Custom Passwords During Registration?.

@lucybarret

This comment has been minimized.

Copy link

@lucybarret lucybarret commented Nov 20, 2015

There are many of my clients who are complaining of issues after upgrading WordPress 4.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.