Redirect improvements for social logins

- FIX: Duplicate invalid login messages in userlogin::login()
- NEW: e_user_provider: Return URL passthrough to go back where intended
- MOD: Cleanup of some confusing APIs in e_user_provider
- MOD: Return URL passthrough in system/xup/*
- MOD: system/xup/test: Logout test renamed into something clearer

e107_core/controllers/system/xup.php

@@ -35,7 +35,7 @@ class core_system_xup_controller extends eController
 	public function init()
 	{
 		//$back = 'system/xup/test';
-		$this->backUrl = vartrue($_GET['back']) ? base64_decode($_GET['back']) : true;	
+		$this->backUrl = isset($_GET['back']) ? $_GET['back'] : null;
 	}
 	
 	public function actionSignup()
@@ -99,7 +99,7 @@ class core_system_xup_controller extends eController
 			return; 	
 		}
 		
-		if(isset($_GET['lgt']))
+		if(isset($_GET['logout']))
 		{
 			e107::getUser()->logout();
 		}
@@ -130,9 +130,18 @@ class core_system_xup_controller extends eController
 		{
 			if($var['enabled'] == 1)
 			{
+				$testLoginUrl = e107::getUrl()->create('system/xup/login', [
+					'provider' => $key,
+					'back' => $testUrl,
+				]);
+				$testSignupUrl = e107::getUrl()->create('system/xup/signup', [
+					'provider' => $key,
+					'back' => $testUrl,
+				]);
+
 				echo '<h3>'.$key.'</h3><ul>';
-				echo '<li><a class="btn btn-default btn-secondary" href="'.e107::getUrl()->create('system/xup/login?provider='.$key.'&back='.base64_encode($testUrl)).'">'.e107::getParser()->lanVars(LAN_XUP_ERRM_09, array('x'=>$key)).'</a></li>';
+				echo '<li><a class="btn btn-default btn-secondary" href="'.$testLoginUrl.'">'.e107::getParser()->lanVars(LAN_XUP_ERRM_09, array('x'=>$key)).'</a></li>';
-				echo '<li><a class="btn btn-default btn-secondary" href="'.e107::getUrl()->create('system/xup/signup?provider='.$key.'&back='.base64_encode($testUrl)).'">'.e107::getParser()->lanVars(LAN_XUP_ERRM_10, array('x'=>$key)).'</a></li>';
+				echo '<li><a class="btn btn-default btn-secondary" href="'.$testSignupUrl.'">'.e107::getParser()->lanVars(LAN_XUP_ERRM_10, array('x'=>$key)).'</a></li>';
 			
 				echo "</ul>";
 			}
@@ -140,7 +149,7 @@ class core_system_xup_controller extends eController
 		//	print_a($var);
 		}
 		
-			echo '<br /><br /><a class="btn btn-default btn-secondary" href="'.e107::getUrl()->create('system/xup/test?lgt').'">'.LAN_XUP_ERRM_12.'</a>';
+			echo '<br /><br /><a class="btn btn-default btn-secondary" href="'.e107::getUrl()->create('system/xup/test?logout=true').'">'.LAN_XUP_ERRM_12.'</a>';
 		
 		/*
 		echo '<h3>Facebook</h3>';

e107_handlers/login.php

@@ -174,13 +174,13 @@ class userlogin
 		{
 			if (!$this->lookupUser($username, $forceLogin))
 			{
-				return $this->invalidLogin($username,LOGIN_BAD_USERNAME);		// User doesn't exist
+				return false;		// User doesn't exist
 			}
 		}
 
 		if ($authorized !== true && $this->checkUserPassword($username, $userpass, $response, $forceLogin) !== true)
 		{
-			return $this->invalidLogin($username,LOGIN_BAD_PW);
+			return false;
 		}
 
 

e107_handlers/user_handler.php

@@ -1120,11 +1120,7 @@ class e_user_provider
 		else
 		{
 			$this->_config = array(
-				"callback" => e107::getUrl()->create(
+				"callback" => $this->generateCallbackUrl($provider),
-					'system/xup/login',
-					array('provider' => $provider),
-					array('full' => true, 'encode' => false)
-				),
 				"providers" => $this->social_login_config_manager->getValidConfiguredProviderConfigs(),
 				"debug_mode" => 'error',
 				"debug_file" => e_LOG . "hybridAuth.log"
@@ -1132,8 +1128,19 @@ class e_user_provider
 
 		}
 
-		$this->hybridauth = new Hybridauth\Hybridauth($this->_config);
+		$this->respawnHybridauth();
 		$this->setProvider($provider);
+
+		$providerId = $this->getProvider();
+		if ($providerId && $this->hybridauth->isConnectedWith($providerId))
+		{
+			$this->adapter = $this->hybridauth->getAdapter($providerId);
+		}
+	}
+
+	private function respawnHybridauth()
+	{
+		$this->hybridauth = new Hybridauth\Hybridauth($this->_config);
 	}
 
 	public function setProvider($provider)
@@ -1147,10 +1154,11 @@ class e_user_provider
 	}
 
 
-	public function setBackUrl($url)
+	public function setBackUrl($url, $action)
 	{
 		# system/xup/login by default
-		$this->_config['callback'] = $url;
+		$this->_config['callback'] = $this->generateCallbackUrl($this->getProvider(), $action, $url);
+		$this->respawnHybridauth();
 	}
 
 	public function getProvider()
@@ -1291,6 +1299,8 @@ class e_user_provider
 			//	throw new Exception( "Signup failed! User already signed in. ", 1); // TODO lan
 		}
 
+		$this->setBackUrl($redirectUrl, "signup");
+
 		$this->adapter = $this->hybridauth->authenticate($this->getProvider());
 		$profile = $this->adapter->getUserProfile();
 
@@ -1424,7 +1434,7 @@ class e_user_provider
 
 			if ($redirectUrl)
 			{
-				e107::getRedirect()->redirect($redirectUrl);
+				$this->redirectAndForwardMessages($redirectUrl);
 			}
 
 			return true;
@@ -1441,7 +1451,7 @@ class e_user_provider
 
 		if (!e107::getPref('social_login_active', false))
 		{
-			throw new Exception("Signup failed! This feature is disabled.", 100); // TODO lan
+			throw new Exception("Login failed! This feature is disabled.", 100); // TODO lan
 		}
 
 		if (!$this->getProvider())
@@ -1471,37 +1481,23 @@ class e_user_provider
 			return true;
 		}
 
+		$this->setBackUrl($redirectUrl, "login");
+
 		$this->adapter = $this->hybridauth->authenticate($this->getProvider());
 		$check = e107::getUser()->setProvider($this)->loginProvider($this->userId());
 
 
 		if ($redirectUrl)
 		{
-			e107::getRedirect()->redirect($redirectUrl);
+			$this->redirectAndForwardMessages($redirectUrl);
 		}
 
 		return $check;
 	}
 
-
-	public function init()
-	{
-		if (!e107::getPref('social_login_active', false))
-		{
-			return;
-		}
-		$this->adapter = null;
-		$providerId = $this->_provider;
-		if ($providerId && $this->hybridauth->isConnectedWith($providerId))
-		{
-			$this->adapter = $this->hybridauth->getAdapter($providerId);
-		}
-	}
-
 	public function logout()
 	{
 		if (
-			!e107::getPref('social_login_active', false) ||
 			!$this->adapter ||
 			!$this->hybridauth->isConnectedWith($this->getProvider())
 		) return true;
@@ -1517,6 +1513,36 @@ class e_user_provider
 		return true;
 	}
 
+	/**
+	 * @param $provider
+	 * @param string $xupAction
+	 * @param string $backUrl
+	 * @return string
+	 */
+	private function generateCallbackUrl($provider, $xupAction = "login", $backUrl = null)
+	{
+		return e107::getUrl()->create(
+			"system/xup/$xupAction",
+			array(
+				'provider' => $provider,
+				'back' => $backUrl,
+				),
+			array('full' => true, 'encode' => false)
+		);
+	}
+
+	/**
+	 * @param $redirectUrl
+	 */
+	private function redirectAndForwardMessages($redirectUrl)
+	{
+		$messages = e107::getMessage()->getAll('default', true, false);
+		foreach ($messages as $type => $message_stack)
+		{
+			e107::getMessage()->addSessionStack($message_stack, 'default', $type);
+		}
+		e107::getRedirect()->redirect($redirectUrl);
+	}
 }
 
 

e107_handlers/user_model.php

@@ -1542,7 +1542,6 @@ class e_user extends e_user_model
 		{
 			$providerId = $this->getProviderName();
 			$this->_provider = e107::getUserProvider($providerId);
-			$this->_provider->init();
 		}
 
 		return $this;