Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[make:auth] fix security controller attributes #985

Merged
merged 1 commit into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/Maker/MakeAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/makers.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
4 changes: 4 additions & 0 deletions src/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
36 changes: 34 additions & 2 deletions src/Security/SecurityControllerBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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'
Expand Down
96 changes: 74 additions & 22 deletions tests/Security/SecurityControllerBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down
Original file line number Diff line number Diff line change
@@ -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.');
}
}
Original file line number Diff line number Diff line change
@@ -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]);
}
}
Original file line number Diff line number Diff line change
@@ -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.');
}
}