Merge pull request #593 from getformwork/refactor/user-authentication

Refactor user authentication
getformwork · Oct 25, 2024 · b99d313 · b99d313
2 parents f4da7ed + 4aeeeca
commit b99d313
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 62 deletions.
diff --git a/formwork/src/Panel/Controllers/AuthenticationController.php b/formwork/src/Panel/Controllers/AuthenticationController.php
@@ -8,16 +8,24 @@
 use Formwork\Log\Log;
 use Formwork\Log\Registry;
 use Formwork\Panel\Security\AccessLimiter;
+use Formwork\Users\Exceptions\AuthenticationFailedException;
+use Formwork\Users\Exceptions\UserNotLoggedException;
+use Formwork\Users\User;
 use Formwork\Utils\FileSystem;
-use RuntimeException;
 
 class AuthenticationController extends AbstractController
 {
+    public const SESSION_REDIRECT_KEY = '_formwork_redirect_to';
+
     /**
      * Authentication@login action
      */
     public function login(AccessLimiter $accessLimiter): Response
     {
+        if ($this->panel()->isLoggedIn()) {
+            return $this->redirect($this->generateRoute('panel.index'));
+        }
+
         $csrfTokenName = $this->panel()->getCsrfTokenName();
 
         if ($accessLimiter->hasReachedLimit()) {
@@ -26,82 +34,86 @@ public function login(AccessLimiter $accessLimiter): Response
             return $this->error($this->translate('panel.login.attempt.tooMany', $minutes));
         }
 
-        switch ($this->request->method()) {
-            case RequestMethod::GET:
-                if ($this->request->session()->has('FORMWORK_USERNAME')) {
-                    return $this->redirect($this->generateRoute('panel.index'));
-                }
+        if ($this->request->method() === RequestMethod::POST) {
+            // Delay request processing for 0.5-1s
+            usleep(random_int(500, 1000) * 1000);
 
-                // Always generate a new CSRF token
-                $this->csrfToken->generate($csrfTokenName);
+            $data = $this->request->input();
 
-                return new Response($this->view('authentication.login', [
-                    'title' => $this->translate('panel.login.login'),
-                ]));
+            // Ensure no required data is missing
+            if (!$data->hasMultiple(['username', 'password'])) {
+                $this->csrfToken->generate($csrfTokenName);
+                $this->error($this->translate('panel.login.attempt.failed'));
+            }
 
-            case RequestMethod::POST:
-                // Delay request processing for 0.5-1s
-                usleep(random_int(500, 1000) * 1000);
+            $accessLimiter->registerAttempt();
 
-                $data = $this->request->input();
+            $username = $data->get('username');
 
-                // Ensure no required data is missing
-                if (!$data->hasMultiple(['username', 'password'])) {
-                    $this->csrfToken->generate($csrfTokenName);
-                    $this->error($this->translate('panel.login.attempt.failed'));
-                }
+            /** @var User */
+            $user = $this->site->users()->get($username);
 
-                $accessLimiter->registerAttempt();
-
-                $user = $this->site->users()->get($data->get('username'));
-
-                // Authenticate user
-                if ($user !== null && $user->authenticate($data->get('password'))) {
-                    $this->request->session()->regenerate();
-                    $this->request->session()->set('FORMWORK_USERNAME', $data->get('username'));
+            // Authenticate user
+            if ($user !== null) {
+                try {
+                    $user->authenticate($data->get('password'));
 
                     // Regenerate CSRF token
                     $this->csrfToken->generate($csrfTokenName);
 
                     $accessLog = new Log(FileSystem::joinPaths($this->config->get('system.panel.paths.logs'), 'access.json'));
                     $lastAccessRegistry = new Registry(FileSystem::joinPaths($this->config->get('system.panel.paths.logs'), 'lastAccess.json'));
 
-                    $time = $accessLog->log($data->get('username'));
-                    $lastAccessRegistry->set($data->get('username'), $time);
+                    $time = $accessLog->log($username);
+                    $lastAccessRegistry->set($username, $time);
 
                     $accessLimiter->resetAttempts();
 
-                    if (($destination = $this->request->session()->get('FORMWORK_REDIRECT_TO')) !== null) {
-                        $this->request->session()->remove('FORMWORK_REDIRECT_TO');
+                    if (($destination = $this->request->session()->get(self::SESSION_REDIRECT_KEY)) !== null) {
+                        $this->request->session()->remove(self::SESSION_REDIRECT_KEY);
                         return new RedirectResponse($this->panel->uri($destination));
                     }
 
                     return $this->redirect($this->generateRoute('panel.index'));
+                } catch (AuthenticationFailedException) {
+                    // Do nothing, the error response will be sent below
                 }
+            }
 
-                $this->csrfToken->generate($csrfTokenName);
-                return $this->error($this->translate('panel.login.attempt.failed'), [
-                    'username' => $data->get('username'),
-                    'error'    => true,
-                ]);
+            $this->csrfToken->generate($csrfTokenName);
+
+            return $this->error($this->translate('panel.login.attempt.failed'), [
+                'username' => $username,
+                'error'    => true,
+            ]);
         }
 
-        throw new RuntimeException('Invalid Method');
+        // Always generate a new CSRF token
+        $this->csrfToken->generate($csrfTokenName);
+
+        return new Response($this->view('authentication.login', [
+            'title' => $this->translate('panel.login.login'),
+        ]));
     }
 
     /**
      * Authentication@logout action
      */
     public function logout(): RedirectResponse
     {
-        $this->csrfToken->destroy($this->panel()->getCsrfTokenName());
-        $this->request->session()->remove('FORMWORK_USERNAME');
-        $this->request->session()->destroy();
+        try {
+            $this->panel->user()->logout();
+            $this->csrfToken->destroy($this->panel()->getCsrfTokenName());
 
-        if ($this->config->get('system.panel.logoutRedirect') === 'home') {
-            return $this->redirect('/');
+            if ($this->config->get('system.panel.logoutRedirect') === 'home') {
+                return $this->redirect('/');
+            }
+
+            $this->panel()->notify($this->translate('panel.login.loggedOut'), 'info');
+        } catch (UserNotLoggedException) {
+            // Do nothing if user is not logged, the user will be redirected to the login page
         }
-        $this->panel()->notify($this->translate('panel.login.loggedOut'), 'info');
+
         return $this->redirect($this->generateRoute('panel.index'));
     }
 

diff --git a/formwork/src/Panel/Controllers/RegisterController.php b/formwork/src/Panel/Controllers/RegisterController.php
@@ -10,6 +10,7 @@
 use Formwork\Panel\Security\Password;
 use Formwork\Parsers\Yaml;
 use Formwork\Schemes\Schemes;
+use Formwork\Users\User;
 use Formwork\Utils\FileSystem;
 use RuntimeException;
 
@@ -57,7 +58,7 @@ public function register(Schemes $schemes): Response
                 Yaml::encodeToFile($userData, FileSystem::joinPaths($this->config->get('system.users.paths.accounts'), $username . '.yaml'));
 
                 $this->request->session()->regenerate();
-                $this->request->session()->set('FORMWORK_USERNAME', $username);
+                $this->request->session()->set(User::SESSION_LOGGED_USER_KEY, $username);
 
                 $accessLog = new Log(FileSystem::joinPaths($this->config->get('system.panel.paths.logs'), 'access.json'));
                 $lastAccessRegistry = new Registry(FileSystem::joinPaths($this->config->get('system.panel.paths.logs'), 'lastAccess.json'));

diff --git a/formwork/src/Panel/Panel.php b/formwork/src/Panel/Panel.php
@@ -8,6 +8,7 @@
 use Formwork\Http\Session\MessageType;
 use Formwork\Languages\LanguageCodes;
 use Formwork\Users\ColorScheme;
+use Formwork\Users\Exceptions\UserNotLoggedException;
 use Formwork\Users\User;
 use Formwork\Users\Users;
 use Formwork\Utils\FileSystem;
@@ -41,17 +42,16 @@ public function isLoggedIn(): bool
         if (!$this->request->hasPreviousSession()) {
             return false;
         }
-        $username = $this->request->session()->get('FORMWORK_USERNAME');
-        return !empty($username) && $this->users->has($username);
+        return $this->users->loggedIn() !== null;
     }
 
     /**
      * Return currently logged in user
      */
     public function user(): User
     {
-        $username = $this->request->session()->get('FORMWORK_USERNAME');
-        return $this->users->get($username);
+        return $this->users->loggedIn()
+            ?? throw new UserNotLoggedException('No user is logged in');
     }
 
     /**

diff --git a/formwork/src/Users/Exceptions/AuthenticationFailedException.php b/formwork/src/Users/Exceptions/AuthenticationFailedException.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class AuthenticationFailedException extends RuntimeException
+{
+}
diff --git a/formwork/src/Users/Exceptions/UserImageNotFoundException.php b/formwork/src/Users/Exceptions/UserImageNotFoundException.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserImageNotFoundException extends RuntimeException
+{
+}
diff --git a/formwork/src/Users/Exceptions/UserNotFoundException.php b/formwork/src/Users/Exceptions/UserNotFoundException.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserNotFoundException extends RuntimeException
+{
+}
diff --git a/formwork/src/Users/Exceptions/UserNotLoggedException.php b/formwork/src/Users/Exceptions/UserNotLoggedException.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserNotLoggedException extends RuntimeException
+{
+}
diff --git a/formwork/src/Users/User.php b/formwork/src/Users/User.php
@@ -10,11 +10,16 @@
 use Formwork\Log\Registry;
 use Formwork\Model\Model;
 use Formwork\Panel\Security\Password;
+use Formwork\Users\Exceptions\AuthenticationFailedException;
+use Formwork\Users\Exceptions\UserImageNotFoundException;
+use Formwork\Users\Exceptions\UserNotLoggedException;
 use Formwork\Utils\FileSystem;
-use UnexpectedValueException;
+use SensitiveParameter;
 
 class User extends Model
 {
+    public const SESSION_LOGGED_USER_KEY = '_formwork_logged_user';
+
     protected const MODEL_IDENTIFIER = 'user';
 
     /**
@@ -86,7 +91,7 @@ public function image(): Image
         $file = $this->fileFactory->make($path);
 
         if (!($file instanceof Image)) {
-            throw new UnexpectedValueException('Invalid user image');
+            throw new UserImageNotFoundException('Invalid user image');
         }
 
         return $this->image = $file;
@@ -115,20 +120,48 @@ public function permissions(): Permissions
         return $this->role->permissions();
     }
 
+    /**
+     * Authenticate the user
+     */
+    public function authenticate(
+        #[SensitiveParameter]
+        string $password
+    ): void {
+        if (!$this->verifyPassword($password)) {
+            throw new AuthenticationFailedException(sprintf('Authentication failed for user "%s"', $this->username()));
+        }
+        $this->request->session()->regenerate();
+        $this->request->session()->set(self::SESSION_LOGGED_USER_KEY, $this->username());
+    }
+
     /**
      * Return whether a given password authenticates the user
      */
-    public function authenticate(string $password): bool
-    {
+    public function verifyPassword(
+        #[SensitiveParameter]
+        string $password
+    ): bool {
         return Password::verify($password, $this->hash());
     }
 
+    /**
+     * Log out the user
+     */
+    public function logout(): void
+    {
+        if (!$this->isLoggedIn()) {
+            throw new UserNotLoggedException(sprintf('Cannot logout user "%s": user not logged', $this->username()));
+        }
+        $this->request->session()->remove(self::SESSION_LOGGED_USER_KEY);
+        $this->request->session()->destroy();
+    }
+
     /**
      * Return whether the user is logged or not
      */
-    public function isLogged(): bool
+    public function isLoggedIn(): bool
     {
-        return $this->request->session()->get('FORMWORK_USERNAME') === $this->username();
+        return $this->request->session()->get(self::SESSION_LOGGED_USER_KEY) === $this->username();
     }
 
     /**
@@ -144,7 +177,7 @@ public function isAdmin(): bool
      */
     public function canDeleteUser(User $user): bool
     {
-        return $this->isAdmin() && !$user->isLogged();
+        return $this->isAdmin() && !$user->isLoggedIn();
     }
 
     /**
@@ -155,7 +188,7 @@ public function canChangeOptionsOf(User $user): bool
         if ($this->isAdmin()) {
             return true;
         }
-        return $user->isLogged();
+        return $user->isLoggedIn();
     }
 
     /**
@@ -166,15 +199,15 @@ public function canChangePasswordOf(User $user): bool
         if ($this->isAdmin()) {
             return true;
         }
-        return $user->isLogged();
+        return $user->isLoggedIn();
     }
 
     /**
      * Return whether the user can change the role of a given user
      */
     public function canChangeRoleOf(User $user): bool
     {
-        return $this->isAdmin() && !$user->isLogged();
+        return $this->isAdmin() && !$user->isLoggedIn();
     }
 
     /**

diff --git a/formwork/src/Users/UserCollection.php b/formwork/src/Users/UserCollection.php
@@ -29,4 +29,12 @@ public function availableRoles(): array
     {
         return $this->roleCollection->everyItem()->title()->toArray();
     }
+
+    /**
+     * Get logged in user or null if no user is authenticated
+     */
+    public function loggedIn(): ?User
+    {
+        return $this->find(fn (User $user): bool => $user->isLoggedIn());
+    }
 }
diff --git a/panel/routes.php b/panel/routes.php
@@ -7,6 +7,7 @@
 use Formwork\Http\Request;
 use Formwork\Http\Response;
 use Formwork\Http\ResponseStatus;
+use Formwork\Panel\Controllers\AuthenticationController;
 use Formwork\Panel\Panel;
 use Formwork\Security\CsrfToken;
 use Formwork\Site;
@@ -271,7 +272,7 @@
 
                 if (!$csrfToken->validate($tokenName, $token)) {
                     $csrfToken->destroy($tokenName);
-                    $request->session()->remove('FORMWORK_USERNAME');
+                    $panel->user()->logout();
 
                     $panel->notify(
                         $translations->getCurrent()->translate('panel.login.suspiciousRequestDetected'),
@@ -323,8 +324,8 @@
         'panel.redirectToLogin' => [
             'action' => static function (Request $request, Site $site, Panel $panel) {
                 // Redirect to login if no user is logged
-                if (!$site->users()->isEmpty() && !$panel->isLoggedIn() && $panel->route() !== '/login/') {
-                    $request->session()->set('FORMWORK_REDIRECT_TO', $panel->route());
+                if (!$site->users()->isEmpty() && !$panel->isLoggedIn() && !in_array($panel->route(), ['/login/', '/logout/'], true)) {
+                    $request->session()->set(AuthenticationController::SESSION_REDIRECT_KEY, $panel->route());
                     return new RedirectResponse($panel->uri('/login/'));
                 }
             },