Refactor password checker, add extender (#2176)

src/Extend/Auth.php

@@ -0,0 +1,71 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Extend;
+
+use Flarum\Extension\Extension;
+use Flarum\Foundation\ContainerUtil;
+use Illuminate\Contracts\Container\Container;
+
+class Auth implements ExtenderInterface
+{
+    private $addPasswordCheckers = [];
+    private $removePasswordCheckers = [];
+
+    /**
+     * Add a new password checker.
+     *
+     * @param string $identifier: Unique identifier for password checker.
+     * @param callable|string $callback: A closure or invokable class that contains the logic of the password checker.
+     *                                 It should return:
+     *                                   - `true` if the given password is valid.
+     *                                   - `null` (or not return anything) if the given password is invalid, or this checker does not apply.
+     *                                             Generally, `null` should be returned instead of `false` so that other
+     *                                             password checkers can run.
+     *                                   - `false` if the given password is invalid, and no other checkers should be considered.
+     *                                              Evaluation will be immediately halted if any checkers return `false`.
+     * @return self
+     */
+    public function addPasswordChecker(string $identifier, $callback)
+    {
+        $this->addPasswordCheckers[$identifier] = $callback;
+
+        return $this;
+    }
+
+    /**
+     * Remove a password checker.
+     *
+     * @param string $identifier: The unique identifier of the password checker to remove.
+     * @return self
+     */
+    public function removePasswordChecker(string $identifier)
+    {
+        $this->removePasswordCheckers[] = $identifier;
+
+        return $this;
+    }
+
+    public function extend(Container $container, Extension $extension = null)
+    {
+        $container->extend('flarum.user.password_checkers', function ($passwordCheckers) use ($container) {
+            foreach ($this->removePasswordCheckers as $identifier) {
+                if (array_key_exists($identifier, $passwordCheckers)) {
+                    unset($passwordCheckers[$identifier]);
+                }
+            }
+
+            foreach ($this->addPasswordCheckers as $identifier => $checker) {
+                $passwordCheckers[$identifier] = ContainerUtil::wrapCallback($checker, $container);
+            }
+
+            return $passwordCheckers;
+        });
+    }
+}

src/User/Event/CheckingPassword.php

@@ -11,6 +11,9 @@ namespace Flarum\User\Event;
 
 use Flarum\User\User;
 
+/**
+ * @deprecated beta 16, remove in beta 17. Use Auth extender instead.
+ */
 class CheckingPassword
 {
     /**

src/User/User.php

@@ -120,6 +120,13 @@ class User extends AbstractModel
      */
     protected static $gate;
 
+    /**
+     * Callbacks to check passwords.
+     *
+     * @var array
+     */
+    protected static $passwordCheckers;
+
     /**
      * Boot the model.
      *
@@ -183,6 +190,11 @@ class User extends AbstractModel
         static::$displayNameDriver = $driver;
     }
 
+    public static function setPasswordCheckers(array $checkers)
+    {
+        static::$passwordCheckers = $checkers;
+    }
+
     /**
      * Rename the user.
      *
@@ -333,11 +345,17 @@ class User extends AbstractModel
     {
         $valid = static::$dispatcher->until(new CheckingPassword($this, $password));
 
-        if ($valid !== null) {
+        foreach (static::$passwordCheckers as $checker) {
-            return $valid;
+            $result = $checker($this, $password);
+
+            if ($result === false) {
+                return false;
+            } elseif ($result === true) {
+                $valid = true;
+            }
         }
 
-        return static::$hasher->check($password, $this->password);
+        return $valid || false;
     }
 
     /**

src/User/UserServiceProvider.php

@@ -38,6 +38,7 @@ class UserServiceProvider extends AbstractServiceProvider
     {
         $this->registerAvatarsFilesystem();
         $this->registerDisplayNameDrivers();
+        $this->registerPasswordCheckers();
 
         $this->app->singleton('flarum.user.group_processors', function () {
             return [];
@@ -88,6 +89,19 @@ class UserServiceProvider extends AbstractServiceProvider
             ->give($avatarsFilesystem);
     }
 
+    protected function registerPasswordCheckers()
+    {
+        $this->app->singleton('flarum.user.password_checkers', function () {
+            return [
+                'standard' => function (User $user, $password) {
+                    if ($this->app->make('hash')->check($password, $user->password)) {
+                        return true;
+                    }
+                }
+            ];
+        });
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -97,12 +111,13 @@ class UserServiceProvider extends AbstractServiceProvider
             User::addGroupProcessor(ContainerUtil::wrapCallback($callback, $this->app));
         }
 
-        $events = $this->app->make('events');
+        User::setPasswordCheckers($this->app->make('flarum.user.password_checkers'));
-
         User::setHasher($this->app->make('hash'));
         User::setGate($this->app->makeWith(Access\Gate::class, ['policyClasses' => $this->app->make('flarum.policies')]));
         User::setDisplayNameDriver($this->app->make('flarum.user.display_name.driver'));
 
+        $events = $this->app->make('events');
+
         $events->listen(Saving::class, SelfDemotionGuard::class);
         $events->listen(Registered::class, AccountActivationMailer::class);
         $events->listen(EmailChangeRequested::class, EmailConfirmationMailer::class);

tests/integration/extenders/AuthTest.php

@@ -0,0 +1,94 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\integration\extenders;
+
+use Flarum\Extend;
+use Flarum\Tests\integration\RetrievesAuthorizedUsers;
+use Flarum\Tests\integration\TestCase;
+use Flarum\User\User;
+
+class AuthTest extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    /**
+     * @test
+     */
+    public function standard_password_works_by_default()
+    {
+        $this->app();
+
+        $user = User::find(1);
+
+        $this->assertTrue($user->checkPassword('password'));
+    }
+
+    /**
+     * @test
+     */
+    public function standard_password_can_be_disabled()
+    {
+        $this->extend(
+            (new Extend\Auth)
+                ->removePasswordChecker('standard')
+        );
+
+        $this->app();
+
+        $user = User::find(1);
+
+        $this->assertFalse($user->checkPassword('password'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_checker_can_be_added()
+    {
+        $this->extend(
+            (new Extend\Auth)
+                ->removePasswordChecker('standard')
+                ->addPasswordChecker('custom_true', CustomTrueChecker::class)
+        );
+
+        $this->app();
+
+        $user = User::find(1);
+
+        $this->assertTrue($user->checkPassword('DefinitelyNotThePassword'));
+    }
+
+    /**
+     * @test
+     */
+    public function false_checker_overrides_true()
+    {
+        $this->extend(
+            (new Extend\Auth)
+                ->addPasswordChecker('custom_false', function (User $user, $password) {
+                    return false;
+                })
+        );
+
+        $this->app();
+
+        $user = User::find(1);
+
+        $this->assertFalse($user->checkPassword('password'));
+    }
+}
+
+class CustomTrueChecker
+{
+    public function __invoke(User $user, $password)
+    {
+        return true;
+    }
+}