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.

Sign up for GitHub

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

Jump to bottom

[make:registration] drop guard authentication support #1243

Merged
merged 9 commits into from Mar 4, 2024
Merged

[make:registration] drop guard authentication support #1243

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
977835c
[make:registration] drop guard authentication support
jrushlow Nov 28, 2022
7c25f0b
words mean everything
jrushlow Feb 27, 2024
13348d5
return auth response instead
jrushlow Feb 27, 2024
340ab01
yup - we dont need that anymore
jrushlow Feb 27, 2024
824451f
manage our expectations
jrushlow Feb 28, 2024
ef75722
lets see how many pass with this
jrushlow Feb 28, 2024
97fcbb1
use built in authenticators for login
jrushlow Feb 28, 2024
caaae50
improve help readability
jrushlow Mar 1, 2024
0d96a67
these vars on constant
jrushlow Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 32 additions & 33 deletions src/Maker/MakeRegistrationForm.php
View file
Open in desktop
Expand Up @@ -26,6 +26,8 @@
use Symfony\Bundle\MakerBundle\InputConfiguration;
use Symfony\Bundle\MakerBundle\Renderer\FormTypeRenderer;
use Symfony\Bundle\MakerBundle\Security\InteractiveSecurityHelper;
use Symfony\Bundle\MakerBundle\Security\Model\Authenticator;
use Symfony\Bundle\MakerBundle\Security\Model\AuthenticatorType;
use Symfony\Bundle\MakerBundle\Str;
use Symfony\Bundle\MakerBundle\Util\ClassDetails;
use Symfony\Bundle\MakerBundle\Util\ClassNameDetails;
Expand All @@ -34,6 +36,7 @@
use Symfony\Bundle\MakerBundle\Util\UseStatementGenerator;
use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator;
use Symfony\Bundle\MakerBundle\Validator;
use Symfony\Bundle\SecurityBundle\Security;
use Symfony\Bundle\SecurityBundle\SecurityBundle;
use Symfony\Bundle\TwigBundle\TwigBundle;
use Symfony\Component\Console\Command\Command;
Expand All @@ -49,7 +52,6 @@
use Symfony\Component\Routing\Attribute\Route;
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Authentication\UserAuthenticatorInterface;
use Symfony\Component\Translation\Translator;
use Symfony\Component\Validator\Validation;
use Symfony\Contracts\Translation\TranslatorInterface;
Expand All @@ -75,8 +77,7 @@ final class MakeRegistrationForm extends AbstractMaker
private $emailGetter;
private $fromEmailAddress;
private $fromEmailName;
private $autoLoginAuthenticator;
private $firewallName;
private ?Authenticator $autoLoginAuthenticator = null;
private $redirectRouteName;
private $addUniqueEntityConstraint;

Expand Down Expand Up @@ -110,7 +111,7 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
$interactiveSecurityHelper = new InteractiveSecurityHelper();

if (null === $this->router) {
throw new RuntimeCommandException('Router have been explicitely disabled in your configuration. This command needs to use the router.');
throw new RuntimeCommandException('Router have been explicitly disabled in your configuration. This command needs to use the router.');
}

if (!$this->fileManager->fileExists($path = 'config/packages/security.yaml')) {
Expand Down Expand Up @@ -184,32 +185,20 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma

private function interactAuthenticatorQuestions(ConsoleStyle $io, InteractiveSecurityHelper $interactiveSecurityHelper, array $securityData): void
{
$firewallsData = $securityData['security']['firewalls'] ?? [];
$firewallName = $interactiveSecurityHelper->guessFirewallName(
$io,
$securityData,
'Which firewall key in security.yaml holds the authenticator you want to use for logging in?'
);
// get list of authenticators
$authenticators = $interactiveSecurityHelper->getAuthenticatorsFromConfig($securityData['security']['firewalls'] ?? []);

if (!isset($firewallsData[$firewallName])) {
$io->note('No firewalls found - skipping authentication after registration. You might want to configure your security before running this command.');
if (empty($authenticators)) {
$io->note('No authenticators found - so your user won\'t be automatically authenticated after registering.');

return;
}

$this->firewallName = $firewallName;

// get list of guard authenticators
$authenticatorClasses = $interactiveSecurityHelper->getAuthenticatorClasses($firewallsData[$firewallName]);
if (empty($authenticatorClasses)) {
$io->note('No Guard authenticators found - so your user won\'t be automatically authenticated after registering.');
} else {
$this->autoLoginAuthenticator =
1 === \count($authenticatorClasses) ? $authenticatorClasses[0] : $io->choice(
'Which authenticator\'s onAuthenticationSuccess() should be used after logging in?',
$authenticatorClasses
);
}
$this->autoLoginAuthenticator =
1 === \count($authenticators) ? $authenticators[0] : $io->choice(
'Which authenticator should be used to login the user?',
$authenticators
);
}

public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void
Expand Down Expand Up @@ -312,11 +301,22 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
}
}

if ($this->autoLoginAuthenticator) {
$autoLoginVars = [
'login_after_registration' => null !== $this->autoLoginAuthenticator,
];

if (null !== $this->autoLoginAuthenticator) {
$useStatements->addUseStatement([
$this->autoLoginAuthenticator,
UserAuthenticatorInterface::class,
Security::class,
]);

$autoLoginVars['firewall'] = $this->autoLoginAuthenticator->firewallName;
$autoLoginVars['authenticator'] = sprintf('\'%s\'', $this->autoLoginAuthenticator->type->value);

if (AuthenticatorType::CUSTOM === $this->autoLoginAuthenticator->type) {
$useStatements->addUseStatement($this->autoLoginAuthenticator->authenticatorClass);
$autoLoginVars['authenticator'] = sprintf('%s::class', Str::getShortClassName($this->autoLoginAuthenticator->authenticatorClass));
}
}

if ($isTranslatorAvailable = class_exists(Translator::class)) {
Expand All @@ -339,14 +339,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
'from_email' => $this->fromEmailAddress,
'from_email_name' => addslashes($this->fromEmailName),
'email_getter' => $this->emailGetter,
'authenticator_class_name' => $this->autoLoginAuthenticator ? Str::getShortClassName($this->autoLoginAuthenticator) : null,
'authenticator_full_class_name' => $this->autoLoginAuthenticator,
'firewall_name' => $this->firewallName,
'redirect_route_name' => $this->redirectRouteName,
'password_hasher_class_details' => $generator->createClassNameDetails(UserPasswordHasherInterface::class, '\\'),
'password_hasher_class_details' => $hasherDetails = $generator->createClassNameDetails(UserPasswordHasherInterface::class, '\\'),
'password_hasher_variable_name' => sprintf('$%s', lcfirst($hasherDetails->getShortName())),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this stuff still need to be dynamic?

Copy link
Collaborator Author

@jrushlow jrushlow Mar 1, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno, I thought it was a good idea 2 years ago :D I'll check haha If it doesn't - ill refactor the template before hitting the big ol merge button nope, i think i was going for "if they use another hasher interface..." but thats their problem to change the hasher signature here...

done

'translator_available' => $isTranslatorAvailable,
],
$userRepoVars
$userRepoVars,
$autoLoginVars,
)
);

Expand Down
13 changes: 5 additions & 8 deletions src/Resources/skeleton/registration/RegistrationController.tpl.php
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@ public function __construct(<?= $email_verifier_class_details->getShortName() ?>

<?php endif; ?>
<?= $generator->generateRouteForControllerMethod($route_path, $route_name) ?>
public function register(Request $request, <?= $password_hasher_class_details->getShortName() ?> $userPasswordHasher<?= $authenticator_full_class_name ? sprintf(', UserAuthenticatorInterface $userAuthenticator, %s $authenticator', $authenticator_class_name) : '' ?>, EntityManagerInterface $entityManager): Response
public function register(Request $request, <?= $password_hasher_class_details->getShortName() ?> <?= $password_hasher_variable_name ?><?= $login_after_registration ? ', Security $security': '' ?>, EntityManagerInterface $entityManager): Response
{
$user = new <?= $user_class_name ?>();
$form = $this->createForm(<?= $form_class_name ?>::class, $user);
Expand All @@ -25,7 +25,7 @@ public function register(Request $request, <?= $password_hasher_class_details->g
if ($form->isSubmitted() && $form->isValid()) {
// encode the plain password
$user->set<?= ucfirst($password_field) ?>(
$userPasswordHasher->hashPassword(
<?= $password_hasher_variable_name ?>->hashPassword(
$user,
$form->get('plainPassword')->getData()
)
Expand All @@ -44,14 +44,11 @@ public function register(Request $request, <?= $password_hasher_class_details->g
->htmlTemplate('registration/confirmation_email.html.twig')
);
<?php endif; ?>

// do anything else you need here, like send an email

<?php if ($authenticator_full_class_name): ?>
return $userAuthenticator->authenticateUser(
$user,
$authenticator,
$request
);
<?php if ($login_after_registration): ?>
return $security->login($user, <?= $authenticator ?>, '<?= $firewall ?>');
<?php else: ?>
return $this->redirectToRoute('<?= $redirect_route_name ?>');
<?php endif; ?>
Expand Down
58 changes: 48 additions & 10 deletions src/Security/InteractiveSecurityHelper.php
View file
Open in desktop
Expand Up @@ -11,10 +11,13 @@

namespace Symfony\Bundle\MakerBundle\Security;

use Symfony\Bundle\MakerBundle\Security\Model\Authenticator;
use Symfony\Bundle\MakerBundle\Security\Model\AuthenticatorType;
use Symfony\Bundle\MakerBundle\Str;
use Symfony\Bundle\MakerBundle\Validator;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;

/**
* @internal
Expand Down Expand Up @@ -140,22 +143,57 @@ public function guessPasswordField(SymfonyStyle $io, string $userClass): string
);
}

public function getAuthenticatorClasses(array $firewallData): array
/**
* @param array<string, array<string, mixed>> $firewalls Config data from security.firewalls
*
* @return Authenticator[]
*/
public function getAuthenticatorsFromConfig(array $firewalls): array
{
if (isset($firewallData['guard'])) {
return array_filter($firewallData['guard']['authenticators'] ?? [], static fn ($authenticator) => class_exists($authenticator));
}
$authenticators = [];

if (isset($firewallData['custom_authenticator'])) {
$authenticators = $firewallData['custom_authenticator'];
if (\is_string($authenticators)) {
$authenticators = [$authenticators];
// Iterate of each firewall that exists e.g. security.firewalls.main
foreach ($firewalls as $firewallName => $firewallConfig) {
if (!\is_array($firewallConfig)) {
continue;
}

return array_filter($authenticators, static fn ($authenticator) => class_exists($authenticator));
foreach ($firewallConfig as $potentialAuthenticator => $configData) {
if (null === ($authenticator = AuthenticatorType::tryFrom($potentialAuthenticator))) {
// This entry is probably security.firewalls.main.lazy or security.firewalls.main.providers
continue;
}

if (AuthenticatorType::CUSTOM !== $authenticator) {
// We found a "built in" authenticator - "form_login", "json_login", etc...
$authenticators[] = new Authenticator($authenticator, $firewallName);

continue;
}

// custom_authenticators can be set as a string or an array in security.yaml
if (\is_string($configData)) {
$configData = [$configData];
}

foreach ($configData as $customAuthenticatorClass) {
// if (!class_exists($customAuthenticatorClass)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should check if the authenticator exists and that it implements AuthenticatorInterface::class. Doing so would be a bonus? but it would also require refactoring the test suite to implement a concrete CustomAuthenticatorFixture::class Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually a bit lost in this method. We're looping over the keys under a firewall, right? And the first if statement with tryFrom will exit unless the key matches the specific set we have in AuthenticatorType, right? So then, $configData is represents the values beneath those keys. So if I have:

firewalls:
    main:
        form_login:
            path: /login

Won't $configData at this point be ['path' => '/login']? Or am I totally misreading the method?

// continue;
// }

// $isValidAuthenticator = (new \ReflectionClass($customAuthenticatorClass))
// ->implementsInterface(AuthenticatorInterface::class)
// ;

// if ($isValidAuthenticator) {
// We found an actual authenticator and not something else
$authenticators[] = new Authenticator($authenticator, $firewallName, $customAuthenticatorClass);
// }
}
}
}

return [];
return $authenticators;
}

public function guessPasswordSetter(SymfonyStyle $io, string $userClass): string
Expand Down
39 changes: 39 additions & 0 deletions src/Security/Model/Authenticator.php
View file
Open in desktop
@@ -0,0 +1,39 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Security\Model;

/**
* @author Jesse Rushlow<jr@rushlow.dev>
*
* @internal
*/
final class Authenticator
{
public function __construct(
public AuthenticatorType $type,
public string $firewallName,
public ?string $authenticatorClass = null,
) {
}

/**
* Useful for asking questions like "Which authenticator do you want to use?".
*/
public function __toString(): string
{
return sprintf(
'"%s" in the "%s" firewall',
$this->authenticatorClass ?? $this->type->value,
$this->firewallName,
);
}
}
30 changes: 30 additions & 0 deletions src/Security/Model/AuthenticatorType.php
View file
Open in desktop
@@ -0,0 +1,30 @@
<?php

/*
* This file is part of the Symfony MakerBundle package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\MakerBundle\Security\Model;

/**
* @author Jesse Rushlow <jr@rushlow.dev>
*
* @internal
*/
enum AuthenticatorType: string
{
case FORM_LOGIN = 'form_login';
case JSON_LOGIN = 'json_login';
case HTTP_BASIC = 'http_basic';
case LOGIN_LINK = 'login_link';
case ACCESS_TOKEN = 'access_token';
case X509 = 'x509';
case REMOTE_USER = 'remote_user';

case CUSTOM = 'custom_authenticator';
}
63 changes: 62 additions & 1 deletion tests/Maker/MakeRegistrationFormTest.php
View file
Open in desktop
Expand Up @@ -38,7 +38,64 @@ private function createRegistrationFormTest(): MakerTestDetails

public function getTestDetails(): \Generator
{
yield 'it_generates_registration_with_entity_and_authenticator' => [$this->createRegistrationFormTest()
yield 'it_generates_registration_with_entity_and_form_login_with_no_login' => [$this->createRegistrationFormTest()
->run(function (MakerTestRunner $runner) {
$this->makeUser($runner);

$runner->runMaker([
// user class guessed,
// username field guessed
// password guessed
'', // yes to add UniqueEntity
'n', // verify user
// firewall name guessed
'n', // yes authenticate after
'2', // redirect to app_anonymous after registration
]);

$fixturePath = \dirname(__DIR__, 1).'/fixtures/make-registration-form/expected';

$this->assertFileEquals($fixturePath.'/RegistrationControllerNoLogin.php', $runner->getPath('src/Controller/RegistrationController.php'));

$this->runRegistrationTest($runner, 'it_generates_registration_with_entity_and_authenticator.php');
}),
];

yield 'it_generates_registration_with_entity_and_form_login_with_security_bundle_login' => [$this->createRegistrationFormTest()
->run(function (MakerTestRunner $runner) {
if (60200 > $runner->getSymfonyVersion()) {
$this->markTestSkipped('Requires Symfony 6.2+');
}

$this->makeUser($runner);

$runner->modifyYamlFile('config/packages/security.yaml', function (array $data) {
$data['security']['firewalls']['main']['form_login']['login_path'] = 'app_login';
$data['security']['firewalls']['main']['form_login']['check_path'] = 'app_login';

return $data;
});

$runner->runMaker([
// user class guessed,
// username field guessed
// password guessed
'', // yes to add UniqueEntity
'n', // verify user
// firewall name guessed
'', // yes authenticate after
// 1 authenticator will be guessed
]);

$fixturePath = \dirname(__DIR__, 1).'/fixtures/make-registration-form/expected';

$this->assertFileEquals($fixturePath.'/RegistrationControllerFormLogin.php', $runner->getPath('src/Controller/RegistrationController.php'));

$this->runRegistrationTest($runner, 'it_generates_registration_with_entity_and_authenticator.php');
}),
];

yield 'it_generates_registration_with_entity_and_custom_authenticator' => [$this->createRegistrationFormTest()
->run(function (MakerTestRunner $runner) {
$this->makeUser($runner);

Expand All @@ -59,6 +116,10 @@ public function getTestDetails(): \Generator
// 1 authenticator will be guessed
]);

$fixturePath = \dirname(__DIR__, 1).'/fixtures/make-registration-form/expected';

$this->assertFileEquals($fixturePath.'/RegistrationControllerCustomAuthenticator.php', $runner->getPath('src/Controller/RegistrationController.php'));

$this->runRegistrationTest($runner, 'it_generates_registration_with_entity_and_authenticator.php');
}),
];
Expand Down