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

Refactor user authentication

formwork/src/Panel/Controllers/AuthenticationController.php

@@ -8,16 +8,24 @@ use Formwork\Http\Response;
 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,39 +34,29 @@ class AuthenticationController extends AbstractController
             return $this->error($this->translate('panel.login.attempt.tooMany', $minutes));
         }
 
-        switch ($this->request->method()) {
+        if ($this->request->method() === RequestMethod::POST) {
-            case RequestMethod::GET:
+            // Delay request processing for 0.5-1s
-                if ($this->request->session()->has('FORMWORK_USERNAME')) {
+            usleep(random_int(500, 1000) * 1000);
-                    return $this->redirect($this->generateRoute('panel.index'));
-                }
 
-                // Always generate a new CSRF token
+            $data = $this->request->input();
+
+            // Ensure no required data is missing
+            if (!$data->hasMultiple(['username', 'password'])) {
                 $this->csrfToken->generate($csrfTokenName);
+                $this->error($this->translate('panel.login.attempt.failed'));
+            }
 
-                return new Response($this->view('authentication.login', [
+            $accessLimiter->registerAttempt();
-                    'title' => $this->translate('panel.login.login'),
-                ]));
 
-            case RequestMethod::POST:
+            $username = $data->get('username');
-                // Delay request processing for 0.5-1s
-                usleep(random_int(500, 1000) * 1000);
 
-                $data = $this->request->input();
+            /** @var User */
+            $user = $this->site->users()->get($username);
 
-                // Ensure no required data is missing
+            // Authenticate user
-                if (!$data->hasMultiple(['username', 'password'])) {
+            if ($user !== null) {
-                    $this->csrfToken->generate($csrfTokenName);
+                try {
-                    $this->error($this->translate('panel.login.attempt.failed'));
+                    $user->authenticate($data->get('password'));
-                }
-
-                $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'));
 
                     // Regenerate CSRF token
                     $this->csrfToken->generate($csrfTokenName);
@@ -66,27 +64,36 @@ class AuthenticationController extends AbstractController
                     $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'));
+                    $time = $accessLog->log($username);
-                    $lastAccessRegistry->set($data->get('username'), $time);
+                    $lastAccessRegistry->set($username, $time);
 
                     $accessLimiter->resetAttempts();
 
-                    if (($destination = $this->request->session()->get('FORMWORK_REDIRECT_TO')) !== null) {
+                    if (($destination = $this->request->session()->get(self::SESSION_REDIRECT_KEY)) !== null) {
-                        $this->request->session()->remove('FORMWORK_REDIRECT_TO');
+                        $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);
+            $this->csrfToken->generate($csrfTokenName);
-                return $this->error($this->translate('panel.login.attempt.failed'), [
+
-                    'username' => $data->get('username'),
+            return $this->error($this->translate('panel.login.attempt.failed'), [
-                    'error'    => true,
+                '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'),
+        ]));
     }
 
     /**
@@ -94,14 +101,19 @@ class AuthenticationController extends AbstractController
      */
     public function logout(): RedirectResponse
     {
-        $this->csrfToken->destroy($this->panel()->getCsrfTokenName());
+        try {
-        $this->request->session()->remove('FORMWORK_USERNAME');
+            $this->panel->user()->logout();
-        $this->request->session()->destroy();
+            $this->csrfToken->destroy($this->panel()->getCsrfTokenName());
 
-        if ($this->config->get('system.panel.logoutRedirect') === 'home') {
+            if ($this->config->get('system.panel.logoutRedirect') === 'home') {
-            return $this->redirect('/');
+                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'));
     }
 

formwork/src/Panel/Controllers/RegisterController.php

@@ -10,6 +10,7 @@ use Formwork\Log\Registry;
 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 @@ class RegisterController extends AbstractController
                 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'));

formwork/src/Panel/Panel.php

@@ -8,6 +8,7 @@ use Formwork\Http\Request;
 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,8 +42,7 @@ final class Panel
         if (!$this->request->hasPreviousSession()) {
             return false;
         }
-        $username = $this->request->session()->get('FORMWORK_USERNAME');
+        return $this->users->loggedIn() !== null;
-        return !empty($username) && $this->users->has($username);
     }
 
     /**
@@ -50,8 +50,8 @@ final class Panel
      */
     public function user(): User
     {
-        $username = $this->request->session()->get('FORMWORK_USERNAME');
+        return $this->users->loggedIn()
-        return $this->users->get($username);
+            ?? throw new UserNotLoggedException('No user is logged in');
     }
 
     /**

formwork/src/Users/Exceptions/AuthenticationFailedException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class AuthenticationFailedException extends RuntimeException
+{
+}

formwork/src/Users/Exceptions/UserImageNotFoundException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserImageNotFoundException extends RuntimeException
+{
+}

formwork/src/Users/Exceptions/UserNotFoundException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserNotFoundException extends RuntimeException
+{
+}

formwork/src/Users/Exceptions/UserNotLoggedException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Formwork\Users\Exceptions;
+
+use RuntimeException;
+
+class UserNotLoggedException extends RuntimeException
+{
+}

formwork/src/Users/User.php

@@ -10,11 +10,16 @@ use Formwork\Images\Image;
 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 @@ class User extends Model
         $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 @@ class User extends Model
         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 @@ class User extends Model
      */
     public function canDeleteUser(User $user): bool
     {
-        return $this->isAdmin() && !$user->isLogged();
+        return $this->isAdmin() && !$user->isLoggedIn();
     }
 
     /**
@@ -155,7 +188,7 @@ class User extends Model
         if ($this->isAdmin()) {
             return true;
         }
-        return $user->isLogged();
+        return $user->isLoggedIn();
     }
 
     /**
@@ -166,7 +199,7 @@ class User extends Model
         if ($this->isAdmin()) {
             return true;
         }
-        return $user->isLogged();
+        return $user->isLoggedIn();
     }
 
     /**
@@ -174,7 +207,7 @@ class User extends Model
      */
     public function canChangeRoleOf(User $user): bool
     {
-        return $this->isAdmin() && !$user->isLogged();
+        return $this->isAdmin() && !$user->isLoggedIn();
     }
 
     /**

formwork/src/Users/UserCollection.php

@@ -29,4 +29,12 @@ class UserCollection extends AbstractCollection
     {
         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());
+    }
 }

panel/routes.php

@@ -7,6 +7,7 @@ use Formwork\Http\RedirectResponse;
 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 @@ return [
 
                 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 @@ return [
         '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/') {
+                if (!$site->users()->isEmpty() && !$panel->isLoggedIn() && !in_array($panel->route(), ['/login/', '/logout/'], true)) {
-                    $request->session()->set('FORMWORK_REDIRECT_TO', $panel->route());
+                    $request->session()->set(AuthenticationController::SESSION_REDIRECT_KEY, $panel->route());
                     return new RedirectResponse($panel->uri('/login/'));
                 }
             },