s2Member code problem causes conflict

Please refer to this thread – http://wordpress.org/support/topic/experiencing-an-error?replies=28#post-4278240
Description of problem:

• Ajax popup form appears.
• User clicks login.
• Modal Login calls wp_signon with the POSTed data.
• Modal Login expects to retrieve a WP_User object in return.

Meanwhile….

• s2Member adds an action to wp_login – their redirect logic.
• The wp_login hook is fired by Modal Login’s wp_signon call.
• s2Member performs it’s wp_redirect (the success of which can be seen by watching the Network panel of Chrome’s developer toolbar).

Back to Modal Login…
Modal Login expects to retrieve the WP_User object, but by this time, we’ve already moved on to the redirected page. So No WP_User for Modal Login. Hence, Modal Login just sits.

For a fix…I believe it is on the end of s2Member – maybe using the login_redirect filter INSTEAD of the wp_login hook.

Thank you for reporting this important issue.

I’ll send this to our development team for their thoughts.

I know it’s been over a weekend, but I believe this to be a significant issue with the way s2Member is set up. Is there any new information on this?

I have made a few changes to s2Member to fit what I believe is the most appropriate place to redirect a logged in user. The first step was changing the logic from hooking into wp_login to filtering the login_redirect. Since s2Member removes all filters on login_redirect, I also had to modify login-redirects-r.inc.php to add the filter back after all others are removed.

The altered login_redirect function in login-redirects.inc.php:

public static function login_redirect ($redirect,$request,$user)
					{
						foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
						do_action ("ws_plugin__s2member_before_login_redirect", get_defined_vars ());
						unset /* Unset defined __refs, __v. */ ($__refs, $__v);
						$username = $user->user_login;
						if (is_string($username) && $username && is_object ($user) && !empty ($user->ID) && ($user_id = $user->ID))
							{
								update_user_option ($user_id, "s2member_last_login_time", time());

								if /* Have we got this yet? */ (!get_user_option ("s2member_registration_ip", $user_id))
									update_user_option ($user_id, "s2member_registration_ip", $_SERVER["REMOTE_ADDR"]);

								if (($logins = (int)get_user_option ("s2member_login_counter", $user_id) + 1) >= 1 || ($logins = 1))
									update_user_option ($user_id, "s2member_login_counter", $logins);

								if /* Nag em? */ ($GLOBALS["WS_PLUGIN__"]["s2member"]["o"]["custom_reg_password"])
									delete_user_setting ("default_password_nag") . update_user_option ($user_id, "default_password_nag", false, true);

								$disable_login_ip_restrictions = apply_filters ("ws_plugin__s2member_disable_login_ip_restrictions", false, get_defined_vars ());

								if (($ok = true) && !is_super_admin ($user_id) && $username !== "demo" && !$disable_login_ip_restrictions)
									$ok = c_ws_plugin__s2member_ip_restrictions::ip_restrictions_ok ($_SERVER["REMOTE_ADDR"], $username);

								if (($redirect = apply_filters ("ws_plugin__s2member_login_redirect", (($user->has_cap ("edit_posts")) ? false : true), get_defined_vars ())))
									{
										$obey_redirect_to = apply_filters ("ws_plugin__s2member_obey_login_redirect_to", /* By default, we obey this. */ true, get_defined_vars ());

										if (!$obey_redirect_to || empty ($_REQUEST["redirect_to"]) || !is_string ($_REQUEST["redirect_to"]) || $_REQUEST["redirect_to"] === admin_url () || preg_match ("/^\/?wp-admin\/?$/", $_REQUEST["redirect_to"]))
											{
												foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
												do_action ("ws_plugin__s2member_during_login_redirect", get_defined_vars ());
												unset /* Unset defined __refs, __v. */ ($__refs, $__v);

												if /* Is this a string? */ ($redirect && is_string ($redirect))
													return /* Dynamic URL introduced by a Filter? */ ($redirect);

												else if ($redirection_url = c_ws_plugin__s2member_login_redirects::login_redirection_url ($user))
													return /* Special Redirection URL configured with s2Member. */ ($redirection_url);

												else // Else we use the Login Welcome Page configured for s2Member.
													return (get_page_link ($GLOBALS["WS_PLUGIN__"]["s2member"]["o"]["login_welcome_page"]));

												exit /* Clean exit. */ ();
											}
									}
							}

						foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
						do_action ("ws_plugin__s2member_after_login_redirect", get_defined_vars ());
						unset /* Unset defined __refs, __v. */ ($__refs, $__v);

						return /* Return for uniformity. */;
					}

My altered filter in hooks.php (replaces the add_action(‘wp_login’)):

add_filter("login_redirect", "c_ws_plugin__s2member_login_redirects::login_redirect", 20, 3);

My altered remove_login_redirect_filters in login-redirects-r.inc.php:

public static function remove_login_redirect_filters ()
					{
						//if(defined ("DOING_AJAX") || DOING_AJAX) return;
						do_action ("ws_plugin__s2member_before_remove_login_redirect_filters", get_defined_vars ());

						if (!apply_filters ("ws_plugin__s2member_allow_other_login_redirect_filters", false, get_defined_vars ()))
							{
								remove_all_filters /* Removes all `login_redirect` Filters. */("login_redirect");
								add_filter ("login_redirect", "c_ws_plugin__s2member_login_redirects_r::_empty_login_redirect_filter");
								add_filter("login_redirect", "c_ws_plugin__s2member_login_redirects::login_redirect", 20, 3);

								do_action ("ws_plugin__s2member_during_remove_login_redirect_filters", get_defined_vars ());
							}

						do_action ("ws_plugin__s2member_after_remove_login_redirect_filters", get_defined_vars ());

						return /* Return for uniformity. */;
					}

I realize that filtering the login_redirect twice may be redundant, but I will leave that decision up to s2Member’s team.

The summary of my changes are:

1. Changing the use of wp_login hook to login_redirect filter.
2. Altering the login_redirect function of s2Member to return the url instead of redirecting to it.
3. Re-adding the filter after all filters are removed by remove_login_redirect_filters.

Thanks for your patience.

I contacted Jason about this. Here is his reply:

---

Unfortunately this is a plugin conflict that is not easy to resolve. s2Member MUST attach itself to the wp_signon hook and NOT to the login_redirect filter, because it needs to make sure it is tracking ALL attempts to log into the site; so it can properly prevent Brute Force attacks, track IP addresses, and maintain its login counters. If we attached only to the login_redirect filter, it would not provide enough protection.

One possible solution is for the other plugin to add a filter as follows.

I understand the logic you present about needing to track all logins. However, I still disagree with the need to use the wp_signon hook for the redirection. If you need to track user logins, why not hook the wp_signon to perform user login tracking, then perform your redirect logic separately? I feel like performing user tracking, brute force, and ip logging logic should not lie in a function called login_redirect. I suggest something like:

public static function login_redirect ($redirect,$request,$user)
{
	foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
	do_action ("ws_plugin__s2member_before_login_redirect", get_defined_vars ());
	unset /* Unset defined __refs, __v. */ ($__refs, $__v);
	$username = $user->user_login;
	if (is_string($username) && $username && is_object ($user) && !empty ($user->ID) && ($user_id = $user->ID))
		{

			if (($redirect = apply_filters ("ws_plugin__s2member_login_redirect", (($user->has_cap ("edit_posts")) ? false : true), get_defined_vars ())))
				{
					$obey_redirect_to = apply_filters ("ws_plugin__s2member_obey_login_redirect_to", /* By default, we obey this. */ true, get_defined_vars ());

					if (!$obey_redirect_to || empty ($_REQUEST["redirect_to"]) || !is_string ($_REQUEST["redirect_to"]) || $_REQUEST["redirect_to"] === admin_url () || preg_match ("/^\/?wp-admin\/?$/", $_REQUEST["redirect_to"]))
						{
							foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
							do_action ("ws_plugin__s2member_during_login_redirect", get_defined_vars ());
							unset /* Unset defined __refs, __v. */ ($__refs, $__v);

							if /* Is this a string? */ ($redirect && is_string ($redirect))
								return /* Dynamic URL introduced by a Filter? */ ($redirect);

							else if ($redirection_url = c_ws_plugin__s2member_login_redirects::login_redirection_url ($user))
								return /* Special Redirection URL configured with s2Member. */ ($redirection_url);

							else // Else we use the Login Welcome Page configured for s2Member.
								return (get_page_link ($GLOBALS["WS_PLUGIN__"]["s2member"]["o"]["login_welcome_page"]));

							exit /* Clean exit. */ ();
						}
				}
		}

	foreach(array_keys(get_defined_vars())as$__v)$__refs[$__v]=&$$__v;
	do_action ("ws_plugin__s2member_after_login_redirect", get_defined_vars ());
	unset /* Unset defined __refs, __v. */ ($__refs, $__v);

	return $redirect /* Return for uniformity. */;
}

public static function user_login_tracking($username,$user){
	do_action ("ws_plugin__s2member_before_user_login_tracking", get_defined_vars ());
	if (is_string($username) && $username && is_object ($user) && !empty ($user->ID) && ($user_id = $user->ID))
		{
			update_user_option ($user_id, "s2member_last_login_time", time());

			if /* Have we got this yet? */ (!get_user_option ("s2member_registration_ip", $user_id))
				update_user_option ($user_id, "s2member_registration_ip", $_SERVER["REMOTE_ADDR"]);

			if (($logins = (int)get_user_option ("s2member_login_counter", $user_id) + 1) >= 1 || ($logins = 1))
				update_user_option ($user_id, "s2member_login_counter", $logins);

			if /* Nag em? */ ($GLOBALS["WS_PLUGIN__"]["s2member"]["o"]["custom_reg_password"])
				delete_user_setting ("default_password_nag") . update_user_option ($user_id, "default_password_nag", false, true);

			$disable_login_ip_restrictions = apply_filters ("ws_plugin__s2member_disable_login_ip_restrictions", false, get_defined_vars ());

			if (($ok = true) && !is_super_admin ($user_id) && $username !== "demo" && !$disable_login_ip_restrictions)
				$ok = c_ws_plugin__s2member_ip_restrictions::ip_restrictions_ok ($_SERVER["REMOTE_ADDR"], $username);
		}
	do_action ("ws_plugin__s2member_after_user_login_tracking", get_defined_vars ());
	return $user;
}

Thanks for the information. If you’d like you can use your “patched” file in your installation for now by taking your edited copy of this file and loading it in as a Must-Use Plugin. I’ll send this code to Jason for him to look over and consider for a future release of s2Member, but for now you’ll have to have the plugin’s developer use the information from my last reply. Thanks.