Handle authentication exceptions in Login module

- Break down large methods into smaller ones
- Add more authentication exception
- Add a legacy User::authenticate method

src/Model/User.php

@@ -94,56 +94,36 @@ class User
 
 
 	/**
+	 * Authenticate a user with a clear text password
+	 *
 	 * @brief Authenticate a user with a clear text password
-	 *
-	 * User info can be any of the following:
-	 * - User DB object
-	 * - User Id
-	 * - User email or username or nickname
-	 * - User array with at least the uid and the hashed password
-	 *
 	 * @param mixed $user_info
 	 * @param string $password
-	 * @return boolean
+	 * @return int|boolean
+	 * @deprecated since version 3.6
+	 * @see Friendica\Model\User::getIdFromPasswordAuthentication()
 	 */
 	public static function authenticate($user_info, $password)
 	{
-		if (is_object($user_info)) {
+		try {
-			$user = (array) $user_info;
+			return self::getIdFromPasswordAuthentication($user_info, $password);
-		} elseif (is_int($user_info)) {
+		} catch (Exception $ex) {
-			$user = dba::selectFirst('user', ['uid', 'password', 'legacy_password'],
+			return false;
-				[
+		}
-					'uid' => $user_info,
-					'blocked' => 0,
-					'account_expired' => 0,
-					'account_removed' => 0,
-					'verified' => 1
-				]
-			);
-		} elseif (is_string($user_info)) {
-			$user = dba::fetch_first('SELECT `uid`, `password`, `legacy_password`
-				FROM `user`
-				WHERE (`email` = ? OR `username` = ? OR `nickname` = ?)
-				AND `blocked` = 0
-				AND `account_expired` = 0
-				AND `account_removed` = 0
-				AND `verified` = 1
-				LIMIT 1',
-				$user_info,
-				$user_info,
-				$user_info
-			);
-		} else {
-			$user = $user_info;
 	}
 
-		if (!DBM::is_result($user)
+	/**
-			|| !isset($user['uid'])
+	 * Returns the user id associated with a successful password authentication
-			|| !isset($user['password'])
+	 *
-			|| !isset($user['legacy_password'])
+	 * @brief Authenticate a user with a clear text password
-		) {
+	 * @param mixed $user_info
-			throw new Exception('Not enough information to authenticate');
+	 * @param string $password
-		}
+	 * @return int User Id if authentication is successful
+	 * @throws Exception
+	 */
+	public static function getIdFromPasswordAuthentication($user_info, $password)
+	{
+		$user = self::getAuthenticationInfo($user_info);
 
 		if ($user['legacy_password']) {
 			if (password_verify(self::hashPasswordLegacy($password), $user['password'])) {
@@ -159,7 +139,69 @@ class User
 			return $user['uid'];
 		}
 
-		return false;
+		throw new Exception(L10n::t('Login failed'));
+	}
+
+	/**
+	 * Returns authentication info from various parameters types
+	 *
+	 * User info can be any of the following:
+	 * - User DB object
+	 * - User Id
+	 * - User email or username or nickname
+	 * - User array with at least the uid and the hashed password
+	 *
+	 * @param mixed $user_info
+	 * @return array
+	 * @throws Exception
+	 */
+	private static function getAuthenticationInfo($user_info)
+	{
+		if (is_object($user_info) || is_array($user_info)) {
+			if (is_object($user_info)) {
+				$user = (array) $user_info;
+			} else {
+				$user = $user_info;
+			}
+
+			if (!isset($user['uid'])
+				|| !isset($user['password'])
+				|| !isset($user['legacy_password'])
+			) {
+				throw new Exception(L10n::t('Not enough information to authenticate'));
+			}
+		} elseif (is_int($user_info) || is_string($user_info)) {
+			if (is_int($user_info)) {
+				$user = dba::selectFirst('user', ['uid', 'password', 'legacy_password'],
+					[
+						'uid' => $user_info,
+						'blocked' => 0,
+						'account_expired' => 0,
+						'account_removed' => 0,
+						'verified' => 1
+					]
+				);
+			} else {
+				$user = dba::fetch_first('SELECT `uid`, `password`, `legacy_password`
+					FROM `user`
+					WHERE (`email` = ? OR `username` = ? OR `nickname` = ?)
+					AND `blocked` = 0
+					AND `account_expired` = 0
+					AND `account_removed` = 0
+					AND `verified` = 1
+					LIMIT 1',
+					$user_info,
+					$user_info,
+					$user_info
+				);
+			}
+
+			if (!DBM::is_result($user)) {
+				throw new Exception(L10n::t('User not found'));
+			}
+		}
+
+		return $user;
 	}
 
 	/**

src/Module/Login.php

@@ -51,15 +51,35 @@ class Login extends BaseModule
 		session_unset();
 		// OpenId Login
 		if (
-			!x($_POST, 'password')
+			empty($_POST['password'])
 			&& (
-				x($_POST, 'openid_url')
+				!empty($_POST['openid_url'])
-				|| x($_POST, 'username')
+				|| !empty($_POST['username'])
 			)
 		) {
-			$noid = Config::get('system', 'no_openid');
+			$openid_url = trim(defaults($_POST, 'openid_url', $_POST['username']));
 
-			$openid_url = trim($_POST['openid_url'] ? : $_POST['username']);
+			self::openIdAuthentication($openid_url, !empty($_POST['remember']));
+		}
+
+		if (x($_POST, 'auth-params') && $_POST['auth-params'] === 'login') {
+			self::passwordAuthentication(
+				trim($_POST['username']),
+				trim($_POST['password']),
+				!empty($_POST['remember'])
+			);
+		}
+	}
+
+	/**
+	 * Attempts to authenticate using OpenId
+	 *
+	 * @param string $openid_url OpenID URL string
+	 * @param bool   $remember   Whether to set the session remember flag
+	 */
+	private static function openIdAuthentication($openid_url, $remember)
+	{
+		$noid = Config::get('system', 'no_openid');
 
 		// if it's an email address or doesn't resolve to a URL, fail.
 		if ($noid || strpos($openid_url, '@') || !Network::isUrlValid($openid_url)) {
@@ -73,21 +93,28 @@ class Login extends BaseModule
 			$openid = new LightOpenID;
 			$openid->identity = $openid_url;
 			$_SESSION['openid'] = $openid_url;
-				$_SESSION['remember'] = $_POST['remember'];
+			$_SESSION['remember'] = $remember;
 			$openid->returnUrl = self::getApp()->get_baseurl(true) . '/openid';
 			goaway($openid->authUrl());
 		} catch (Exception $e) {
 			notice(L10n::t('We encountered a problem while logging in with the OpenID you provided. Please check the correct spelling of the ID.') . '<br /><br >' . L10n::t('The error message was:') . ' ' . $e->getMessage());
 		}
-			// NOTREACHED
 	}
 
-		if (x($_POST, 'auth-params') && $_POST['auth-params'] === 'login') {
+	/**
+	 * Attempts to authenticate using login/password
+	 *
+	 * @param string $username User name
+	 * @param string $password Clear password
+	 * @param bool   $remember Whether to set the session remember flag
+	 */
+	private static function passwordAuthentication($username, $password, $remember)
+	{
 		$record = null;
 
 		$addon_auth = [
-				'username' => trim($_POST['username']),
+			'username' => $username,
-				'password' => trim($_POST['password']),
+			'password' => $password,
 			'authenticated' => 0,
 			'user_record' => null
 		];
@@ -99,27 +126,30 @@ class Login extends BaseModule
 		 */
 		Addon::callHooks('authenticate', $addon_auth);
 
-			if ($addon_auth['authenticated'] && count($addon_auth['user_record'])) {
+		try {
+			if ($addon_auth['authenticated']) {
 				$record = $addon_auth['user_record'];
+
+				if (empty($record)) {
+					throw new Exception(L10n::t('Login failed.'));
+				}
 			} else {
-				$user_id = User::authenticate(trim($_POST['username']), trim($_POST['password']));
+				$record = dba::selectFirst('user', [],
-				if ($user_id) {
+					['uid' => User::getIdFromPasswordAuthentication($username, $password)]
-					$record = dba::selectFirst('user', [], ['uid' => $user_id]);
+				);
 			}
+		} catch (Exception $e) {
+			logger('authenticate: failed login attempt: ' . notags($username) . ' from IP ' . $_SERVER['REMOTE_ADDR']);
+			notice($e->getMessage() . EOL);
+			goaway(self::getApp()->get_baseurl() . '/login');
 		}
 
-			if (!$record || !count($record)) {
+		if (!$remember) {
-				logger('authenticate: failed login attempt: ' . notags(trim($_POST['username'])) . ' from IP ' . $_SERVER['REMOTE_ADDR']);
-				notice(L10n::t('Login failed.') . EOL);
-				goaway(self::getApp()->get_baseurl());
-			}
-
-			if (!$_POST['remember']) {
 			new_cookie(0); // 0 means delete on browser exit
 		}
 
 		// if we haven't failed up this point, log them in.
-			$_SESSION['remember'] = $_POST['remember'];
+		$_SESSION['remember'] = $remember;
 		$_SESSION['last_login_date'] = DateTimeFormat::utcNow();
 		authenticate_success($record, true, true);
 
@@ -132,7 +162,6 @@ class Login extends BaseModule
 
 		goaway($return_url);
 	}
-	}
 
 	/**
 	 * @brief Tries to auth the user from the cookie or session