Fix login error

Fixed ticket #183
nucleos · Jan 8, 2021 · 481a838 · 481a838
2 parents abe925c + 7c1405a
commit 481a838
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 61 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1,7 +1,7 @@
 parameters:
 	ignoreErrors:
 		-
-			message: "#^Call to method Symfony\\\\Component\\\\HttpFoundation\\\\Request\\:\\:hasSession\\(\\) will always evaluate to true\\.$#"
+			message: "#^Call to an undefined method Symfony\\\\Component\\\\Form\\\\FormError\\|Symfony\\\\Component\\\\Form\\\\FormErrorIterator\\:\\:getMessage\\(\\)\\.$#"
 			count: 1
 			path: src/Action/LoginAction.php
 

diff --git a/src/Action/LoginAction.php b/src/Action/LoginAction.php
@@ -16,14 +16,11 @@
 use Nucleos\UserBundle\Event\GetResponseLoginEvent;
 use Nucleos\UserBundle\Form\Type\LoginFormType;
 use Nucleos\UserBundle\NucleosUserEvents;
-use Symfony\Component\Form\FormError;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Routing\RouterInterface;
-use Symfony\Component\Security\Core\Exception\AuthenticationException;
-use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 use Twig\Environment;
 
@@ -49,16 +46,23 @@ final class LoginAction
      */
     private $router;
 
+    /**
+     * @var CsrfTokenManagerInterface
+     */
+    private $csrfTokenManager;
+
     public function __construct(
         Environment $twig,
         EventDispatcherInterface $eventDispatcher,
         FormFactoryInterface $formFactory,
-        RouterInterface $router
+        RouterInterface $router,
+        CsrfTokenManagerInterface $csrfTokenManager
     ) {
-        $this->twig            = $twig;
-        $this->eventDispatcher = $eventDispatcher;
-        $this->formFactory     = $formFactory;
-        $this->router          = $router;
+        $this->twig             = $twig;
+        $this->eventDispatcher  = $eventDispatcher;
+        $this->formFactory      = $formFactory;
+        $this->router           = $router;
+        $this->csrfTokenManager = $csrfTokenManager;
     }
 
     /**
@@ -73,46 +77,22 @@ public function __invoke(Request $request): Response
             return $event->getResponse();
         }
 
-        $session = $this->getSession($request);
-
-        $authErrorKey    = Security::AUTHENTICATION_ERROR;
-        $lastUsernameKey = Security::LAST_USERNAME;
-
-        // get the error if any (works with forward and redirect -- see below)
-        if ($request->attributes->has($authErrorKey)) {
-            $error = $request->attributes->get($authErrorKey);
-        } elseif (null !== $session && $session->has($authErrorKey)) {
-            $error = $session->get($authErrorKey);
-            $session->remove($authErrorKey);
-        } else {
-            $error = null;
-        }
-
         $form = $this->formFactory->create(LoginFormType::class, null, [
             'action' => $this->router->generate('nucleos_user_security_check'),
             'method' => 'POST',
         ]);
 
-        if (!$error instanceof AuthenticationException) {
-            $error = null; // The value does not come from the security component.
-        } else {
-            $form->addError(new FormError($error->getMessageKey(), null, $error->getMessageData()));
+        $error = null;
+        if ($form->getErrors()->count() > 0) {
+            $error = $form->getErrors()->current()->getMessage();
         }
 
-        // last username entered by the user
-        $lastUsername = (null === $session) ? '' : $session->get($lastUsernameKey);
-
         return new Response($this->twig->render('@NucleosUser/Security/login.html.twig', [
-            'last_username' => $lastUsername,
             'form'          => $form->createView(),
             // TODO: Remove this fields with the next major release
+            'last_username' => $form->getData()['_username'],
             'error'         => $error,
-            'csrf_token'    => '',
+            'csrf_token'    => $this->csrfTokenManager->getToken('authenticate'),
         ]));
     }
-
-    private function getSession(Request $request): ?SessionInterface
-    {
-        return $request->hasSession() ? $request->getSession() : null;
-    }
 }
diff --git a/src/Form/Type/LoginFormType.php b/src/Form/Type/LoginFormType.php
@@ -21,20 +21,19 @@
 use Symfony\Component\Form\FormError;
 use Symfony\Component\Form\FormEvent;
 use Symfony\Component\Form\FormEvents;
-use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\OptionsResolver\OptionsResolver;
-use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
 
 final class LoginFormType extends AbstractType
 {
     /**
-     * @var RequestStack
+     * @var AuthenticationUtils
      */
-    private $requestStack;
+    private $authenticationUtils;
 
-    public function __construct(RequestStack $requestStack)
+    public function __construct(AuthenticationUtils $authenticationUtils)
     {
-        $this->requestStack = $requestStack;
+        $this->authenticationUtils = $authenticationUtils;
     }
 
     public function buildForm(FormBuilderInterface $builder, array $options): void
@@ -63,27 +62,16 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
             ])
         ;
 
-        $request = $this->requestStack->getCurrentRequest();
-
-        $builder->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event) use ($request): void {
-            if (null === $request) {
-                return;
-            }
-
-            $error = null;
-
-            if ($request->attributes->has(Security::AUTHENTICATION_ERROR)) {
-                $error = $request->attributes->get(Security::AUTHENTICATION_ERROR);
-            } else {
-                $error = $request->getSession()->get(Security::AUTHENTICATION_ERROR);
-            }
+        $authenticationUtils = $this->authenticationUtils;
 
+        $builder->addEventListener(FormEvents::PRE_SET_DATA, static function (FormEvent $event) use ($authenticationUtils): void {
+            $error = $authenticationUtils->getLastAuthenticationError();
             if (null !== $error) {
                 $event->getForm()->addError(new FormError($error->getMessage()));
             }
 
             $event->setData(array_replace((array) $event->getData(), [
-                'username' => $request->getSession()->get(Security::LAST_USERNAME),
+                'username' => $authenticationUtils->getLastUsername(),
             ]));
         });
     }
@@ -92,6 +80,9 @@ public function configureOptions(OptionsResolver $resolver): void
     {
         $resolver->setDefaults([
             'translation_domain' => 'NucleosUserBundle',
+
+            'csrf_field_name' => '_csrf_token',
+            'csrf_token_id'   => 'authenticate',
         ]);
     }
 

diff --git a/src/Resources/config/security.php b/src/Resources/config/security.php
@@ -62,7 +62,7 @@
         ->set(LoginFormType::class)
             ->tag('form.type')
             ->args([
-                new Reference('request_stack'),
+                new Reference('security.authentication_utils'),
             ])
 
         ->set(LoginAction::class)
@@ -72,6 +72,7 @@
                 new Reference('event_dispatcher'),
                 new Reference('form.factory'),
                 new Reference('router'),
+                new Reference('security.csrf.token_manager'),
             ])
 
         ->set(LogoutAction::class)