Merge pull request #10344 from nupplaphil/bug/friendica-10342

[friendica.git] / src / Model / User.php

diff --git a/src/Model/User.php b/src/Model/User.php
index 3b11ee6ce8b273607878988533edaa32af951d64..029613b17b0b692e19e3301fcd6d2b841d54640e 100644 (file)
--- a/src/Model/User.php
+++ b/src/Model/User.php
@@ -312,8 +312,8 @@ class User
         */
        public static function getIdForURL(string $url)
        {
-               // Avoid any database requests when the hostname isn't even part of the url.
-               if (!strpos($url, DI::baseUrl()->getHostname())) {
+               // Avoid database queries when the local node hostname isn't even part of the url.
+               if (!Contact::isLocal($url)) {
                        return 0;
                }
 
@@ -515,7 +515,27 @@ class User
         */
        public static function getIdFromPasswordAuthentication($user_info, $password, $third_party = false)
        {
-               $user = self::getAuthenticationInfo($user_info);
+               // Addons registered with the "authenticate" hook may create the user on the
+               // fly. `getAuthenticationInfo` will fail if the user doesn't exist yet. If
+               // the user doesn't exist, we should give the addons a chance to create the
+               // user in our database, if applicable, before re-throwing the exception if
+               // they fail.
+               try {
+                       $user = self::getAuthenticationInfo($user_info);
+               } catch (Exception $e) {
+                       $username = (is_string($user_info) ? $user_info : $user_info['nickname'] ?? '');
+
+                       // 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($username)) {
+                               throw $e;
+                       }
+
+                       return self::getIdFromAuthenticateHooks($username, $password);
+               }
 
                if ($third_party && DI::pConfig()->get($user['uid'], '2fa', 'verified')) {
                        // Third-party apps can't verify two-factor authentication, we use app-specific passwords instead
@@ -545,23 +565,40 @@ class User
 
                        return $user['uid'];
                } else {
-                       $addon_auth = [
-                               'username'      => $user['nickname'],
-                               'password'      => $password,
-                               'authenticated' => 0,
-                               'user_record'   => null
-                       ];
+                       return self::getIdFromAuthenticateHooks($user['nickname'], $password); // throws
+               }
 
-                       /*
-                        * An addon indicates successful login by setting 'authenticated' to non-zero value and returning a user record
-                        * Addons should never set 'authenticated' except to indicate success - as hooks may be chained
-                        * and later addons should not interfere with an earlier one that succeeded.
-                        */
-                       Hook::callAll('authenticate', $addon_auth);
+               throw new HTTPException\ForbiddenException(DI::l10n()->t('Login failed'));
+       }
 
-                       if ($addon_auth['authenticated'] && $addon_auth['user_record']) {
-                               return $user['uid'];
-                       }
+       /**
+        * Try to obtain a user ID via "authenticate" hook addons
+        *
+        * Returns the user id associated with a successful password authentication
+        *
+        * @param string $username
+        * @param string $password
+        * @return int User Id if authentication is successful
+        * @throws HTTPException\ForbiddenException
+        */
+       public static function getIdFromAuthenticateHooks($username, $password)
+       {
+               $addon_auth = [
+                       'username'      => $username,
+                       'password'      => $password,
+                       'authenticated' => 0,
+                       'user_record'   => null
+               ];
+
+               /*
+                * An addon indicates successful login by setting 'authenticated' to non-zero value and returning a user record
+                * Addons should never set 'authenticated' except to indicate success - as hooks may be chained
+                * and later addons should not interfere with an earlier one that succeeded.
+                */
+               Hook::callAll('authenticate', $addon_auth);
+
+               if ($addon_auth['authenticated'] && $addon_auth['user_record']) {
+                       return $addon_auth['user_record']['uid'];
                }
 
                throw new HTTPException\ForbiddenException(DI::l10n()->t('Login failed'));
@@ -1086,6 +1123,8 @@ class User
                                        Photo::update(['profile' => 1], ['resource-id' => $resource_id]);
                                }
                        }
+
+                       Contact::updateSelfFromUserID($uid, true);
                }
 
                Hook::callAll('register_account', $uid);