From cc71c7a40eb6eec5040b4ecfd3a7210cfd5f7c66 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Fri, 1 Oct 2021 16:39:12 -0400 Subject: [PATCH] [make:auth] fix security controller attributes --- src/Maker/MakeAuthenticator.php | 11 ++- src/Resources/config/makers.xml | 1 + src/Resources/config/services.xml | 4 + src/Security/SecurityControllerBuilder.php | 36 ++++++- .../SecurityControllerBuilderTest.php | 96 ++++++++++++++----- .../expected/SecurityController_login.php | 4 +- .../SecurityController_login_logout.php | 8 +- .../expected/SecurityController_logout.php | 4 +- .../SecurityController_login_logout.php | 36 +++++++ .../SecurityController_login.php | 28 ++++++ .../SecurityController_logout.php | 17 ++++ 11 files changed, 205 insertions(+), 40 deletions(-) create mode 100644 tests/Security/fixtures/expected/legacy_add_login_logout_method/SecurityController_login_logout.php create mode 100644 tests/Security/fixtures/expected/legacy_add_login_method/SecurityController_login.php create mode 100644 tests/Security/fixtures/expected/legacy_add_logout_method/SecurityController_logout.php diff --git a/src/Maker/MakeAuthenticator.php b/src/Maker/MakeAuthenticator.php index dfcec2e2c..fc3c84014 100644 --- a/src/Maker/MakeAuthenticator.php +++ b/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()); diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index fdb3e9149..624a230e3 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -12,6 +12,7 @@ + diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 26af9d257..a61bf9e19 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -68,6 +68,10 @@ + + + + diff --git a/src/Security/SecurityControllerBuilder.php b/src/Security/SecurityControllerBuilder.php index 99be648fc..84abe0829 100644 --- a/src/Security/SecurityControllerBuilder.php +++ b/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' diff --git a/tests/Security/SecurityControllerBuilderTest.php b/tests/Security/SecurityControllerBuilderTest.php index 8ca761c80..92186eae9 100644 --- a/tests/Security/SecurityControllerBuilderTest.php +++ b/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); } } diff --git a/tests/Security/fixtures/expected/SecurityController_login.php b/tests/Security/fixtures/expected/SecurityController_login.php index 16608e401..6771f2c9c 100644 --- a/tests/Security/fixtures/expected/SecurityController_login.php +++ b/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()) { diff --git a/tests/Security/fixtures/expected/SecurityController_login_logout.php b/tests/Security/fixtures/expected/SecurityController_login_logout.php index 65c096f8f..15b59ced0 100644 --- a/tests/Security/fixtures/expected/SecurityController_login_logout.php +++ b/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.'); diff --git a/tests/Security/fixtures/expected/SecurityController_logout.php b/tests/Security/fixtures/expected/SecurityController_logout.php index 037a1539b..07fc8086c 100644 --- a/tests/Security/fixtures/expected/SecurityController_logout.php +++ b/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.'); diff --git a/tests/Security/fixtures/expected/legacy_add_login_logout_method/SecurityController_login_logout.php b/tests/Security/fixtures/expected/legacy_add_login_logout_method/SecurityController_login_logout.php new file mode 100644 index 000000000..65c096f8f --- /dev/null +++ b/tests/Security/fixtures/expected/legacy_add_login_logout_method/SecurityController_login_logout.php @@ -0,0 +1,36 @@ +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.'); + } +} diff --git a/tests/Security/fixtures/expected/legacy_add_login_method/SecurityController_login.php b/tests/Security/fixtures/expected/legacy_add_login_method/SecurityController_login.php new file mode 100644 index 000000000..16608e401 --- /dev/null +++ b/tests/Security/fixtures/expected/legacy_add_login_method/SecurityController_login.php @@ -0,0 +1,28 @@ +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]); + } +} diff --git a/tests/Security/fixtures/expected/legacy_add_logout_method/SecurityController_logout.php b/tests/Security/fixtures/expected/legacy_add_logout_method/SecurityController_logout.php new file mode 100644 index 000000000..037a1539b --- /dev/null +++ b/tests/Security/fixtures/expected/legacy_add_logout_method/SecurityController_logout.php @@ -0,0 +1,17 @@ +