Skip to content

Commit cd86b3b

Browse files
nikophilnikophil
committed
form login : PR review : batch 4
1 parent 78abbf4 commit cd86b3b

File tree

6 files changed

+128
-82
lines changed

6 files changed

+128
-82
lines changed

‎src/Generator.php‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,20 @@ public function dumpFile(string $targetPath, string $contents)
8585
];
8686
}
8787

88-
public function getFileContents(string $targetPath): string
88+
public function getFileContentsForPendingOperation(string $targetPath): string
8989
{
9090
if (!isset($this->pendingOperations[$targetPath])) {
9191
throw new RuntimeCommandException(sprintf('File "%s" is not in the Generator\'s pending operations', $targetPath));
9292
}
9393

94-
return $this->getFileContentsForPendingOperation(
95-
$targetPath,
96-
$this->pendingOperations[$targetPath]
97-
);
94+
$templatePath = $this->pendingOperations[$targetPath]['template'];
95+
$parameters = $this->pendingOperations[$targetPath]['variables'];
96+
97+
$templateParameters = array_merge($parameters, [
98+
'relative_path' => $this->fileManager->relativizePath($targetPath),
99+
]);
100+
101+
return $this->fileManager->parseTemplate($templatePath, $templateParameters);
98102
}
99103

100104
/**
@@ -200,18 +204,6 @@ public function writeChanges()
200204
$this->pendingOperations = [];
201205
}
202206

203-
private function getFileContentsForPendingOperation(string $targetPath, array $templateData): string
204-
{
205-
$templatePath = $templateData['template'];
206-
$parameters = $templateData['variables'];
207-
208-
$templateParameters = array_merge($parameters, [
209-
'relative_path' => $this->fileManager->relativizePath($targetPath),
210-
]);
211-
212-
return $this->fileManager->parseTemplate($templatePath, $templateParameters);
213-
}
214-
215207
public function getRootNamespace(): string
216208
{
217209
return $this->namespacePrefix;

‎src/Maker/AbstractMaker.php‎

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

1414
use Symfony\Bundle\MakerBundle\ConsoleStyle;
15+
use Symfony\Bundle\MakerBundle\DependencyBuilder;
1516
use Symfony\Bundle\MakerBundle\MakerInterface;
1617
use Symfony\Component\Console\Command\Command;
1718
use Symfony\Component\Console\Input\InputInterface;
@@ -33,4 +34,18 @@ protected function writeSuccessMessage(ConsoleStyle $io)
3334
$io->writeln(' <bg=green;fg=white> </>');
3435
$io->newLine();
3536
}
37+
38+
protected function addDependencies(array $dependencies, string $message = null): string
39+
{
40+
$dependencyBuilder = new DependencyBuilder();
41+
42+
foreach ($dependencies as $class => $name) {
43+
$dependencyBuilder->addClassDependency($class, $name);
44+
}
45+
46+
return $dependencyBuilder->getMissingPackagesMessage(
47+
$this->getCommandName(),
48+
$message
49+
);
50+
}
3651
}

‎src/Maker/MakeAuthenticator.php‎

Lines changed: 97 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,15 @@ public function configureCommand(Command $command, InputConfiguration $inputConf
7777
public function interact(InputInterface $input, ConsoleStyle $io, Command $command)
7878
{
7979
if (!$this->fileManager->fileExists($path = 'config/packages/security.yaml')) {
80-
throw new RuntimeCommandException('File "security.yaml" does not exist!');
80+
throw new RuntimeCommandException('The file "config/packages/security.yaml" does not exist. This command requires that file to exist so that it can be updated.');
8181
}
8282
$manipulator = new YamlSourceManipulator($this->fileManager->getFileContents($path));
8383
$securityData = $manipulator->getData();
8484

8585
// authenticator type
8686
$authenticatorTypeValues = [
8787
'Empty authenticator' => self::AUTH_TYPE_EMPTY_AUTHENTICATOR,
88-
'Form login' => self::AUTH_TYPE_FORM_LOGIN,
88+
'Login form authenticator' => self::AUTH_TYPE_FORM_LOGIN,
8989
];
9090
$command->addArgument('authenticator-type', InputArgument::REQUIRED);
9191
$authenticatorType = $io->choice(
@@ -99,22 +99,15 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
9999
);
100100

101101
if (self::AUTH_TYPE_FORM_LOGIN === $input->getArgument('authenticator-type')) {
102-
$dependencies = new DependencyBuilder();
103-
$dependencies->addClassDependency(
104-
TwigBundle::class,
105-
'twig'
106-
);
107-
$missingPackagesMessage = 'Twig must be installed to display login form';
102+
$neededDependencies = [TwigBundle::class => 'twig'];
103+
$missingPackagesMessage = 'Twig must be installed to display the login form.';
108104

109105
if (Kernel::VERSION_ID < 40100) {
110-
$dependencies->addClassDependency(
111-
Form::class,
112-
'symfony/form'
113-
);
114-
$missingPackagesMessage = 'Twig and symfony/form must be installed to display login form';
106+
$neededDependencies[Form::class] = 'symfony/form';
107+
$missingPackagesMessage = 'Twig and symfony/form must be installed to display the login form';
115108
}
116109

117-
$missingPackagesMessage = $dependencies->getMissingPackagesMessage(self::getCommandName(), $missingPackagesMessage);
110+
$missingPackagesMessage = $this->addDependencies($neededDependencies, $missingPackagesMessage);
118111
if ($missingPackagesMessage) {
119112
throw new RuntimeCommandException($missingPackagesMessage);
120113
}
@@ -175,7 +168,16 @@ function ($answer) {
175168

176169
public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator)
177170
{
178-
$this->generateAuthenticatorClass($input);
171+
$manipulator = new YamlSourceManipulator($this->fileManager->getFileContents('config/packages/security.yaml'));
172+
$securityData = $manipulator->getData();
173+
174+
$this->generateAuthenticatorClass(
175+
$securityData,
176+
$input->getArgument('authenticator-type'),
177+
$input->getArgument('authenticator-class'),
178+
$input->hasArgument('user-class') ? $input->getArgument('user-class') : null,
179+
$input->hasArgument('username-field') ? $input->getArgument('username-field') : null
180+
);
179181

180182
// update security.yaml with guard config
181183
$securityYamlUpdated = false;
@@ -192,71 +194,59 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
192194
}
193195

194196
if (self::AUTH_TYPE_FORM_LOGIN === $input->getArgument('authenticator-type')) {
195-
$this->generateFormLoginFiles($input);
197+
$this->generateFormLoginFiles($input->getArgument('controller-class'), $input->getArgument('username-field'));
196198
}
197199

198200
$generator->writeChanges();
199201

200202
$this->writeSuccessMessage($io);
201203

202-
$text = ['Next: Customize your new authenticator.'];
203-
if (!$securityYamlUpdated) {
204-
$yamlExample = $this->configUpdater->updateForAuthenticator(
205-
'security: {}',
206-
'main',
207-
null,
208-
$input->getArgument('authenticator-class')
209-
);
210-
$text[] = "Your <info>security.yaml</info> could not be updated automatically. You'll need to add the following config manually:\n\n".$yamlExample;
211-
}
212-
$io->text($text);
204+
$io->text(
205+
$this->generateNextMessage(
206+
$securityYamlUpdated,
207+
$input->getArgument('authenticator-type'),
208+
$input->getArgument('authenticator-class'),
209+
$securityData,
210+
$input->hasArgument('user-class') ? $input->getArgument('user-class') : null
211+
)
212+
);
213213
}
214214

215-
private function generateAuthenticatorClass(InputInterface $input)
215+
private function generateAuthenticatorClass(array $securityData, string $authenticatorType, string $authenticatorClass, $userClass, $userNameField)
216216
{
217217
// generate authenticator class
218-
if (self::AUTH_TYPE_EMPTY_AUTHENTICATOR === $input->getArgument('authenticator-type')) {
218+
if (self::AUTH_TYPE_EMPTY_AUTHENTICATOR === $authenticatorType) {
219219
$this->generator->generateClass(
220-
$input->getArgument('authenticator-class'),
220+
$authenticatorClass,
221221
'authenticator/EmptyAuthenticator.tpl.php',
222222
[]
223223
);
224-
} else {
225-
$manipulator = new YamlSourceManipulator($this->fileManager->getFileContents('config/packages/security.yaml'));
226-
$securityData = $manipulator->getData();
227-
228-
$userNeedsEncoder = false;
229-
if (isset($securityData['security']['encoders']) && $securityData['security']['encoders']) {
230-
foreach ($securityData['security']['encoders'] as $userClassWithEncoder => $encoder) {
231-
if ($input->getArgument('user-class') === $userClassWithEncoder || is_subclass_of($input->getArgument('user-class'), $userClassWithEncoder)) {
232-
$userNeedsEncoder = true;
233-
}
234-
}
235-
}
236-
237-
$userClassNameDetails = $this->generator->createClassNameDetails(
238-
'\\'.$input->getArgument('user-class'),
239-
'Entity\\'
240-
);
241224

242-
$this->generator->generateClass(
243-
$input->getArgument('authenticator-class'),
244-
'authenticator/LoginFormAuthenticator.tpl.php',
245-
[
246-
'user_fully_qualified_class_name' => trim($userClassNameDetails->getFullName(), '\\'),
247-
'user_class_name' => $userClassNameDetails->getShortName(),
248-
'username_field' => $input->getArgument('username-field'),
249-
'user_needs_encoder' => $userNeedsEncoder,
250-
'user_is_entity' => $this->doctrineHelper->isClassAMappedEntity($input->getArgument('user-class')),
251-
]
252-
);
225+
return;
253226
}
227+
228+
$userClassNameDetails = $this->generator->createClassNameDetails(
229+
'\\'.$userClass,
230+
'Entity\\'
231+
);
232+
233+
$this->generator->generateClass(
234+
$authenticatorClass,
235+
'authenticator/LoginFormAuthenticator.tpl.php',
236+
[
237+
'user_fully_qualified_class_name' => trim($userClassNameDetails->getFullName(), '\\'),
238+
'user_class_name' => $userClassNameDetails->getShortName(),
239+
'username_field' => $userNameField,
240+
'user_needs_encoder' => $this->userNeedsEncoder($securityData, $userClass),
241+
'user_is_entity' => $this->doctrineHelper->isClassAMappedEntity($userClass),
242+
]
243+
);
254244
}
255245

256-
private function generateFormLoginFiles(InputInterface $input)
246+
private function generateFormLoginFiles(string $controllerClass, string $userNameField)
257247
{
258248
$controllerClassNameDetails = $this->generator->createClassNameDetails(
259-
$input->getArgument('controller-class'),
249+
$controllerClass,
260250
'Controller\\',
261251
'Controller'
262252
);
@@ -267,7 +257,7 @@ private function generateFormLoginFiles(InputInterface $input)
267257
'authenticator/EmptySecurityController.tpl.php'
268258
);
269259

270-
$controllerSourceCode = $this->generator->getFileContents($controllerPath);
260+
$controllerSourceCode = $this->generator->getFileContentsForPendingOperation($controllerPath);
271261
} else {
272262
$controllerPath = $this->fileManager->getRelativePathForFutureClass($controllerClassNameDetails->getFullName());
273263
$controllerSourceCode = $this->fileManager->getFileContents($controllerPath);
@@ -289,11 +279,56 @@ private function generateFormLoginFiles(InputInterface $input)
289279
'templates/security/login.html.twig',
290280
'authenticator/login_form.tpl.php',
291281
[
292-
'username_field' => $input->getArgument('username-field'),
282+
'username_field' => $userNameField,
293283
]
294284
);
295285
}
296286

287+
private function generateNextMessage(bool $securityYamlUpdated, string $authenticatorType, string $authenticatorClass, array $securityData, $userClass): array
288+
{
289+
$nextTexts = ['Next:'];
290+
$nextTexts[] = '- Customize your new authenticator.';
291+
292+
if (!$securityYamlUpdated) {
293+
$yamlExample = $this->configUpdater->updateForAuthenticator(
294+
'security: {}',
295+
'main',
296+
null,
297+
$authenticatorClass
298+
);
299+
$nextTexts[] = '- Your <info>security.yaml</info> could not be updated automatically. You\'ll need to add the following config manually:\n\n'.$yamlExample;
300+
}
301+
302+
if (self::AUTH_TYPE_FORM_LOGIN === $authenticatorType) {
303+
$nextTexts[] = sprintf('- You must provide a valid redirection in the method <info>%s::onAuthenticationSuccess()</info>.', $authenticatorClass);
304+
$nextTexts[] = '- Review & adapt the login template : <info>/templates/security/login.html.twig</info>.';
305+
306+
if (!$this->doctrineHelper->isClassAMappedEntity($userClass)) {
307+
$nextTexts[] = sprintf('- Review <info>%s::getUser()</info>, if it match your needs.', $authenticatorClass);
308+
}
309+
310+
if (!$this->userNeedsEncoder($securityData, $userClass)) {
311+
$nextTexts[] = sprintf('- Check user\'s password in <info>%s::checkCredentials()</info>.', $authenticatorClass);
312+
}
313+
}
314+
315+
return $nextTexts;
316+
}
317+
318+
private function userNeedsEncoder(array $securityData, string $userClass): bool
319+
{
320+
$userNeedsEncoder = false;
321+
if (isset($securityData['security']['encoders']) && $securityData['security']['encoders']) {
322+
foreach ($securityData['security']['encoders'] as $userClassWithEncoder => $encoder) {
323+
if ($userClass === $userClassWithEncoder || is_subclass_of($userClass, $userClassWithEncoder)) {
324+
$userNeedsEncoder = true;
325+
}
326+
}
327+
}
328+
329+
return $userNeedsEncoder;
330+
}
331+
297332
public function configureDependencies(DependencyBuilder $dependencies, InputInterface $input = null)
298333
{
299334
$dependencies->addClassDependency(

‎src/Resources/skeleton/authenticator/LoginFormAuthenticator.tpl.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ public function getUser($credentials, UserProviderInterface $userProvider)
7171

7272
public function checkCredentials($credentials, UserInterface $user)
7373
{
74-
<?= $user_needs_encoder ? "return \$this->passwordEncoder->isPasswordValid(\$user, \$credentials['password']);\n" : "return true;\n" ?>
74+
<?= $user_needs_encoder ? "return \$this->passwordEncoder->isPasswordValid(\$user, \$credentials['password']);\n"
75+
: "// Check the user’s password or other credentials and return true or false
76+
// If there are no credentials to check, you can just return true
77+
throw new \Exception('TODO: check the credentials inside '.__FILE__);\n" ?>
7578
}
7679

7780
public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey)
@@ -80,6 +83,7 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token,
8083
return new RedirectResponse($targetPath);
8184
}
8285

86+
// e.g. : return new RedirectResponse($this->router->generate('some_route'));
8387
throw new \Exception('TODO: provide a valid redirection inside '.__FILE__);
8488
}
8589

‎tests/fixtures/MakeAuthenticatorLoginFormUserEntityNoEncoder/tests/SecurityControllerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ public function testCommand()
4343

4444
$client->submit($form);
4545

46-
$this->assertContains('TODO: provide a valid redirection', $client->getResponse()->getContent());
46+
$this->assertContains('TODO: check the credentials', $client->getResponse()->getContent());
4747
}
4848
}

‎tests/fixtures/MakeAuthenticatorLoginFormUserNotEntityNoEncoder/tests/SecurityControllerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,6 @@ public function testCommand()
3030
);
3131
$client->submit($form);
3232

33-
$this->assertContains('TODO: provide a valid redirection', $client->getResponse()->getContent());
33+
$this->assertContains('TODO: check the credentials', $client->getResponse()->getContent());
3434
}
3535
}

0 commit comments

Comments
 (0)