bug #985 [make:auth] fix security controller attributes (jrushlow)

This PR was merged into the 1.0-dev branch.

Discussion
----------

[make:auth] fix security controller attributes

fixes #981

Commits
-------

cc71c7a [make:auth] fix security controller attributes

src/Maker/MakeAuthenticator.php

@@ -60,14 +60,17 @@ final class MakeAuthenticator extends AbstractMaker
 
     private $doctrineHelper;
 
+    private $securityControllerBuilder;
+
     private $useSecurity52 = false;
 
-    public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater, Generator $generator, DoctrineHelper $doctrineHelper)
+    public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater, Generator $generator, DoctrineHelper $doctrineHelper, SecurityControllerBuilder $securityControllerBuilder)
     {
         $this->fileManager = $fileManager;
         $this->configUpdater = $configUpdater;
         $this->generator = $generator;
         $this->doctrineHelper = $doctrineHelper;
+        $this->securityControllerBuilder = $securityControllerBuilder;
     }
 
     public static function getCommandName(): string
@@ -323,10 +326,10 @@ private function generateFormLoginFiles(string $controllerClass, string $userNam
 
         $manipulator = new ClassSourceManipulator($controllerSourceCode, true);
 
-        $securityControllerBuilder = new SecurityControllerBuilder();
-        $securityControllerBuilder->addLoginMethod($manipulator);
+        $this->securityControllerBuilder->addLoginMethod($manipulator);
+
         if ($logoutSetup) {
-            $securityControllerBuilder->addLogoutMethod($manipulator);
+            $this->securityControllerBuilder->addLogoutMethod($manipulator);
         }
 
         $this->generator->dumpFile($controllerPath, $manipulator->getSourceCode());

src/Resources/config/makers.xml

@@ -12,6 +12,7 @@
                 <argument type="service" id="maker.security_config_updater" />
                 <argument type="service" id="maker.generator" />
                 <argument type="service" id="maker.doctrine_helper" />
+                <argument type="service" id="maker.security_controller_builder" />
                 <tag name="maker.command" />
             </service>
 

src/Resources/config/services.xml

@@ -68,6 +68,10 @@
                 <argument type="service" id="maker.generator" />
             </service>
 
+            <service id="maker.security_controller_builder" class="Symfony\Bundle\MakerBundle\Security\SecurityControllerBuilder">
+                <argument type="service" id="maker.php_compat_util" />
+            </service>
+
             <service id="maker.php_compat_util" class="Symfony\Bundle\MakerBundle\Util\PhpCompatUtil">
                 <argument type="service" id="maker.file_manager" />
             </service>

src/Security/SecurityControllerBuilder.php

@@ -12,6 +12,7 @@
 namespace Symfony\Bundle\MakerBundle\Security;
 
 use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
+use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
 use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
@@ -21,9 +22,28 @@
  */
 final class SecurityControllerBuilder
 {
+    private $phpCompatUtil;
+
+    public function __construct(PhpCompatUtil $phpCompatUtil)
+    {
+        $this->phpCompatUtil = $phpCompatUtil;
+    }
+
     public function addLoginMethod(ClassSourceManipulator $manipulator): void
     {
-        $loginMethodBuilder = $manipulator->createMethodBuilder('login', 'Response', false, ['@Route("/login", name="app_login")']);
+        $loginMethodBuilder = $manipulator->createMethodBuilder('login', 'Response', false);
+
+        // @legacy Refactor when annotations are no longer supported
+        if ($this->phpCompatUtil->canUseAttributes()) {
+            $loginMethodBuilder->addAttribute($manipulator->buildAttributeNode(Route::class, ['path' => '/login', 'name' => 'app_login']));
+        } else {
+            $loginMethodBuilder->setDocComment(<<< 'EOT'
+/**
+ * @Route("/login", name="app_login")
+ */
+EOT
+            );
+        }
 
         $manipulator->addUseStatementIfNecessary(Response::class);
         $manipulator->addUseStatementIfNecessary(Route::class);
@@ -66,7 +86,19 @@ public function addLoginMethod(ClassSourceManipulator $manipulator): void
 
     public function addLogoutMethod(ClassSourceManipulator $manipulator): void
     {
-        $logoutMethodBuilder = $manipulator->createMethodBuilder('logout', 'void', false, ['@Route("/logout", name="app_logout")']);
+        $logoutMethodBuilder = $manipulator->createMethodBuilder('logout', 'void', false);
+
+        // @legacy Refactor when annotations are no longer supported
+        if ($this->phpCompatUtil->canUseAttributes()) {
+            $logoutMethodBuilder->addAttribute($manipulator->buildAttributeNode(Route::class, ['path' => '/logout', 'name' => 'app_logout']));
+        } else {
+            $logoutMethodBuilder->setDocComment(<<< 'EOT'
+/**
+ * @Route("/logout", name="app_logout")
+ */
+EOT
+            );
+        }
 
         $manipulator->addUseStatementIfNecessary(Route::class);
         $manipulator->addMethodBody($logoutMethodBuilder, <<<'CODE'

tests/Security/SecurityControllerBuilderTest.php

@@ -14,46 +14,98 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Bundle\MakerBundle\Security\SecurityControllerBuilder;
 use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
+use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
 
 class SecurityControllerBuilderTest extends TestCase
 {
-    public function testAddLoginMethod()
+    private $expectedBasePath = __DIR__.'/fixtures/expected';
+
+    public function testLoginMethod(): void
     {
-        $source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
-        $expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_login.php');
+        /* @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
+        $this->runMethodTest(
+            'addLoginMethod',
+            true,
+            sprintf('%s/legacy_add_login_method/%s', $this->expectedBasePath, 'SecurityController_login.php')
+        );
 
-        $manipulator = new ClassSourceManipulator($source);
+        if ((\PHP_VERSION_ID >= 80000)) {
+            $this->runMethodTest(
+                'addLoginMethod',
+                false,
+                sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_login.php')
+            );
+        }
+    }
 
-        $securityControllerBuilder = new SecurityControllerBuilder();
-        $securityControllerBuilder->addLoginMethod($manipulator);
+    public function testLogoutMethod(): void
+    {
+        /* @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
+        $this->runMethodTest(
+            'addLogoutMethod',
+            true,
+            sprintf('%s/legacy_add_logout_method/%s', $this->expectedBasePath, 'SecurityController_logout.php')
+        );
 
-        $this->assertSame($expectedSource, $manipulator->getSourceCode());
+        if ((\PHP_VERSION_ID >= 80000)) {
+            $this->runMethodTest(
+                'addLogoutMethod',
+                false,
+                sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_logout.php')
+            );
+        }
     }
 
-    public function testLogoutMethod()
+    public function testLoginAndLogoutMethod(): void
     {
-        $source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
-        $expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_logout.php');
+        /** @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
+        $builder = $this->getSecurityControllerBuilder(true);
+        $csm = $this->getClassSourceManipulator();
+
+        $builder->addLoginMethod($csm);
+        $builder->addLogoutMethod($csm);
+
+        $this->assertStringEqualsFile(
+            sprintf('%s/legacy_add_login_logout_method/%s', $this->expectedBasePath, 'SecurityController_login_logout.php'),
+            $csm->getSourceCode()
+        );
 
-        $manipulator = new ClassSourceManipulator($source);
+        if ((\PHP_VERSION_ID >= 80000)) {
+            $builder = $this->getSecurityControllerBuilder(false);
+            $csm = $this->getClassSourceManipulator();
 
-        $securityControllerBuilder = new SecurityControllerBuilder();
-        $securityControllerBuilder->addLogoutMethod($manipulator);
+            $builder->addLoginMethod($csm);
+            $builder->addLogoutMethod($csm);
 
-        $this->assertSame($expectedSource, $manipulator->getSourceCode());
+            $this->assertStringEqualsFile(
+                sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_login_logout.php'),
+                $csm->getSourceCode()
+            );
+        }
     }
 
-    public function testLoginAndLogoutMethod()
+    private function runMethodTest(string $builderMethod, bool $isLegacyTest, string $expectedFilePath): void
     {
-        $source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
-        $expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_login_logout.php');
+        $builder = $this->getSecurityControllerBuilder($isLegacyTest);
+        $csm = $this->getClassSourceManipulator();
 
-        $manipulator = new ClassSourceManipulator($source);
+        $builder->$builderMethod($csm);
+        $this->assertStringEqualsFile($expectedFilePath, $csm->getSourceCode());
+    }
 
-        $securityControllerBuilder = new SecurityControllerBuilder();
-        $securityControllerBuilder->addLoginMethod($manipulator);
-        $securityControllerBuilder->addLogoutMethod($manipulator);
+    private function getClassSourceManipulator(): ClassSourceManipulator
+    {
+        return new ClassSourceManipulator(file_get_contents(__DIR__.'/fixtures/source/SecurityController.php'));
+    }
+
+    private function getSecurityControllerBuilder(bool $isLegacyTest): SecurityControllerBuilder
+    {
+        $compatUtil = $this->createMock(PhpCompatUtil::class);
+        $compatUtil
+            ->method('canUseAttributes')
+            ->willReturn(!$isLegacyTest)
+        ;
 
-        $this->assertSame($expectedSource, $manipulator->getSourceCode());
+        return new SecurityControllerBuilder($compatUtil);
     }
 }

tests/Security/fixtures/expected/SecurityController_login.php

@@ -9,9 +9,7 @@
 
 class SecurityController extends AbstractController
 {
-    /**
-     * @Route("/login", name="app_login")
-     */
+    #[Route(path: '/login', name: 'app_login')]
     public function login(AuthenticationUtils $authenticationUtils): Response
     {
         // if ($this->getUser()) {

tests/Security/fixtures/expected/SecurityController_login_logout.php

@@ -9,9 +9,7 @@
 
 class SecurityController extends AbstractController
 {
-    /**
-     * @Route("/login", name="app_login")
-     */
+    #[Route(path: '/login', name: 'app_login')]
     public function login(AuthenticationUtils $authenticationUtils): Response
     {
         // if ($this->getUser()) {
@@ -26,9 +24,7 @@ public function login(AuthenticationUtils $authenticationUtils): Response
         return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
     }
 
-    /**
-     * @Route("/logout", name="app_logout")
-     */
+    #[Route(path: '/logout', name: 'app_logout')]
     public function logout(): void
     {
         throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');

tests/Security/fixtures/expected/SecurityController_logout.php

@@ -7,9 +7,7 @@
 
 class SecurityController extends AbstractController
 {
-    /**
-     * @Route("/logout", name="app_logout")
-     */
+    #[Route(path: '/logout', name: 'app_logout')]
     public function logout(): void
     {
         throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');

tests/Security/fixtures/expected/legacy_add_login_logout_method/SecurityController_login_logout.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace App\Controller;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
+
+class SecurityController extends AbstractController
+{
+    /**
+     * @Route("/login", name="app_login")
+     */
+    public function login(AuthenticationUtils $authenticationUtils): Response
+    {
+        // if ($this->getUser()) {
+        //     return $this->redirectToRoute('target_path');
+        // }
+
+        // get the login error if there is one
+        $error = $authenticationUtils->getLastAuthenticationError();
+        // last username entered by the user
+        $lastUsername = $authenticationUtils->getLastUsername();
+
+        return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
+    }
+
+    /**
+     * @Route("/logout", name="app_logout")
+     */
+    public function logout(): void
+    {
+        throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');
+    }
+}

tests/Security/fixtures/expected/legacy_add_login_method/SecurityController_login.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace App\Controller;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
+
+class SecurityController extends AbstractController
+{
+    /**
+     * @Route("/login", name="app_login")
+     */
+    public function login(AuthenticationUtils $authenticationUtils): Response
+    {
+        // if ($this->getUser()) {
+        //     return $this->redirectToRoute('target_path');
+        // }
+
+        // get the login error if there is one
+        $error = $authenticationUtils->getLastAuthenticationError();
+        // last username entered by the user
+        $lastUsername = $authenticationUtils->getLastUsername();
+
+        return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
+    }
+}

tests/Security/fixtures/expected/legacy_add_logout_method/SecurityController_logout.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace App\Controller;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+class SecurityController extends AbstractController
+{
+    /**
+     * @Route("/logout", name="app_logout")
+     */
+    public function logout(): void
+    {
+        throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');
+    }
+}