Move the 'authenticate' hook deeper into the authentication flow so that password...

diff --git a/doc/Addons.md b/doc/Addons.md
index be9dd421894ea9eadf32b3205281ee5f6bc42cb5..a058753605a1533b18d9cd68a7599ccf6d0a7610 100644 (file)
--- a/doc/Addons.md
+++ b/doc/Addons.md
@@ -31,7 +31,7 @@ Here's the structure:
  * Status: {Unsupported|Arbitrary status}
  */
 ```
- 
+
 You can also provide a longer documentation in a `README` or `README.md` file.
 The latter will be converted from Markdown to HTML in the addon detail page.
 
@@ -447,7 +447,7 @@ Form field array structure is:
 - **field**: Standard field data structure to be used by `field_checkbox.tpl` and `field_select.tpl`.
 
 For `checkbox`, **field** is:
-  - [0] (String): Form field name; Mandatory. 
+  - [0] (String): Form field name; Mandatory.
   - [1]: (String): Form field label; Optional, default is none.
   - [2]: (Boolean): Whether the checkbox should be checked by default; Optional, default is false.
   - [3]: (String): Additional help text; Optional, default is none.
@@ -458,7 +458,7 @@ For `select`, **field** is:
   - [1] (String): Form field label; Optional, default is none.
   - [2] (Boolean): Default value to be selected by default; Optional, default is none.
   - [3] (String): Additional help text; Optional, default is none.
-  - [4] (Array): Associative array of options. Item key is option value, item value is option label; Mandatory. 
+  - [4] (Array): Associative array of options. Item key is option value, item value is option label; Mandatory.
 
 ### route_collection
 Called just before dispatching the router.
@@ -470,7 +470,7 @@ Hook data is a `\FastRoute\RouterCollector` object that should be used to add ad
 
 Called before trying to detect the target network of a URL.
 If any registered hook function sets the `result` key of the hook data array, it will be returned immediately.
-Hook functions should also return immediately if the hook data contains an existing result. 
+Hook functions should also return immediately if the hook data contains an existing result.
 
 Hook data:
 
@@ -666,8 +666,13 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep-
     Hook::callAll('event_updated', $event['id']);
     Hook::callAll("event_created", $event['id']);
 
+### src/Model/Register.php
+
+    Hook::callAll('authenticate', $addon_auth);
+
 ### src/Model/User.php
 
+    Hook::callAll('authenticate', $addon_auth);
     Hook::callAll('register_account', $uid);
     Hook::callAll('remove_user', $user);
 
@@ -675,6 +680,22 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep-
 
     Hook::callAll('lockview_content', $item);
 
+### src/Module/Settings/Delegation.php
+
+    Hook::callAll('authenticate', $addon_auth);
+
+### src/Module/Settings/TwoFactor/Index.php
+
+    Hook::callAll('authenticate', $addon_auth);
+
+### src/Security/Authenticate.php
+
+    Hook::callAll('authenticate', $addon_auth);
+
+### src/Security/ExAuth.php
+
+    Hook::callAll('authenticate', $addon_auth);
+
 ### src/Content/ContactBlock.php
 
     Hook::callAll('contact_block_end', $arr);
@@ -713,7 +734,7 @@ Here is a complete list of all hook callbacks with file locations (as of 24-Sep-
 ### src/Core/Authentication.php
 
     Hook::callAll('logged_in', $a->user);
-    
+
 ### src/Core/StorageManager
 
     Hook::callAll('storage_instance', $data);

diff --git a/src/Model/User.php b/src/Model/User.php
index 4959c8c21607c66f799d9449b457084ce29889d2..3b11ee6ce8b273607878988533edaa32af951d64 100644 (file)
--- a/src/Model/User.php
+++ b/src/Model/User.php
@@ -544,6 +544,24 @@ class User
                        }
 
                        return $user['uid'];
+               } else {
+                       $addon_auth = [
+                               'username'      => $user['nickname'],
+                               '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 $user['uid'];
+                       }
                }
 
                throw new HTTPException\ForbiddenException(DI::l10n()->t('Login failed'));
@@ -584,7 +602,7 @@ class User
                        if (is_int($user_info)) {
                                $user = DBA::selectFirst(
                                        'user',
-                                       ['uid', 'password', 'legacy_password'],
+                                       ['uid', 'nickname', 'password', 'legacy_password'],
                                        [
                                                'uid' => $user_info,
                                                'blocked' => 0,
@@ -594,7 +612,7 @@ class User
                                        ]
                                );
                        } else {
-                               $fields = ['uid', 'password', 'legacy_password'];
+                               $fields = ['uid', 'nickname', 'password', 'legacy_password'];
                                $condition = [
                                        "(`email` = ? OR `username` = ? OR `nickname` = ?)
                                        AND NOT `blocked` AND NOT `account_expired` AND NOT `account_removed` AND `verified`",

diff --git a/src/Security/Authentication.php b/src/Security/Authentication.php
index eab75ba5d0cea145f3e68dbba1d50d9e5ff0578d..61553907fa9ff9b349aa59949eb68af0312033ac 100644 (file)
--- a/src/Security/Authentication.php
+++ b/src/Security/Authentication.php
@@ -239,34 +239,12 @@ class Authentication
        {
                $record = null;
 
-               $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);
-
                try {
-                       if ($addon_auth['authenticated']) {
-                               $record = $addon_auth['user_record'];
-
-                               if (empty($record)) {
-                                       throw new Exception($this->l10n->t('Login failed.'));
-                               }
-                       } else {
-                               $record = $this->dba->selectFirst(
-                                       'user',
-                                       [],
-                                       ['uid' => User::getIdFromPasswordAuthentication($username, $password)]
-                               );
-                       }
+                       $record = $this->dba->selectFirst(
+                               'user',
+                               [],
+                               ['uid' => User::getIdFromPasswordAuthentication($username, $password)]
+                       );
                } catch (Exception $e) {
                        $this->logger->warning('authenticate: failed login attempt', ['action' => 'login', 'username' => Strings::escapeTags($username), 'ip' => $_SERVER['REMOTE_ADDR']]);
                        notice($this->l10n->t('Login failed. Please check your credentials.'));