diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 897ccfc7b7..c186a5a024 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -1,6 +1,10 @@ security: - # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + enable_authenticator_manager: true + password_hashers: + App\Entity\LocalUser: + algorithm: auto providers: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers local_user: chain: providers: [local_user_by_nickname, local_user_by_email] @@ -11,18 +15,21 @@ security: local_user_by_email: entity: class: 'App\Entity\LocalUser' - property: 'email' + property: 'outgoing_email' firewalls: dev: pattern: ^/(_(profiler|wdt)|css|images|js)/ security: false main: - anonymous: true - lazy: true - provider: local_user + entry_point: App\Security\Authenticator guard: - authenticators: - - App\Security\Authenticator + authenticators: + - App\Security\Authenticator + provider: local_user + form_login: + login_path: security_login + check_path: security_login + enable_csrf: true logout: path: security_logout # where to redirect after logout diff --git a/config/services.yaml b/config/services.yaml index cd2a9087a4..45b5ac9d9f 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -52,3 +52,4 @@ services: resource: '../components/*' exclude: '../components/*/{scripts,classes,lib,actions,locale,doc}' tags: ['controller.service_arguments'] + diff --git a/src/Controller/AdminPanel.php b/src/Controller/AdminPanel.php index 4bb9f319e9..c5961204da 100644 --- a/src/Controller/AdminPanel.php +++ b/src/Controller/AdminPanel.php @@ -54,7 +54,7 @@ class AdminPanel extends Controller */ public function site(Request $request) { - // TODO CHECK PERMISSION + $this->denyAccessUnlessGranted('ROLE_ADMIN'); $defaults = Common::getConfigDefaults(); $options = []; foreach ($defaults as $key => $inner) { diff --git a/src/Controller/Security.php b/src/Controller/Security.php index 9f47f2b450..1d150c624b 100644 --- a/src/Controller/Security.php +++ b/src/Controller/Security.php @@ -8,12 +8,9 @@ use App\Core\Controller; use App\Core\DB\DB; use App\Core\Event; use App\Core\Form; -use function App\Core\I18n\_m; use App\Core\Log; -use App\Core\VisibilityScope; use App\Entity\Actor; use App\Entity\LocalUser; -use App\Entity\Note; use App\Entity\Subscription; use App\Security\Authenticator; use App\Security\EmailVerifier; @@ -37,10 +34,12 @@ use Symfony\Component\Form\Extension\Core\Type\SubmitType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; use Symfony\Component\Security\Http\Authentication\AuthenticationUtils; use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; +use function App\Core\I18n\_m; class Security extends Controller { @@ -49,6 +48,7 @@ class Security extends Controller */ public function login(AuthenticationUtils $authenticationUtils) { + // Skip if already logged in if ($this->getUser()) { return $this->redirectToRoute('main_all'); } @@ -90,9 +90,10 @@ class Security extends Controller */ public function register( Request $request, - GuardAuthenticatorHandler $guard_handler, + UserPasswordHasherInterface $user_password_hasher, Authenticator $authenticator, - ): array|Response|null { + GuardAuthenticatorHandler $guard, + ): array|Response { $form = Form::create([ ['nickname', TextType::class, [ 'label' => _m('Nickname'), @@ -100,10 +101,9 @@ class Security extends Controller 'constraints' => [ new NotBlank(['message' => _m('Please enter a nickname')]), new Length([ - 'min' => 1, - 'minMessage' => _m(['Your nickname must be at least # characters long'], ['count' => 1]), 'max' => Nickname::MAX_LEN, - 'maxMessage' => _m(['Your nickname must be at most # characters long'], ['count' => Nickname::MAX_LEN]), ]), + 'maxMessage' => _m(['Your nickname must be at most # characters long'], ['count' => Nickname::MAX_LEN]), + ]), ], 'block_name' => 'nickname', 'label_attr' => ['class' => 'section-form-label'], @@ -116,8 +116,9 @@ class Security extends Controller 'block_name' => 'email', 'label_attr' => ['class' => 'section-form-label'], 'invalid_message' => _m('Email not valid. Please provide a valid email.'), + 'attr' => ['autocomplete' => 'email'], ]], - FormFields::repeated_password(), + FormFields::repeated_password(['attr' => ['autocomplete' => 'new-password']]), ['register', SubmitType::class, ['label' => _m('Register')]], ], form_options: ['block_prefix' => 'register']); @@ -128,15 +129,11 @@ class Security extends Controller $data['password'] = $form->get('password')->getData(); // Already used is checked below - $sanitized_nickname = null; - if (Event::handle('SanitizeNickname', [$data['nickname'], &$sanitized_nickname]) != Event::stop) { - $sanitized_nickname = $data['nickname']; - // $sanitized_nickname = Nickname::normalize($data['nickname'], check_already_used: false, which: Nickname::CHECK_LOCAL_USER, check_is_allowed: false); - } + $nickname = Nickname::normalize($data['nickname'], check_already_used: false, which: Nickname::CHECK_LOCAL_USER, check_is_allowed: false); try { - $found_user = DB::findOneBy('local_user', ['or' => ['nickname' => $sanitized_nickname, 'outgoing_email' => $data['email']]]); - if ($found_user->getNickname() === $sanitized_nickname) { + $found_user = DB::findOneBy('local_user', ['or' => ['nickname' => $nickname, 'outgoing_email' => $data['email']]]); + if ($found_user->getNickname() === $nickname) { throw new NicknameTakenException($found_user->getActor()); } elseif ($found_user->getOutgoingEmail() === $data['email']) { throw new EmailTakenException($found_user->getActor()); @@ -148,13 +145,13 @@ class Security extends Controller try { // This already checks if the nickname is being used - $actor = Actor::create(['nickname' => $sanitized_nickname, 'is_local' => true]); + $actor = Actor::create(['nickname' => $nickname, 'is_local' => true]); $user = LocalUser::create([ - 'nickname' => $sanitized_nickname, + 'nickname' => $nickname, 'outgoing_email' => $data['email'], 'incoming_email' => $data['email'], - 'password' => LocalUser::hashPassword($data['password']), ]); + $user->setPassword($user_password_hasher->hashPassword($user, $data['password'])); DB::persistWithSameId( $actor, $user, @@ -169,7 +166,7 @@ class Security extends Controller } catch (UniqueConstraintViolationException $e) { // _something_ was duplicated, but since we already check if nickname is in use, we can't tell what went wrong $m = 'An error occurred while trying to register'; - Log::critical($m . " with nickname: '{$sanitized_nickname}' and email '{$data['email']}'"); + Log::critical($m . " with nickname: '{$nickname}' and email '{$data['email']}'"); throw new ServerException(_m($m), previous: $e); } // @codeCoverageIgnoreEnd @@ -177,24 +174,24 @@ class Security extends Controller // generate a signed url and email it to the user if ($_ENV['APP_ENV'] !== 'dev' && Common::config('site', 'use_email')) { // @codeCoverageIgnoreStart - EmailVerifier::sendEmailConfirmation($user); + // TODO: Implement send confirmation email + // (new EmailVerifier())->sendEmailConfirmation($user); // @codeCoverageIgnoreEnd } else { $user->setIsEmailVerified(true); } - return $guard_handler->authenticateUserAndHandleSuccess( + return $guard->authenticateUserAndHandleSuccess( $user, $request, $authenticator, - 'main', // firewall name in security.yaml + 'main', ); } return [ '_template' => 'security/register.html.twig', 'registration_form' => $form->createView(), - 'notes_fn' => fn () => Note::getAllNotes(VisibilityScope::$instance_scope), ]; } } diff --git a/src/Core/GNUsocial.php b/src/Core/GNUsocial.php index e73b084af8..e717f52fb2 100644 --- a/src/Core/GNUsocial.php +++ b/src/Core/GNUsocial.php @@ -49,7 +49,6 @@ use App\Core\I18n\I18n; use App\Core\Queue\Queue; use App\Core\Router\Router; use App\Kernel; -use App\Security\EmailVerifier; use App\Util\Common; use App\Util\Exception\ConfigurationException; use App\Util\Formatting; @@ -174,7 +173,6 @@ class GNUsocial implements EventSubscriberInterface HTTPClient::setClient($this->client); Formatting::setTwig($this->twig); Cache::setupCache(); - EmailVerifier::setEmailHelpers($this->mailer_helper, $this->email_verify_helper, $this->reset_password_helper); DB::initTableMap(); diff --git a/src/Entity/LocalUser.php b/src/Entity/LocalUser.php index f8007b6cf0..4201abe8c2 100644 --- a/src/Entity/LocalUser.php +++ b/src/Entity/LocalUser.php @@ -31,6 +31,7 @@ use App\Util\Common; use DateTimeInterface; use Exception; use libphonenumber\PhoneNumber; +use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; /** @@ -47,7 +48,7 @@ use Symfony\Component\Security\Core\User\UserInterface; * @copyright 2020-2021 Free Software Foundation, Inc http://www.fsf.org * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ -class LocalUser extends Entity implements UserInterface +class LocalUser extends Entity implements UserInterface, PasswordAuthenticatedUserInterface { // {{{ Autocode // @codeCoverageIgnoreStart @@ -301,7 +302,7 @@ class LocalUser extends Entity implements UserInterface return false; } - public static function hashPassword(string $password) + public static function hashPassword(string $password): string { $algorithm = self::algoNameToConstant(Common::config('security', 'algorithm')); $options = Common::config('security', 'options'); @@ -329,6 +330,29 @@ class LocalUser extends Entity implements UserInterface throw new Exception('Unsupported or unsafe hashing algorithm requested'); } } + + /** + * Returns the username used to authenticate the user. + * Part of the Symfony UserInterface + * + * @return string + * + * @deprecated since Symfony 5.3, use getUserIdentifier() instead + */ + public function getUsername(): string + { + return $this->getUserIdentifier(); + } + + /** + * returns the identifier for this user (e.g. its nickname) + * Part of the Symfony UserInterface + * @return string + */ + public function getUserIdentifier(): string + { + return $this->getNickname(); + } // }}} Authentication public function getActor() @@ -344,14 +368,6 @@ class LocalUser extends Entity implements UserInterface return UserRoles::toArray($this->getActor()->getRoles()); } - /** - * Returns the username used to authenticate the user. Part of the Symfony UserInterface - */ - public function getUsername() - { - return $this->nickname; - } - public static function getByNickname(string $nickname): ?self { $key = str_replace('_', '-', $nickname); diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index 45ae780f4c..a5f56e3757 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -1,6 +1,6 @@ attributes->get('_route') && $request->isMethod('POST'); } @@ -77,12 +79,12 @@ class Authenticator extends AbstractFormLoginAuthenticator /** * @return array */ - public function getCredentials(Request $request) + public function getCredentials(Request $request): array { return [ 'nickname_or_email' => $request->request->get('nickname_or_email'), - 'password' => $request->request->get('password'), - 'csrf_token' => $request->request->get('_csrf_token'), + 'password' => $request->request->get('password'), + 'csrf_token' => $request->request->get('_csrf_token'), ]; } @@ -90,10 +92,12 @@ class Authenticator extends AbstractFormLoginAuthenticator * Get a user given credentials and a CSRF token * * @param array $credentials result of self::getCredentials - * + * @param UserProviderInterface $userProvider * @return ?LocalUser + * @throws NoSuchActorException + * @throws ServerException */ - public function getUser($credentials, UserProviderInterface $userProvider) + public function getUser($credentials, UserProviderInterface $userProvider): ?LocalUser { $token = new CsrfToken('authenticate', $credentials['csrf_token']); if (!$this->csrfTokenManager->isTokenValid($token)) { @@ -106,11 +110,11 @@ class Authenticator extends AbstractFormLoginAuthenticator } elseif (Nickname::isValid($credentials['nickname_or_email'])) { $user = LocalUser::getByNickname($credentials['nickname_or_email']); } - if ($user === null) { + if (is_null($user)) { throw new NoSuchActorException('No such local user.'); } $credentials['nickname'] = $user->getNickname(); - } catch (NotFoundException) { + } catch (NoSuchActorException|NotFoundException) { throw new CustomUserMessageAuthenticationException( _m('Invalid login credentials.'), ); @@ -120,9 +124,11 @@ class Authenticator extends AbstractFormLoginAuthenticator /** * @param array $credentials result of self::getCredentials - * @param LocalUser $user + * @param LocalUser $user + * @return bool + * @throws ServerException */ - public function checkCredentials($credentials, $user) + public function checkCredentials($credentials, $user): bool { if (!$user->checkPassword($credentials['password'])) { throw new CustomUserMessageAuthenticationException(_m('Invalid login credentials.')); @@ -134,13 +140,13 @@ class Authenticator extends AbstractFormLoginAuthenticator /** * After a successful login, redirect user to the path saved in their session or to the root of the website */ - public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey) + public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey): RedirectResponse { $nickname = $token->getUser(); if ($nickname instanceof Stringable) { - $nickname = (string) $nickname; + $nickname = (string)$nickname; } elseif ($nickname instanceof UserInterface) { - $nickname = $nickname->getUsername(); + $nickname = $nickname->getUserIdentifier(); } $request->getSession()->set( @@ -155,7 +161,7 @@ class Authenticator extends AbstractFormLoginAuthenticator return new RedirectResponse(Router::url('main_all')); } - protected function getLoginUrl() + protected function getLoginUrl(): string { return Router::url(self::LOGIN_ROUTE); } diff --git a/src/Security/EmailVerifier.php b/src/Security/EmailVerifier.php index d566bff62f..c5da57b521 100644 --- a/src/Security/EmailVerifier.php +++ b/src/Security/EmailVerifier.php @@ -4,109 +4,56 @@ declare(strict_types = 1); namespace App\Security; -use App\Controller\ResetPassword; -use App\Core\DB\DB; -use function App\Core\I18n\_m; -use App\Entity\LocalUser; -use App\Util\Common; -use App\Util\Exception\NotFoundException; -use App\Util\Exception\RedirectException; +use Doctrine\ORM\EntityManagerInterface; use Symfony\Bridge\Twig\Mime\TemplatedEmail; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Mailer\MailerInterface; -use Symfony\Component\Mime\Address; -use SymfonyCasts\Bundle\ResetPassword\Exception\ResetPasswordExceptionInterface; -use SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelperInterface; +use Symfony\Component\Security\Core\User\UserInterface; use SymfonyCasts\Bundle\VerifyEmail\Exception\VerifyEmailExceptionInterface; use SymfonyCasts\Bundle\VerifyEmail\VerifyEmailHelperInterface; -abstract class EmailVerifier +class EmailVerifier { - private static ?MailerInterface $mailer_helper; - private static ?VerifyEmailHelperInterface $verify_email_helper; - private static ?ResetPasswordHelperInterface $reset_password_helper; + private $verifyEmailHelper; + private $mailer; + private $entityManager; - public static function setEmailHelpers(MailerInterface $mailer, VerifyEmailHelperInterface $email_helper, ResetPasswordHelperInterface $reset_helper) + public function __construct(VerifyEmailHelperInterface $helper, MailerInterface $mailer, EntityManagerInterface $manager) { - self::$mailer_helper = $mailer; - self::$verify_email_helper = $email_helper; - self::$reset_password_helper = $reset_helper; + $this->verifyEmailHelper = $helper; + $this->mailer = $mailer; + $this->entityManager = $manager; } - public static function validateTokenAndFetchUser(string $token) + public function sendEmailConfirmation(string $verifyEmailRouteName, UserInterface $user, TemplatedEmail $email): void { - return self::$reset_password_helper->validateTokenAndFetchUser($token); - } - - public static function removeResetRequest(string $token): void - { - self::$reset_password_helper->removeResetRequest($token); - } - - public static function sendEmailConfirmation(LocalUser $user): void - { - $email = (new TemplatedEmail()) - ->from(new Address(Common::config('site', 'email'), Common::config('site', 'nickname'))) - ->to($user->getOutgoingEmail()) - ->subject(_m('Please Confirm your Email')) - ->htmlTemplate('security/confirmation_email.html.twig'); - - $signatureComponents = self::$verify_email_helper->generateSignature( - 'verify_email', + $signatureComponents = $this->verifyEmailHelper->generateSignature( + $verifyEmailRouteName, $user->getId(), $user->getOutgoingEmail(), + ['id' => $user->getId()], ); - $context = $email->getContext(); - $context['signedUrl'] = $signatureComponents->getSignedUrl(); - $context['expiresAt'] = $signatureComponents->getExpiresAt(); + $context = $email->getContext(); + $context['signedUrl'] = $signatureComponents->getSignedUrl(); + $context['expiresAtMessageKey'] = $signatureComponents->getExpirationMessageKey(); + $context['expiresAtMessageData'] = $signatureComponents->getExpirationMessageData(); $email->context($context); - self::send($email); - } - - public static function send($email): void - { - self::$mailer_helper->send($email); + $this->mailer->send($email); } /** * @throws VerifyEmailExceptionInterface */ - public function handleEmailConfirmation(Request $request, LocalUser $user): void + public function handleEmailConfirmation(Request $request, UserInterface $user): void { - self::$verify_email_helper->validateEmailConfirmation($request->getUri(), $user->getId(), $user->getOutgoingEmail()); - $user->setIsEmailVerified(true); - DB::persist($user); - DB::flush(); - } + $this->verifyEmailHelper->validateEmailConfirmation($request->getUri(), $user->getId(), $user->getOutgoingEmail()); - public static function processSendingPasswordResetEmail(string $emailFormData, ResetPassword $controller) - { - try { - $user = DB::findOneBy('local_user', ['outgoing_email' => $emailFormData]); - $reset_token = self::$reset_password_helper->generateResetToken($user); - // Found a user - } catch (NotFoundException|ResetPasswordExceptionInterface) { - // Not found, do not reveal whether a user account was found or not. - throw new RedirectException('check_email'); - } + $user->setIsVerified(true); - $email = (new TemplatedEmail()) - ->from(new Address('foo@email.com', 'FOO NAME')) - ->to($user->getOutgoingEmail()) - ->subject('Your password reset request') - ->htmlTemplate('reset_password/email.html.twig') - ->context([ - 'resetToken' => $reset_token, - ]); - - self::send($email); - - // Store the token object in session for retrieval in check-email route. - $controller->setInSession($reset_token); - - throw new RedirectException('check_email'); + $this->entityManager->persist($user); + $this->entityManager->flush(); } } diff --git a/src/Util/Nickname.php b/src/Util/Nickname.php index 2501017357..de4b508472 100644 --- a/src/Util/Nickname.php +++ b/src/Util/Nickname.php @@ -24,6 +24,7 @@ declare(strict_types = 1); namespace App\Util; use App\Core\DB\DB; +use App\Core\Log; use App\Util\Exception\DuplicateFoundException; use App\Util\Exception\NicknameEmptyException; use App\Util\Exception\NicknameException; @@ -184,9 +185,11 @@ class Nickname */ public static function normalize(string $nickname, bool $check_already_used = false, int $which = self::CHECK_LOCAL_USER, bool $check_is_allowed = true): string { + // Nicknames are lower case and without trailing spaces, it's not offensive to sanitize that to the user $nickname = trim($nickname); $nickname = mb_strtolower($nickname); - // We could do UTF-8 normalization (å to a, etc.) with something like Normalizer::normalize($nickname, Normalizer::FORM_C) + // Anything else would likely be very confusing + // We could, e.g., do UTF-8 normalization (å to a, etc.) with something like Normalizer::normalize($nickname, Normalizer::FORM_C) // We won't as it could confuse tremendously the user, he must know what is valid and should fix his own input if (!self::validate(nickname: $nickname, check_already_used: $check_already_used, which: $which, check_is_allowed: $check_is_allowed) || !self::isCanonical($nickname)) { diff --git a/templates/security/login.html.twig b/templates/security/login.html.twig index a163bbd22e..71b1fcead6 100644 --- a/templates/security/login.html.twig +++ b/templates/security/login.html.twig @@ -39,8 +39,9 @@ {# TODO: Login can be done with email, so the element id's should reflect that #}
- - + +

{{ "Your nickname or email address." | trans }}

@@ -50,14 +51,14 @@
- - + + +
- {% endif %}