Skip to content

Commit 52ceaab

Browse files
jrushlowjrushlow
committed
[make:auth] fix security controller attributes
1 parent 0f40c82 commit 52ceaab

File tree

11 files changed

+210
-40
lines changed

11 files changed

+210
-40
lines changed

‎src/Maker/MakeAuthenticator.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Symfony\Bundle\MakerBundle\Security\SecurityControllerBuilder;
2424
use Symfony\Bundle\MakerBundle\Str;
2525
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
26+
use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
2627
use Symfony\Bundle\MakerBundle\Util\YamlManipulationFailedException;
2728
use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator;
2829
use Symfony\Bundle\MakerBundle\Validator;
@@ -60,14 +61,17 @@ final class MakeAuthenticator extends AbstractMaker
6061

6162
private $doctrineHelper;
6263

64+
private $phpCompatUtil;
65+
6366
private $useSecurity52 = false;
6467

65-
public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater, Generator $generator, DoctrineHelper $doctrineHelper)
68+
public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater, Generator $generator, DoctrineHelper $doctrineHelper, PhpCompatUtil $phpCompatUtil)
6669
{
6770
$this->fileManager = $fileManager;
6871
$this->configUpdater = $configUpdater;
6972
$this->generator = $generator;
7073
$this->doctrineHelper = $doctrineHelper;
74+
$this->phpCompatUtil = $phpCompatUtil;
7175
}
7276

7377
public static function getCommandName(): string
@@ -323,7 +327,7 @@ private function generateFormLoginFiles(string $controllerClass, string $userNam
323327

324328
$manipulator = new ClassSourceManipulator($controllerSourceCode, true);
325329

326-
$securityControllerBuilder = new SecurityControllerBuilder();
330+
$securityControllerBuilder = new SecurityControllerBuilder($this->phpCompatUtil);
327331
$securityControllerBuilder->addLoginMethod($manipulator);
328332
if ($logoutSetup) {
329333
$securityControllerBuilder->addLogoutMethod($manipulator);

‎src/Resources/config/services.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@
6868
<argument type="service" id="maker.generator" />
6969
</service>
7070

71+
<service id="maker.security_controller_builder" class="Symfony\Bundle\MakerBundle\Security\SecurityControllerBuilder">
72+
<argument type="service" id="maker.php_compat_util" />
73+
<argument type="service" id="maker.template_component_generator" />
74+
</service>
75+
7176
<service id="maker.php_compat_util" class="Symfony\Bundle\MakerBundle\Util\PhpCompatUtil">
7277
<argument type="service" id="maker.file_manager" />
7378
</service>

‎src/Security/SecurityControllerBuilder.php

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bundle\MakerBundle\Security;
1313

1414
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
15+
use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\Routing\Annotation\Route;
1718
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
@@ -21,9 +22,27 @@
2122
*/
2223
final class SecurityControllerBuilder
2324
{
25+
private $phpCompatUtil;
26+
27+
public function __construct(PhpCompatUtil $phpCompatUtil)
28+
{
29+
$this->phpCompatUtil = $phpCompatUtil;
30+
}
31+
2432
public function addLoginMethod(ClassSourceManipulator $manipulator): void
2533
{
26-
$loginMethodBuilder = $manipulator->createMethodBuilder('login', 'Response', false, ['@Route("/login", name="app_login")']);
34+
$loginMethodBuilder = $manipulator->createMethodBuilder('login', 'Response', false);
35+
36+
if ($this->phpCompatUtil->canUseAttributes()) {
37+
$loginMethodBuilder->addAttribute($manipulator->buildAttributeNode('Route', ['/login', 'name' => 'app_login']));
38+
} else {
39+
$loginMethodBuilder->setDocComment(<<< 'EOT'
40+
/**
41+
* @Route("/login", name="app_login")
42+
*/
43+
EOT
44+
);
45+
}
2746

2847
$manipulator->addUseStatementIfNecessary(Response::class);
2948
$manipulator->addUseStatementIfNecessary(Route::class);
@@ -66,7 +85,18 @@ public function addLoginMethod(ClassSourceManipulator $manipulator): void
6685

6786
public function addLogoutMethod(ClassSourceManipulator $manipulator): void
6887
{
69-
$logoutMethodBuilder = $manipulator->createMethodBuilder('logout', 'void', false, ['@Route("/logout", name="app_logout")']);
88+
$logoutMethodBuilder = $manipulator->createMethodBuilder('logout', 'void', false, ['']);
89+
90+
if ($this->phpCompatUtil->canUseAttributes()) {
91+
$logoutMethodBuilder->addAttribute($manipulator->buildAttributeNode('Route', ['/logout', 'name' => 'app_logout']));
92+
} else {
93+
$logoutMethodBuilder->setDocComment(<<< 'EOT'
94+
/**
95+
* @Route("/logout", name="app_logout")
96+
*/
97+
EOT
98+
);
99+
}
70100

71101
$manipulator->addUseStatementIfNecessary(Route::class);
72102
$manipulator->addMethodBody($logoutMethodBuilder, <<<'CODE'

‎src/Util/ClassSourceManipulator.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,13 +1427,15 @@ private function buildNodeExprByValue($value): Node\Expr
14271427
* @param string $attributeClass the attribute class which should be used for the attribute
14281428
* @param array $options the named arguments for the attribute ($key = argument name, $value = argument value)
14291429
*/
1430-
private function buildAttributeNode(string $attributeClass, array $options): Node\Attribute
1430+
public function buildAttributeNode(string $attributeClass, array $options): Node\Attribute
14311431
{
14321432
$options = $this->sortOptionsByClassConstructorParameters($options, $attributeClass);
14331433

14341434
$context = $this;
14351435
$nodeArguments = array_map(static function ($option, $value) use ($context) {
1436-
return new Node\Arg($context->buildNodeExprByValue($value), false, false, [], new Node\Identifier($option));
1436+
$ident = \is_int($option) ? null : new Node\Identifier($option);
1437+
1438+
return new Node\Arg($context->buildNodeExprByValue($value), false, false, [], $ident);
14371439
}, array_keys($options), array_values($options));
14381440

14391441
return new Node\Attribute(
@@ -1450,6 +1452,10 @@ private function buildAttributeNode(string $attributeClass, array $options): Nod
14501452
*/
14511453
private function sortOptionsByClassConstructorParameters(array $options, string $classString): array
14521454
{
1455+
if (!class_exists($classString)) {
1456+
return $options;
1457+
}
1458+
14531459
if ('ORM\\' === substr($classString, 0, 4)) {
14541460
$classString = sprintf('Doctrine\\ORM\\Mapping\\%s', substr($classString, 4));
14551461
}

‎tests/Security/SecurityControllerBuilderTest.php

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,46 +14,98 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bundle\MakerBundle\Security\SecurityControllerBuilder;
1616
use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator;
17+
use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
1718

1819
class SecurityControllerBuilderTest extends TestCase
1920
{
20-
public function testAddLoginMethod()
21+
private $expectedBasePath = __DIR__.'/fixtures/expected';
22+
23+
public function testLoginMethod(): void
2124
{
22-
$source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
23-
$expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_login.php');
25+
/* @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
26+
$this->runMethodTest(
27+
'addLoginMethod',
28+
true,
29+
sprintf('%s/legacy_add_login_method/%s', $this->expectedBasePath, 'SecurityController_login.php')
30+
);
2431

25-
$manipulator = new ClassSourceManipulator($source);
32+
if ((\PHP_VERSION_ID >= 80000)) {
33+
$this->runMethodTest(
34+
'addLogoutMethod',
35+
false,
36+
sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_login.php')
37+
);
38+
}
39+
}
2640

27-
$securityControllerBuilder = new SecurityControllerBuilder();
28-
$securityControllerBuilder->addLoginMethod($manipulator);
41+
public function testLogoutMethod(): void
42+
{
43+
/* @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
44+
$this->runMethodTest(
45+
'addLogoutMethod',
46+
true,
47+
sprintf('%s/legacy_add_logout_method/%s', $this->expectedBasePath, 'SecurityController_logout.php')
48+
);
2949

30-
$this->assertSame($expectedSource, $manipulator->getSourceCode());
50+
if ((\PHP_VERSION_ID >= 80000)) {
51+
$this->runMethodTest(
52+
'addLogoutMethod',
53+
false,
54+
sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_logout.php')
55+
);
56+
}
3157
}
3258

33-
public function testLogoutMethod()
59+
public function testLoginAndLogoutMethod(): void
3460
{
35-
$source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
36-
$expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_logout.php');
61+
/** @legacy Can be dropped when PHP 7.x support is dropped in MakerBundle */
62+
$builder = $this->getSecurityControllerBuilder(true);
63+
$csm = $this->getClassSourceManipulator();
64+
65+
$builder->addLoginMethod($csm);
66+
$builder->addLogoutMethod($csm);
67+
68+
$this->assertStringEqualsFile(
69+
sprintf('%s/legacy_add_login_logout_method/%s', $this->expectedBasePath, 'SecurityController_login_logout.php'),
70+
$csm->getSourceCode()
71+
);
3772

38-
$manipulator = new ClassSourceManipulator($source);
73+
if ((\PHP_VERSION_ID >= 80000)) {
74+
$builder = $this->getSecurityControllerBuilder(false);
75+
$csm = $this->getClassSourceManipulator();
3976

40-
$securityControllerBuilder = new SecurityControllerBuilder();
41-
$securityControllerBuilder->addLogoutMethod($manipulator);
77+
$builder->addLoginMethod($csm);
78+
$builder->addLogoutMethod($csm);
4279

43-
$this->assertSame($expectedSource, $manipulator->getSourceCode());
80+
$this->assertStringEqualsFile(
81+
sprintf('%s/%s', $this->expectedBasePath, 'SecurityController_login_logout.php'),
82+
$csm->getSourceCode()
83+
);
84+
}
4485
}
4586

46-
public function testLoginAndLogoutMethod()
87+
private function runMethodTest(string $builderMethod, bool $isLegacyTest, string $expectedFilePath): void
4788
{
48-
$source = file_get_contents(__DIR__.'/fixtures/source/SecurityController.php');
49-
$expectedSource = file_get_contents(__DIR__.'/fixtures/expected/SecurityController_login_logout.php');
89+
$builder = $this->getSecurityControllerBuilder($isLegacyTest);
90+
$csm = $this->getClassSourceManipulator();
5091

51-
$manipulator = new ClassSourceManipulator($source);
92+
$builder->$builderMethod($csm);
93+
$this->assertStringEqualsFile($expectedFilePath, $csm->getSourceCode());
94+
}
5295

53-
$securityControllerBuilder = new SecurityControllerBuilder();
54-
$securityControllerBuilder->addLoginMethod($manipulator);
55-
$securityControllerBuilder->addLogoutMethod($manipulator);
96+
private function getClassSourceManipulator(): ClassSourceManipulator
97+
{
98+
return new ClassSourceManipulator(file_get_contents(__DIR__.'/fixtures/source/SecurityController.php'));
99+
}
100+
101+
private function getSecurityControllerBuilder(bool $isLegacyTest): SecurityControllerBuilder
102+
{
103+
$compatUtil = $this->createMock(PhpCompatUtil::class);
104+
$compatUtil
105+
->method('canUseAttributes')
106+
->willReturn(!$isLegacyTest)
107+
;
56108

57-
$this->assertSame($expectedSource, $manipulator->getSourceCode());
109+
return new SecurityControllerBuilder($compatUtil);
58110
}
59111
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99

1010
class SecurityController extends AbstractController
1111
{
12-
/**
13-
* @Route("/login", name="app_login")
14-
*/
12+
#[Route('/login', name: 'app_login')]
1513
public function login(AuthenticationUtils $authenticationUtils): Response
1614
{
1715
// if ($this->getUser()) {

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99

1010
class SecurityController extends AbstractController
1111
{
12-
/**
13-
* @Route("/login", name="app_login")
14-
*/
12+
#[Route('/login', name: 'app_login')]
1513
public function login(AuthenticationUtils $authenticationUtils): Response
1614
{
1715
// if ($this->getUser()) {
@@ -26,9 +24,7 @@ public function login(AuthenticationUtils $authenticationUtils): Response
2624
return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
2725
}
2826

29-
/**
30-
* @Route("/logout", name="app_logout")
31-
*/
27+
#[Route('/logout', name: 'app_logout')]
3228
public function logout(): void
3329
{
3430
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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77

88
class SecurityController extends AbstractController
99
{
10-
/**
11-
* @Route("/logout", name="app_logout")
12-
*/
10+
#[Route('/logout', name: 'app_logout')]
1311
public function logout(): void
1412
{
1513
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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
namespace App\Controller;
4+
5+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
6+
use Symfony\Component\HttpFoundation\Response;
7+
use Symfony\Component\Routing\Annotation\Route;
8+
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
9+
10+
class SecurityController extends AbstractController
11+
{
12+
/**
13+
* @Route("/login", name="app_login")
14+
*/
15+
public function login(AuthenticationUtils $authenticationUtils): Response
16+
{
17+
// if ($this->getUser()) {
18+
// return $this->redirectToRoute('target_path');
19+
// }
20+
21+
// get the login error if there is one
22+
$error = $authenticationUtils->getLastAuthenticationError();
23+
// last username entered by the user
24+
$lastUsername = $authenticationUtils->getLastUsername();
25+
26+
return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
27+
}
28+
29+
/**
30+
* @Route("/logout", name="app_logout")
31+
*/
32+
public function logout(): void
33+
{
34+
throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');
35+
}
36+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
namespace App\Controller;
4+
5+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
6+
use Symfony\Component\HttpFoundation\Response;
7+
use Symfony\Component\Routing\Annotation\Route;
8+
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
9+
10+
class SecurityController extends AbstractController
11+
{
12+
/**
13+
* @Route("/login", name="app_login")
14+
*/
15+
public function login(AuthenticationUtils $authenticationUtils): Response
16+
{
17+
// if ($this->getUser()) {
18+
// return $this->redirectToRoute('target_path');
19+
// }
20+
21+
// get the login error if there is one
22+
$error = $authenticationUtils->getLastAuthenticationError();
23+
// last username entered by the user
24+
$lastUsername = $authenticationUtils->getLastUsername();
25+
26+
return $this->render('security/login.html.twig', ['last_username' => $lastUsername, 'error' => $error]);
27+
}
28+
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace App\Controller;
4+
5+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
6+
use Symfony\Component\Routing\Annotation\Route;
7+
8+
class SecurityController extends AbstractController
9+
{
10+
/**
11+
* @Route("/logout", name="app_logout")
12+
*/
13+
public function logout(): void
14+
{
15+
throw new \LogicException('This method can be blank - it will be intercepted by the logout key on your firewall.');
16+
}
17+
}

0 commit comments

Comments
 (0)