Apply suggestions from code review

Also clean up some code, make it less needlessly verbose.

Co-authored-by: Hypolite Petovan <hypolite@mrpetovan.com>
This commit is contained in:
very-ape 2021-05-20 11:05:48 -07:00 committed by very-ape
parent c89241dbd8
commit d66f1e30ae

View file

@ -523,19 +523,18 @@ class User
try {
$user = self::getAuthenticationInfo($user_info);
} catch (Exception $e) {
// Addons can create users, and creating a numeric username would create
// abiguity with user IDs, possibly opening up an attack vector.
// So let's be very careful about that.
if (is_numeric($user_info) || is_numeric($user_info['nickname'] ?? '')) {
throw $e;
}
$username = (is_string($user_info) ? $user_info : $user_info['nickname'] ?? '');
if (!$username) {
// Addons can create users, and since this 'catch' branch should only
// execute if getAuthenticationInfo can't find an existing user, that's
// exactly what will happen here. Creating a numeric username would create
// abiguity with user IDs, possibly opening up an attack vector.
// So let's be very careful about that.
if (empty($username) || is_numeric($user_info) || is_numeric($user_info['nickname'] ?? '')) {
throw $e;
}
return self::getIdFromAuthenticateHooks($user_info, $password);
return self::getIdFromAuthenticateHooks($username, $password);
}
if ($third_party && DI::pConfig()->get($user['uid'], '2fa', 'verified')) {
@ -582,7 +581,8 @@ class User
* @return int User Id if authentication is successful
* @throws HTTPException\ForbiddenException
*/
public static function getIdFromAuthenticateHooks($username, $password) {
public static function getIdFromAuthenticateHooks($username, $password)
{
$addon_auth = [
'username' => $username,
'password' => $password,