From 0eba267a733f53b139e014d827bde2e8c32f80d3 Mon Sep 17 00:00:00 2001 From: Hugo Sales Date: Wed, 22 Jul 2020 11:45:03 +0000 Subject: [PATCH] [LOGIN] Implement password checking and related systems --- config/packages/security.yaml | 7 ++++-- src/Core/GNUsocial.php | 15 +++++++++++- src/Entity/LocalUser.php | 45 ++++++++++++++++++++++++++++++++++ src/Security/Authenticator.php | 33 +++++-------------------- src/Util/Common.php | 2 +- 5 files changed, 71 insertions(+), 31 deletions(-) diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 6366543e40..8eb456caf1 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -1,7 +1,10 @@ security: # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers providers: - users_in_memory: { memory: null } + users: + entity: + class: 'App\Entity\LocalUser' + property: 'nickname' firewalls: dev: pattern: ^/(_(profiler|wdt)|css|images|js)/ @@ -9,7 +12,7 @@ security: main: anonymous: true lazy: true - provider: users_in_memory + provider: users guard: authenticators: - App\Security\Authenticator diff --git a/src/Core/GNUsocial.php b/src/Core/GNUsocial.php index 145c8fc9ab..baaeb9e410 100644 --- a/src/Core/GNUsocial.php +++ b/src/Core/GNUsocial.php @@ -51,14 +51,18 @@ use Symfony\Component\Console\Event\ConsoleCommandEvent; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\FormFactoryInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Routing\RouterInterface; +use Symfony\Component\Security\Http\Util\TargetPathTrait; use Symfony\Contracts\Translation\TranslatorInterface; class GNUsocial implements EventSubscriberInterface { + use TargetPathTrait; + protected LoggerInterface $logger; protected TranslatorInterface $translator; protected EntityManagerInterface $entity_manager; @@ -66,6 +70,7 @@ class GNUsocial implements EventSubscriberInterface protected FormFactoryInterface $form_factory; protected MessageBusInterface $message_bus; protected EventDispatcherInterface $event_dispatcher; + protected SessionInterface $session; /** * Symfony dependency injection gives us access to these services @@ -76,7 +81,8 @@ class GNUsocial implements EventSubscriberInterface RouterInterface $router, FormFactoryInterface $ff, MessageBusInterface $mb, - EventDispatcherInterface $ed) + EventDispatcherInterface $ed, + SessionInterface $s) { $this->logger = $logger; $this->translator = $trans; @@ -85,6 +91,7 @@ class GNUsocial implements EventSubscriberInterface $this->form_factory = $ff; $this->message_bus = $mb; $this->event_dispatcher = $ed; + $this->session = $s; $this->register(); } @@ -122,6 +129,12 @@ class GNUsocial implements EventSubscriberInterface public function onKernelRequest(RequestEvent $event, string $event_name): RequestEvent { + $request = $event->getRequest(); + if (!(!$event->isMasterRequest() || $request->isXmlHttpRequest() + || 'login' === $request->attributes->get('_route'))) { + $this->saveTargetPath($this->session, 'main', $request->getUri()); + } + $this->register(); return $event; } diff --git a/src/Entity/LocalUser.php b/src/Entity/LocalUser.php index 211a592bde..cc7ee5c79e 100644 --- a/src/Entity/LocalUser.php +++ b/src/Entity/LocalUser.php @@ -19,7 +19,9 @@ namespace App\Entity; +use App\Core\DB\DB; use App\Core\UserRoles; +use App\Util\Common; use DateTimeInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -307,4 +309,47 @@ class LocalUser implements UserInterface public function eraseCredentials() { } + + public function checkPassword(string $new_password): bool + { + // Timing safe password verification on supported PHP versions + if (password_verify($new_password, $this->getPassword())) { + return true; + } + + // Old format + // crypt understands what the salt part of $this->getPassword() is + if ($this->getPassword() === crypt($new_password, $this->getPassword())) { + $this->changePassword($new_password, true); + return true; + } + + return false; + } + + public function changePassword(string $new_password, bool $override = false): void + { + if ($override || $this->checkPassword($new_password)) { + $this->setPassword($this->hashPassword($new_password)); + DB::flush(); + } + } + + public function hashPassword(string $password) + { + switch (Common::config('security', 'algorithm')) { + case 'bcrypt': + $algorithm = PASSWORD_BCRYPT; + break; + case 'argon2i': + $algorithm = PASSWORD_ARGON2I; + break; + case 'argon2id': + $algorithm = PASSWORD_ARGON2ID; + break; + } + $options = Common::config('security', 'options'); + + return password_hash($password, $algorithm, $options); + } } diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index 0381e88302..ded2e461d4 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -21,7 +21,6 @@ namespace App\Security; use App\Core\DB\DB; use function App\Core\I18n\_m; -use App\Core\Log; use App\Entity\User; use App\Util\Nickname; use Doctrine\ORM\EntityManagerInterface; @@ -95,10 +94,9 @@ class Authenticator extends AbstractFormLoginAuthenticator $nick = Nickname::normalize($credentials['nickname']); $user = DB::findOneBy('local_user', ['or' => ['nickname' => $nick, 'outgoing_email' => $nick]]); - if (!$user) { throw new CustomUserMessageAuthenticationException( - _m('Either \'{nickname}\' doesn\'t match any registered nickname or email, or the supplied password is incorrect.', ['{nickname}' => $credentials['nickname']])); + _m('\'{nickname}\' doesn\'t match any registered nickname or email.', ['{nickname}' => $credentials['nickname']])); } return $user; @@ -106,29 +104,11 @@ class Authenticator extends AbstractFormLoginAuthenticator public function checkCredentials($credentials, UserInterface $user) { - $password = $user->getPassword(); - Log::error(print_r($user, true)); - // crypt understands what the salt part of $user->password is - if ($password === crypt($credentials['password'], $user->password)) { - $this->changePassword($user->nickname, null, $password); - return $user; + if (!$user->checkPassword($credentials['password'])) { + throw new CustomUserMessageAuthenticationException(_m('Invalid login credentials.')); + } else { + return true; } - - // If we check StatusNet hash, for backwards compatibility and migration - if ($this->statusnet && $user->password === md5($password . $user->id)) { - // and update password hash entry to crypt() compatible - if ($this->overwrite) { - $this->changePassword($user->nickname, null, $password); - } - return $user; - } - - // Timing safe password verification on supported PHP versions - if (password_verify($password, $user->password)) { - return $user; - } - - return false; } public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey) @@ -137,8 +117,7 @@ class Authenticator extends AbstractFormLoginAuthenticator return new RedirectResponse($targetPath); } - // For example : return new RedirectResponse($this->urlGenerator->generate('some_route')); - throw new \Exception('TODO: provide a valid redirect inside ' . __FILE__); + return new RedirectResponse($this->urlGenerator->generate('main_all')); } protected function getLoginUrl() diff --git a/src/Util/Common.php b/src/Util/Common.php index 858c48893f..f4dac1c4a9 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -42,7 +42,7 @@ abstract class Common { $c = DB::find('config', ['section' => $section, 'setting' => $setting]); if ($c === null) { - throw new Exception("The field section = {$section} and setting = {$setting} doesn't exist"); + throw new \Exception("The field section = {$section} and setting = {$setting} doesn't exist"); } return unserialize($c->getValue());