[CORE][SECURITY] Move to the new authentication format, for Symfony 5.3

Keep using (deprecated) Guard
This commit is contained in:
Hugo Sales 2021-11-16 14:48:18 +00:00 committed by Diogo Peralta Cordeiro
parent b4ce77320e
commit 3a5e52ee0d
Signed by: diogo
GPG Key ID: 18D2D35001FBFAB0
10 changed files with 122 additions and 146 deletions

View File

@ -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
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

View File

@ -52,3 +52,4 @@ services:
resource: '../components/*'
exclude: '../components/*/{scripts,classes,lib,actions,locale,doc}'
tags: ['controller.service_arguments']

View File

@ -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) {

View File

@ -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),
];
}
}

View File

@ -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();

View File

@ -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);

View File

@ -1,6 +1,6 @@
<?php
declare(strict_types = 1);
declare(strict_types=1);
// {{{ License
// This file is part of GNU social - https://www.gnu.org/software/social
@ -21,13 +21,12 @@ declare(strict_types = 1);
namespace App\Security;
use function App\Core\I18n\_m;
use App\Core\Router\Router;
use App\Entity\LocalUser;
use App\Entity\User;
use App\Util\Common;
use App\Util\Exception\NoSuchActorException;
use App\Util\Exception\NotFoundException;
use App\Util\Exception\ServerException;
use App\Util\Nickname;
use Stringable;
use Symfony\Component\HttpFoundation\RedirectResponse;
@ -41,7 +40,9 @@ use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Security\Guard\Authenticator\AbstractFormLoginAuthenticator;
use Symfony\Component\Security\Guard\AuthenticatorInterface;
use Symfony\Component\Security\Http\Util\TargetPathTrait;
use function App\Core\I18n\_m;
/**
* User authenticator
@ -53,13 +54,13 @@ use Symfony\Component\Security\Http\Util\TargetPathTrait;
* @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 Authenticator extends AbstractFormLoginAuthenticator
class Authenticator extends AbstractFormLoginAuthenticator implements AuthenticatorInterface
{
use TargetPathTrait;
public const LOGIN_ROUTE = 'security_login';
private $csrfTokenManager;
private CsrfTokenManagerInterface $csrfTokenManager;
public function __construct(CsrfTokenManagerInterface $csrfTokenManager)
{
@ -67,9 +68,10 @@ class Authenticator extends AbstractFormLoginAuthenticator
}
/**
* @param Request $request
* @return bool
*/
public function supports(Request $request)
public function supports(Request $request): bool
{
return self::LOGIN_ROUTE === $request->attributes->get('_route') && $request->isMethod('POST');
}
@ -77,7 +79,7 @@ class Authenticator extends AbstractFormLoginAuthenticator
/**
* @return array<string, string>
*/
public function getCredentials(Request $request)
public function getCredentials(Request $request): array
{
return [
'nickname_or_email' => $request->request->get('nickname_or_email'),
@ -90,10 +92,12 @@ class Authenticator extends AbstractFormLoginAuthenticator
* Get a user given credentials and a CSRF token
*
* @param array<string, string> $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.'),
);
@ -121,8 +125,10 @@ class Authenticator extends AbstractFormLoginAuthenticator
/**
* @param array<string, string> $credentials result of self::getCredentials
* @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);
}

View File

@ -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['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();
}
}

View File

@ -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)) {

View File

@ -39,8 +39,9 @@
{# TODO: Login can be done with email, so the element id's should reflect that #}
<div class="form-group">
<label class="section-form-label" for="inputNickname">{{ "Nickname or Email" | trans }}</label>
<input type="text" value="{{ last_login_id }}" name="nickname_or_email" id="inputNickname" class="form-control" required autofocus>
<label class="section-form-label" for="inputNicknameOrEmail">{{ "Nickname or Email" | trans }}</label>
<input type="text" value="{{ last_login_id }}" name="nickname_or_email" id="inputNicknameOrEmail"
class="form-control" required autofocus>
<p class="help-text">{{ "Your nickname or email address." | trans }}</p>
</div>
<div class="form-group">
@ -50,14 +51,14 @@
</div>
<span class="checkbox mb-3">
<input type="checkbox" name="_remember_me">
<label>{{ "Remember me" | trans }}</label>
<label for="inputRememberMe">{{ "Remember me" | trans }}</label>
<input type="checkbox" name="_remember_me" id="inputRememberMe">
</span>
<input type="hidden" name="_csrf_token" value="{{ csrf_token('authenticate') }}">
<div>
<button class="btn btn-lg btn-primary" type="submit">Sign in</button>
</div>
<input type="hidden" name="_csrf_token" value="{{ csrf_token('authenticate') }}">
{% endif %}
</fieldset>