Skip to content

Commit a983caa

Browse files
nikophilnikophil
committed
fixes after PR
1 parent 2186549 commit a983caa

File tree

9 files changed

+82
-160
lines changed

9 files changed

+82
-160
lines changed

‎src/Maker/MakeAuthenticator.php‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@
2020
use Symfony\Bundle\MakerBundle\Security\SecurityConfigUpdater;
2121
use Symfony\Bundle\MakerBundle\Util\YamlManipulationFailedException;
2222
use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator;
23+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
2324
use Symfony\Component\Console\Command\Command;
24-
use Symfony\Component\Console\Exception\InvalidOptionException;
2525
use Symfony\Component\Console\Input\InputArgument;
26-
use Symfony\Bundle\SecurityBundle\SecurityBundle;
2726
use Symfony\Component\Console\Input\InputInterface;
2827
use Symfony\Component\Console\Input\InputOption;
29-
use Symfony\Component\Filesystem\Filesystem;
3028

3129
/**
3230
* @author Ryan Weaver <[email protected]>
31+
*
32+
* @internal
3333
*/
3434
final class MakeAuthenticator extends AbstractMaker
3535
{
@@ -56,15 +56,12 @@ public function configureCommand(Command $command, InputConfiguration $inputConf
5656
$command
5757
->setDescription('Creates an empty Guard authenticator')
5858
->addArgument('authenticator-class', InputArgument::OPTIONAL, 'The class name of the authenticator to create (e.g. <fg=yellow>AppCustomAuthenticator</>)')
59-
->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeAuth.txt'))
60-
;
59+
->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeAuth.txt'));
6160
}
6261

6362
public function interact(InputInterface $input, ConsoleStyle $io, Command $command)
6463
{
65-
$fs = new Filesystem();
66-
67-
if (!$fs->exists($path = 'config/packages/security.yaml')) {
64+
if (!$this->fileManager->fileExists($path = 'config/packages/security.yaml')) {
6865
return;
6966
}
7067

@@ -74,10 +71,19 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma
7471
$interactiveSecurityHelper = new InteractiveSecurityHelper();
7572

7673
$command->addOption('firewall-name', null, InputOption::VALUE_OPTIONAL, '');
77-
$interactiveSecurityHelper->guessFirewallName($input, $io, $securityData);
74+
$input->setOption('firewall-name', $firewallName = $interactiveSecurityHelper->guessFirewallName($io, $securityData));
7875

7976
$command->addOption('entry-point', null, InputOption::VALUE_OPTIONAL);
80-
$interactiveSecurityHelper->guessEntryPoint($input, $io, $this->generator, $securityData);
77+
78+
$authenticatorClassNameDetails = $this->generator->createClassNameDetails(
79+
$input->getArgument('authenticator-class'),
80+
'Security\\'
81+
);
82+
83+
$input->setOption(
84+
'entry-point',
85+
$interactiveSecurityHelper->guessEntryPoint($io, $securityData, $authenticatorClassNameDetails->getFullName(), $firewallName)
86+
);
8187
}
8288

8389
public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator)
@@ -115,7 +121,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
115121

116122
$text = ['Next: Customize your new authenticator.'];
117123
if (!$securityYamlUpdated) {
118-
$text[] = 'Then, configure the "guard" key on your firewall to use it.';
124+
$yamlExample = $this->configUpdater->updateForAuthenticator(
125+
'security: {}',
126+
'main',
127+
null,
128+
$classNameDetails->getFullName()
129+
);
130+
$text[] = "Your <info>security.yaml</info> could not be updated automatically. You'll need to add the following config manually:\n\n".$yamlExample;
119131
}
120132
$io->text($text);
121133
}

‎src/Security/InteractiveSecurityHelper.php‎

Lines changed: 24 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,56 +12,35 @@
1212
namespace Symfony\Bundle\MakerBundle\Security;
1313

1414
use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException;
15-
use Symfony\Bundle\MakerBundle\Generator;
16-
use Symfony\Component\Console\Input\InputInterface;
1715
use Symfony\Component\Console\Style\SymfonyStyle;
1816

1917
/**
2018
* @internal
2119
*/
2220
final class InteractiveSecurityHelper
2321
{
24-
/**
25-
* @param InputInterface $input
26-
* @param SymfonyStyle $io
27-
* @param array $securityData
28-
*/
29-
public function guessFirewallName(InputInterface $input, SymfonyStyle $io, array $securityData)
22+
public function guessFirewallName(SymfonyStyle $io, array $securityData)
3023
{
31-
$firewalls = array_filter(
24+
$realFirewalls = array_filter(
3225
$securityData['security']['firewalls'] ?? [],
3326
function ($item) {
3427
return !isset($item['security']) || true === $item['security'];
3528
}
3629
);
3730

38-
if (count($firewalls) < 2) {
39-
if ($firewalls) {
40-
$input->setOption('firewall-name', key($firewalls));
41-
} else {
42-
$input->setOption('firewall-name', 'main');
43-
}
31+
if (0 === \count($realFirewalls)) {
32+
return 'main';
33+
}
4434

45-
return;
35+
if (1 === \count($realFirewalls)) {
36+
return key($realFirewalls);
4637
}
4738

48-
$firewallName = $io->choice('Which firewall you want to update ?', array_keys($firewalls), key($firewalls));
49-
$input->setOption('firewall-name', $firewallName);
39+
return $io->choice('Which firewall do you want to update ?', array_keys($realFirewalls), key($realFirewalls));
5040
}
5141

52-
/**
53-
* @param InputInterface $input
54-
* @param SymfonyStyle $io
55-
* @param Generator $generator
56-
* @param array $securityData
57-
*/
58-
public function guessEntryPoint(InputInterface $input, SymfonyStyle $io, Generator $generator, array $securityData)
42+
public function guessEntryPoint(SymfonyStyle $io, array $securityData, string $authenticatorClass, string $firewallName)
5943
{
60-
$firewallName = $input->getOption('firewall-name');
61-
if (!$firewallName) {
62-
throw new RuntimeCommandException("Option \"firewall-name\" must be provided.");
63-
}
64-
6544
if (!isset($securityData['security'])) {
6645
$securityData['security'] = [];
6746
}
@@ -72,23 +51,28 @@ public function guessEntryPoint(InputInterface $input, SymfonyStyle $io, Generat
7251

7352
$firewalls = $securityData['security']['firewalls'];
7453
if (!isset($firewalls[$firewallName])) {
75-
throw new RuntimeCommandException("Firewall \"$firewallName\" does not exist");
54+
throw new RuntimeCommandException(sprintf('Firewall "%s" does not exist', $firewallName));
7655
}
7756

7857
if (!isset($firewalls[$firewallName]['guard'])
7958
|| !isset($firewalls[$firewallName]['guard']['authenticators'])
80-
|| !$firewalls[$firewallName]['guard']['authenticators']) {
81-
return;
59+
|| !$firewalls[$firewallName]['guard']['authenticators']
60+
|| isset($firewalls[$firewallName]['guard']['entry_point'])) {
61+
return null;
8262
}
8363

8464
$authenticators = $firewalls[$firewallName]['guard']['authenticators'];
85-
$classNameDetails = $generator->createClassNameDetails(
86-
$input->getArgument('authenticator-class'),
87-
'Security\\'
88-
);
89-
$authenticators[] = $classNameDetails->getFullName();
65+
$authenticators[] = $authenticatorClass;
9066

91-
$entryPoint = $io->choice('Which authenticator will be the entry point ?', $authenticators, current($authenticators));
92-
$input->setOption('entry-point', $entryPoint);
67+
return $io->choice(
68+
'The entry point for your firewall is what should happen when an anonymous user tries to access
69+
a protected page. For example, a common "entry point" behavior is to redirect to the login page.
70+
The "entry point" behavior is controlled by the start() method on your authenticator.
71+
However, you will now have multiple authenticators. You need to choose which authenticator\'s
72+
start() method should be used as the entry point (the start() method on all other
73+
authenticators will be ignored, and can be blank.',
74+
$authenticators,
75+
current($authenticators)
76+
);
9377
}
9478
}

‎src/Security/SecurityConfigUpdater.php‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,7 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u
4848
return $contents;
4949
}
5050

51-
/**
52-
* @param string $yamlSource
53-
* @param string $firewallName
54-
* @param string $chosenEntryPoint
55-
* @param string $authenticatorClass
56-
*
57-
* @return string
58-
*/
59-
public function updateForAuthenticator(string $yamlSource, string $firewallName, $chosenEntryPoint, string $authenticatorClass)
51+
public function updateForAuthenticator(string $yamlSource, string $firewallName, $chosenEntryPoint, string $authenticatorClass): string
6052
{
6153
$this->manipulator = new YamlSourceManipulator($yamlSource);
6254

@@ -69,7 +61,7 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName,
6961
}
7062

7163
if (!isset($newData['security']['firewalls'][$firewallName])) {
72-
$newData['security']['firewalls'][$firewallName] = [];
64+
$newData['security']['firewalls'][$firewallName] = ['anonymous' => true];
7365
}
7466

7567
$firewall = $newData['security']['firewalls'][$firewallName];
@@ -84,7 +76,7 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName,
8476

8577
$firewall['guard']['authenticators'][] = $authenticatorClass;
8678

87-
if (count($firewall['guard']['authenticators']) > 1) {
79+
if (\count($firewall['guard']['authenticators']) > 1) {
8880
$firewall['guard']['entry_point'] = $chosenEntryPoint ?? current($firewall['guard']['authenticators']);
8981
}
9082

‎src/Util/YamlSourceManipulator.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ private function changeValueInYaml($value)
450450
$endValuePosition = $this->findEndPositionOfValue($originalVal);
451451

452452
$newYamlValue = $this->convertToYaml($value);
453-
if (!is_array($originalVal) && is_array($value)) {
453+
if (!\is_array($originalVal) && \is_array($value)) {
454454
// we're converting from a scalar to a (multiline) array
455455
// this means we need to break onto the next line
456456

@@ -798,7 +798,7 @@ private function advanceCurrentPosition(int $newPosition)
798798
* to look for indentation.
799799
*/
800800
if ($this->isCharLineBreak(substr($this->contents, $originalPosition - 1, 1))) {
801-
$originalPosition--;
801+
--$originalPosition;
802802
}
803803

804804
// look for empty lines and track the current indentation
@@ -890,7 +890,7 @@ private function guessNextArrayTypeAndAdvance(): string
890890
// because we are either moving along one line until [ {
891891
// or we are finding a line break and stopping: indentation
892892
// should not be calculated
893-
$this->currentPosition++;
893+
++$this->currentPosition;
894894

895895
if ($this->isCharLineBreak($nextCharacter)) {
896896
return self::ARRAY_FORMAT_MULTILINE;

‎tests/Maker/FunctionalTest.php‎

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Symfony\Component\Config\Loader\LoaderInterface;
3333
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
3434
use Symfony\Component\DependencyInjection\ContainerBuilder;
35+
use Symfony\Component\Filesystem\Filesystem;
3536
use Symfony\Component\Finder\Finder;
3637
use Symfony\Component\HttpKernel\Kernel;
3738
use Symfony\Component\Routing\RouteCollectionBuilder;
@@ -293,17 +294,14 @@ public function getCommandTests()
293294
)
294295
->addExtraDependencies('security')
295296
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticator')
296-
->setRequiredPhpVersion(70100)
297297
->assert(
298298
function (string $output, string $directory) {
299299
$this->assertContains('Success', $output);
300300

301-
$finder = new Finder();
302-
$finder->in($directory.'/src/Security')
303-
->name('AppCustomAuthenticator.php');
304-
$this->assertCount(1, $finder);
301+
$fs = new Filesystem();
302+
$this->assertTrue($fs->exists(sprintf('%s/src/Security/AppCustomAuthenticator.php', $directory)));
305303

306-
$securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml"));
304+
$securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory)));
307305
$this->assertEquals(
308306
'App\\Security\\AppCustomAuthenticator',
309307
$securityConfig['security']['firewalls']['main']['guard']['authenticators'][0]
@@ -318,22 +316,17 @@ function (string $output, string $directory) {
318316
[
319317
// class name
320318
'AppCustomAuthenticator',
319+
// firewall name
321320
1
322321
]
323322
)
324323
->addExtraDependencies('security')
325324
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewalls')
326-
->setRequiredPhpVersion(70100)
327325
->assert(
328326
function (string $output, string $directory) {
329327
$this->assertContains('Success', $output);
330328

331-
$finder = new Finder();
332-
$finder->in($directory.'/src/Security')
333-
->name('AppCustomAuthenticator.php');
334-
$this->assertCount(1, $finder);
335-
336-
$securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml"));
329+
$securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory)));
337330
$this->assertEquals(
338331
'App\\Security\\AppCustomAuthenticator',
339332
$securityConfig['security']['firewalls']['second']['guard']['authenticators'][0]
@@ -348,22 +341,17 @@ function (string $output, string $directory) {
348341
[
349342
// class name
350343
'AppCustomAuthenticator',
344+
// firewall name
351345
1
352346
]
353347
)
354348
->addExtraDependencies('security')
355349
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorExistingAuthenticator')
356-
->setRequiredPhpVersion(70100)
357350
->assert(
358351
function (string $output, string $directory) {
359352
$this->assertContains('Success', $output);
360353

361-
$finder = new Finder();
362-
$finder->in($directory.'/src/Security')
363-
->name('AppCustomAuthenticator.php');
364-
$this->assertCount(1, $finder);
365-
366-
$securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml"));
354+
$securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory)));
367355
$this->assertEquals(
368356
'App\\Security\\AppCustomAuthenticator',
369357
$securityConfig['security']['firewalls']['main']['guard']['entry_point']
@@ -378,23 +366,19 @@ function (string $output, string $directory) {
378366
[
379367
// class name
380368
'AppCustomAuthenticator',
369+
// firewall name
381370
1,
371+
// entry point
382372
1
383373
]
384374
)
385375
->addExtraDependencies('security')
386376
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewallsExistingAuthenticator')
387-
->setRequiredPhpVersion(70100)
388377
->assert(
389378
function (string $output, string $directory) {
390379
$this->assertContains('Success', $output);
391380

392-
$finder = new Finder();
393-
$finder->in($directory.'/src/Security')
394-
->name('AppCustomAuthenticator.php');
395-
$this->assertCount(1, $finder);
396-
397-
$securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml"));
381+
$securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory)));
398382
$this->assertEquals(
399383
'App\\Security\\AppCustomAuthenticator',
400384
$securityConfig['security']['firewalls']['second']['guard']['entry_point']

0 commit comments

Comments
 (0)