From d8c34046f52ef73d8cbcc1fba9cdb12acfdc7bdd Mon Sep 17 00:00:00 2001 From: Giuseppe Criscione <18699708+giuscris@users.noreply.github.com> Date: Fri, 25 Oct 2024 00:41:10 +0200 Subject: [PATCH 1/2] Add user-related exceptions --- .../Users/Exceptions/AuthenticationFailedException.php | 9 +++++++++ .../src/Users/Exceptions/UserImageNotFoundException.php | 9 +++++++++ formwork/src/Users/Exceptions/UserNotFoundException.php | 9 +++++++++ formwork/src/Users/Exceptions/UserNotLoggedException.php | 9 +++++++++ 4 files changed, 36 insertions(+) create mode 100644 formwork/src/Users/Exceptions/AuthenticationFailedException.php create mode 100644 formwork/src/Users/Exceptions/UserImageNotFoundException.php create mode 100644 formwork/src/Users/Exceptions/UserNotFoundException.php create mode 100644 formwork/src/Users/Exceptions/UserNotLoggedException.php diff --git a/formwork/src/Users/Exceptions/AuthenticationFailedException.php b/formwork/src/Users/Exceptions/AuthenticationFailedException.php new file mode 100644 index 00000000..98db2686 --- /dev/null +++ b/formwork/src/Users/Exceptions/AuthenticationFailedException.php @@ -0,0 +1,9 @@ + Date: Fri, 25 Oct 2024 01:13:00 +0200 Subject: [PATCH 2/2] Move authentication to `User` --- .../Controllers/AuthenticationController.php | 100 ++++++++++-------- .../Panel/Controllers/RegisterController.php | 3 +- formwork/src/Panel/Panel.php | 8 +- formwork/src/Users/User.php | 53 ++++++++-- formwork/src/Users/UserCollection.php | 8 ++ panel/routes.php | 7 +- 6 files changed, 117 insertions(+), 62 deletions(-) diff --git a/formwork/src/Panel/Controllers/AuthenticationController.php b/formwork/src/Panel/Controllers/AuthenticationController.php index 5cae8085..02b12d7d 100644 --- 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,39 +34,29 @@ 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); @@ -66,27 +64,36 @@ public function login(AccessLimiter $accessLimiter): Response $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'), + ])); } /** @@ -94,14 +101,19 @@ public function login(AccessLimiter $accessLimiter): Response */ 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 index b30fcc1e..8ea20406 100644 --- 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 index b66868ea..e8e0857b 100644 --- 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,8 +42,7 @@ 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; } /** @@ -50,8 +50,8 @@ public function isLoggedIn(): bool */ 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/User.php b/formwork/src/Users/User.php index 1a7f2e4d..377cba09 100644 --- 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,7 +199,7 @@ public function canChangePasswordOf(User $user): bool if ($this->isAdmin()) { return true; } - return $user->isLogged(); + return $user->isLoggedIn(); } /** @@ -174,7 +207,7 @@ public function canChangePasswordOf(User $user): bool */ 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 index c92aae7c..8ea67feb 100644 --- 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 index 6c146e1c..a4ed988a 100644 --- 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/')); } },