From 2fb5b40404b91b07771814a649ad0fcbba36509c Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Wed, 5 Sep 2018 18:17:31 +0200 Subject: [PATCH 1/5] add authenticator in security.yaml the simplest way --- src/Maker/MakeAuthenticator.php | 27 ++++++++++++ src/Resources/config/makers.xml | 2 + src/Security/SecurityConfigUpdater.php | 58 +++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/src/Maker/MakeAuthenticator.php b/src/Maker/MakeAuthenticator.php index e01157c7c..d10cada52 100644 --- a/src/Maker/MakeAuthenticator.php +++ b/src/Maker/MakeAuthenticator.php @@ -13,8 +13,12 @@ use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; +use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputConfiguration; +use Symfony\Bundle\MakerBundle\Security\SecurityConfigUpdater; +use Symfony\Bundle\MakerBundle\Util\YamlManipulationFailedException; +use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Bundle\SecurityBundle\SecurityBundle; @@ -25,6 +29,16 @@ */ final class MakeAuthenticator extends AbstractMaker { + private $fileManager; + + private $configUpdater; + + public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater) + { + $this->fileManager = $fileManager; + $this->configUpdater = $configUpdater; + } + public static function getCommandName(): string { return 'make:auth'; @@ -51,6 +65,19 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'authenticator/Empty.tpl.php', [] ); + + $path = 'config/packages/security.yaml'; + if ($this->fileManager->fileExists($path)) { + try { + $newYaml = $this->configUpdater->updateForAuthenticator( + $this->fileManager->getFileContents($path), + $classNameDetails->getFullName() + ); + $generator->dumpFile($path, $newYaml); + } catch (YamlManipulationFailedException $e) { + } + } + $generator->writeChanges(); $this->writeSuccessMessage($io); diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index 9994bb5f9..b8ce52956 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -8,6 +8,8 @@ + + diff --git a/src/Security/SecurityConfigUpdater.php b/src/Security/SecurityConfigUpdater.php index 05dd35593..1756afef5 100644 --- a/src/Security/SecurityConfigUpdater.php +++ b/src/Security/SecurityConfigUpdater.php @@ -34,12 +34,7 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u { $this->manipulator = new YamlSourceManipulator($yamlSource); - // normalize the top level, just in case - if (!isset($this->manipulator->getData()['security'])) { - $newData = $this->manipulator->getData(); - $newData['security'] = []; - $this->manipulator->setData($newData); - } + $this->normalizeSecurityYamlFile(); $this->updateProviders($userConfig, $userClass); @@ -53,6 +48,57 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u return $contents; } + public function updateForAuthenticator(string $yamlSource, string $authenticatorFQCN) + { + $this->manipulator = new YamlSourceManipulator($yamlSource); + + $this->normalizeSecurityYamlFile(); + + $newData = $this->manipulator->getData(); + + if (!isset($newData['security']['firewalls'])) { + $newData['security']['firewalls'] = []; + } + + $firewalls = array_filter( + $newData['security']['firewalls'], + function ($item) { + return !isset($item['security']) || true === $item['security']; + } + ); + + if (!$firewalls) { + $firewalls['main'] = ['anonymous' => true]; + } + + $firewall = $firewalls['main']; + if (!isset($firewall['guard'])) { + $firewall['guard'] = []; + } + + if (!isset($firewall['guard']['authenticators'])) { + $firewall['guard']['authenticators'] = []; + } + + $firewall['guard']['authenticators'][] = $authenticatorFQCN; + + $newData['security']['firewalls']['main'] = $firewall; + $this->manipulator->setData($newData); + + $contents = $this->manipulator->getContents(); + + return $contents; + } + + private function normalizeSecurityYamlFile() + { + if (!isset($this->manipulator->getData()['security'])) { + $newData = $this->manipulator->getData(); + $newData['security'] = []; + $this->manipulator->setData($newData); + } + } + private function updateProviders(UserClassConfiguration $userConfig, string $userClass) { if ($this->isSingleInMemoryProviderConfigured()) { From c63a86a2aa49dc262edb8e8f340684ed28a80c37 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Wed, 5 Sep 2018 22:43:46 +0200 Subject: [PATCH 2/5] Add authenticator : let the user chose which firewall to update --- src/Maker/MakeAuthenticator.php | 32 ++++++++-- src/Security/InteractiveSecurityHelper.php | 44 +++++++++++++ src/Security/SecurityConfigUpdater.php | 25 ++++---- tests/Maker/FunctionalTest.php | 64 +++++++++++++++++-- tests/Security/SecurityConfigUpdaterTest.php | 35 ++++++++++ .../expected_authenticator/empty_source.yaml | 5 ++ .../simple_security_source.yaml | 8 +++ .../simple_security_with_firewalls.yaml | 14 ++++ .../simple_security_with_firewalls.yaml | 11 ++++ .../config/packages/security.yaml | 24 +++++++ .../config/packages/security.yaml | 26 ++++++++ 11 files changed, 264 insertions(+), 24 deletions(-) create mode 100644 src/Security/InteractiveSecurityHelper.php create mode 100644 tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml create mode 100644 tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml create mode 100644 tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls.yaml create mode 100644 tests/Security/yaml_fixtures/source/simple_security_with_firewalls.yaml create mode 100644 tests/fixtures/MakeAuthenticator/config/packages/security.yaml create mode 100644 tests/fixtures/MakeAuthenticatorMultipleFirewalls/config/packages/security.yaml diff --git a/src/Maker/MakeAuthenticator.php b/src/Maker/MakeAuthenticator.php index d10cada52..089dca7c3 100644 --- a/src/Maker/MakeAuthenticator.php +++ b/src/Maker/MakeAuthenticator.php @@ -16,13 +16,17 @@ use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputConfiguration; +use Symfony\Bundle\MakerBundle\Security\InteractiveSecurityHelper; use Symfony\Bundle\MakerBundle\Security\SecurityConfigUpdater; use Symfony\Bundle\MakerBundle\Util\YamlManipulationFailedException; use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Exception\InvalidOptionException; use Symfony\Component\Console\Input\InputArgument; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Filesystem\Filesystem; /** * @author Ryan Weaver @@ -53,6 +57,22 @@ public function configureCommand(Command $command, InputConfiguration $inputConf ; } + public function interact(InputInterface $input, ConsoleStyle $io, Command $command) + { + $fs = new Filesystem(); + + if (!$fs->exists($path = 'config/packages/security.yaml')) { + return; + } + + $manipulator = new YamlSourceManipulator($this->fileManager->getFileContents($path)); + $securityData = $manipulator->getData(); + + $interactiveSecurityHelper = new InteractiveSecurityHelper(); + $command->addOption('firewall-name', null, InputOption::VALUE_OPTIONAL, ''); + $interactiveSecurityHelper->guessFirewallName($input, $io, $securityData); + } + public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) { $classNameDetails = $generator->createClassNameDetails( @@ -66,14 +86,17 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen [] ); + $securityYamlUpdated = false; $path = 'config/packages/security.yaml'; if ($this->fileManager->fileExists($path)) { try { $newYaml = $this->configUpdater->updateForAuthenticator( $this->fileManager->getFileContents($path), + $input->getOption('firewall-name'), $classNameDetails->getFullName() ); $generator->dumpFile($path, $newYaml); + $securityYamlUpdated = true; } catch (YamlManipulationFailedException $e) { } } @@ -82,10 +105,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->writeSuccessMessage($io); - $io->text([ - 'Next: Customize your new authenticator.', - 'Then, configure the "guard" key on your firewall to use it.', - ]); + $text = ['Next: Customize your new authenticator.']; + if (!$securityYamlUpdated) { + $text[] = 'Then, configure the "guard" key on your firewall to use it.'; + } + $io->text($text); } public function configureDependencies(DependencyBuilder $dependencies) diff --git a/src/Security/InteractiveSecurityHelper.php b/src/Security/InteractiveSecurityHelper.php new file mode 100644 index 000000000..33e8dcec9 --- /dev/null +++ b/src/Security/InteractiveSecurityHelper.php @@ -0,0 +1,44 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Security; + +use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Component\Console\Input\InputInterface; + +/** + * @internal + */ +final class InteractiveSecurityHelper +{ + public function guessFirewallName(InputInterface $input, ConsoleStyle $io, array $securityData) + { + $firewalls = array_filter( + $securityData['security']['firewalls'], + function ($item) { + return !isset($item['security']) || true === $item['security']; + } + ); + + if (count($firewalls) < 2) { + if ($firewalls) { + $input->setOption('firewall-name', key($firewalls)); + } else { + $input->setOption('firewall-name', 'main'); + } + + return; + } + + $firewallName = $io->choice('Which firewall you want to update ?', array_keys($firewalls), key($firewalls)); + $input->setOption('firewall-name', $firewallName); + } +} diff --git a/src/Security/SecurityConfigUpdater.php b/src/Security/SecurityConfigUpdater.php index 1756afef5..a0ba21655 100644 --- a/src/Security/SecurityConfigUpdater.php +++ b/src/Security/SecurityConfigUpdater.php @@ -48,7 +48,7 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u return $contents; } - public function updateForAuthenticator(string $yamlSource, string $authenticatorFQCN) + public function updateForAuthenticator(string $yamlSource, string $firewallName, string $userClass) { $this->manipulator = new YamlSourceManipulator($yamlSource); @@ -60,18 +60,12 @@ public function updateForAuthenticator(string $yamlSource, string $authenticator $newData['security']['firewalls'] = []; } - $firewalls = array_filter( - $newData['security']['firewalls'], - function ($item) { - return !isset($item['security']) || true === $item['security']; - } - ); - - if (!$firewalls) { - $firewalls['main'] = ['anonymous' => true]; + if (!isset($newData['security']['firewalls'][$firewallName])) { + $newData['security']['firewalls'][$firewallName] = []; } - $firewall = $firewalls['main']; + $firewall = $newData['security']['firewalls'][$firewallName]; + if (!isset($firewall['guard'])) { $firewall['guard'] = []; } @@ -80,11 +74,14 @@ function ($item) { $firewall['guard']['authenticators'] = []; } - $firewall['guard']['authenticators'][] = $authenticatorFQCN; + $firewall['guard']['authenticators'][] = $userClass; - $newData['security']['firewalls']['main'] = $firewall; - $this->manipulator->setData($newData); + if (count($firewall['guard']['authenticators']) > 1) { + $firewall['guard']['entry_point'] = current($firewall['guard']['authenticators']); + } + $newData['security']['firewalls'][$firewallName] = $firewall; + $this->manipulator->setData($newData); $contents = $this->manipulator->getContents(); return $contents; diff --git a/tests/Maker/FunctionalTest.php b/tests/Maker/FunctionalTest.php index c94bc9709..c91a48615 100644 --- a/tests/Maker/FunctionalTest.php +++ b/tests/Maker/FunctionalTest.php @@ -35,6 +35,7 @@ use Symfony\Component\Finder\Finder; use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\Routing\RouteCollectionBuilder; +use Symfony\Component\Yaml\Yaml; class FunctionalTest extends MakerTestCase { @@ -282,12 +283,63 @@ public function getCommandTests() ]) ]; - yield 'auth_empty' => [MakerTestDetails::createTest( - $this->getMakerInstance(MakeAuthenticator::class), - [ - // class name - 'AppCustomAuthenticator', - ]) + yield 'auth_empty_one_firewall' => [ + MakerTestDetails::createTest( + $this->getMakerInstance(MakeAuthenticator::class), + [ + // class name + 'AppCustomAuthenticator', + ] + ) + ->addExtraDependencies('security') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticator') + ->setRequiredPhpVersion(70100) + ->assert( + function (string $output, string $directory) { + $this->assertContains('Success', $output); + + $finder = new Finder(); + $finder->in($directory.'/src/Security') + ->name('AppCustomAuthenticator.php'); + $this->assertCount(1, $finder); + + $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $this->assertEquals( + 'App\\Security\\AppCustomAuthenticator', + $securityConfig['security']['firewalls']['main']['guard']['authenticators'][0] + ); + } + ), + ]; + + yield 'auth_empty_multiple_firewalls' => [ + MakerTestDetails::createTest( + $this->getMakerInstance(MakeAuthenticator::class), + [ + // class name + 'AppCustomAuthenticator', + 1 + ] + ) + ->addExtraDependencies('security') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewalls') + ->setRequiredPhpVersion(70100) + ->assert( + function (string $output, string $directory) { + $this->assertContains('Success', $output); + + $finder = new Finder(); + $finder->in($directory.'/src/Security') + ->name('AppCustomAuthenticator.php'); + $this->assertCount(1, $finder); + + $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $this->assertEquals( + 'App\\Security\\AppCustomAuthenticator', + $securityConfig['security']['firewalls']['second']['guard']['authenticators'][0] + ); + } + ), ]; yield 'user_security_entity_with_password' => [MakerTestDetails::createTest( diff --git a/tests/Security/SecurityConfigUpdaterTest.php b/tests/Security/SecurityConfigUpdaterTest.php index a0eb7d2ee..9118104f6 100644 --- a/tests/Security/SecurityConfigUpdaterTest.php +++ b/tests/Security/SecurityConfigUpdaterTest.php @@ -67,4 +67,39 @@ public function getUserClassTests() 'empty_security.yaml' ]; } + + /** + * @dataProvider getAuthenticatorTests + */ + public function testUpdateForAuthenticator(string $firewallName, string $expectedSourceFilename, string $startingSourceFilename) + { + $updater = new SecurityConfigUpdater(); + $source = file_get_contents(__DIR__.'/yaml_fixtures/source/'.$startingSourceFilename); + $actualSource = $updater->updateForAuthenticator($source, $firewallName, 'App\\Security\\AppCustomAuthenticator'); + $expectedSource = file_get_contents(__DIR__.'/yaml_fixtures/expected_authenticator/'.$expectedSourceFilename); + + $this->assertSame($expectedSource, $actualSource); + } + + public function getAuthenticatorTests() + { + yield 'empty_source' => [ + 'main', + 'empty_source.yaml', + 'empty_security.yaml' + ]; + + // waiting for YamlSourceManipulator to be debugged +// yield 'empty_source' => [ +// 'main', +// 'simple_security_source.yaml', +// 'simple_security.yaml' +// ]; + + yield 'simple_security_with_firewalls' => [ + 'main', + 'simple_security_with_firewalls.yaml', + 'simple_security_with_firewalls.yaml' + ]; + } } \ No newline at end of file diff --git a/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml b/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml new file mode 100644 index 000000000..6b78efaa4 --- /dev/null +++ b/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml @@ -0,0 +1,5 @@ +security: + firewalls: + main: + guard: + authenticators: [App\Security\AppCustomAuthenticator] \ No newline at end of file diff --git a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml new file mode 100644 index 000000000..61b340dee --- /dev/null +++ b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml @@ -0,0 +1,8 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + + firewalls: + dev: ~ + main: ~ diff --git a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls.yaml b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls.yaml new file mode 100644 index 000000000..7337047e1 --- /dev/null +++ b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls.yaml @@ -0,0 +1,14 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + guard: + authenticators: + - App\Security\AppCustomAuthenticator diff --git a/tests/Security/yaml_fixtures/source/simple_security_with_firewalls.yaml b/tests/Security/yaml_fixtures/source/simple_security_with_firewalls.yaml new file mode 100644 index 000000000..e36f35da0 --- /dev/null +++ b/tests/Security/yaml_fixtures/source/simple_security_with_firewalls.yaml @@ -0,0 +1,11 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true diff --git a/tests/fixtures/MakeAuthenticator/config/packages/security.yaml b/tests/fixtures/MakeAuthenticator/config/packages/security.yaml new file mode 100644 index 000000000..48128ebee --- /dev/null +++ b/tests/fixtures/MakeAuthenticator/config/packages/security.yaml @@ -0,0 +1,24 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + + # activate different ways to authenticate + + # http_basic: true + # https://symfony.com/doc/current/security.html#a-configuring-how-your-users-will-authenticate + + # form_login: true + # https://symfony.com/doc/current/security/form_login_setup.html + + # Easy way to control access for large sections of your site + # Note: Only the *first* access control that matches will be used + access_control: + # - { path: ^/admin, roles: ROLE_ADMIN } + # - { path: ^/profile, roles: ROLE_USER } diff --git a/tests/fixtures/MakeAuthenticatorMultipleFirewalls/config/packages/security.yaml b/tests/fixtures/MakeAuthenticatorMultipleFirewalls/config/packages/security.yaml new file mode 100644 index 000000000..23ddcbda6 --- /dev/null +++ b/tests/fixtures/MakeAuthenticatorMultipleFirewalls/config/packages/security.yaml @@ -0,0 +1,26 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + second: + anonymous: true + + # activate different ways to authenticate + + # http_basic: true + # https://symfony.com/doc/current/security.html#a-configuring-how-your-users-will-authenticate + + # form_login: true + # https://symfony.com/doc/current/security/form_login_setup.html + + # Easy way to control access for large sections of your site + # Note: Only the *first* access control that matches will be used + access_control: + # - { path: ^/admin, roles: ROLE_ADMIN } + # - { path: ^/profile, roles: ROLE_USER } From 2186549b282de01276007529bde6262908283a4c Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Thu, 6 Sep 2018 16:21:41 +0200 Subject: [PATCH 3/5] Add entry point if one or more authenticators exist on chosen firewall --- src/Maker/MakeAuthenticator.php | 10 +- src/Resources/config/makers.xml | 1 + src/Security/InteractiveSecurityHelper.php | 56 +++++- src/Security/SecurityConfigUpdater.php | 14 +- tests/Maker/FunctionalTest.php | 61 ++++++ .../InteractiveSecurityHelperTest.php | 175 ++++++++++++++++++ tests/Security/SecurityConfigUpdaterTest.php | 32 +++- .../simple_security_source.yaml | 5 +- ...rity_with_firewalls_and_authenticator.yaml | 18 ++ ...rity_with_firewalls_and_authenticator.yaml | 16 ++ .../config/packages/security.yaml | 27 +++ .../src/Security/Authenticator.php | 53 ++++++ .../config/packages/security.yaml | 29 +++ .../src/Security/Authenticator.php | 53 ++++++ 14 files changed, 534 insertions(+), 16 deletions(-) create mode 100644 tests/Security/InteractiveSecurityHelperTest.php create mode 100644 tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls_and_authenticator.yaml create mode 100644 tests/Security/yaml_fixtures/source/simple_security_with_firewalls_and_authenticator.yaml create mode 100644 tests/fixtures/MakeAuthenticatorExistingAuthenticator/config/packages/security.yaml create mode 100644 tests/fixtures/MakeAuthenticatorExistingAuthenticator/src/Security/Authenticator.php create mode 100644 tests/fixtures/MakeAuthenticatorMultipleFirewallsExistingAuthenticator/config/packages/security.yaml create mode 100644 tests/fixtures/MakeAuthenticatorMultipleFirewallsExistingAuthenticator/src/Security/Authenticator.php diff --git a/src/Maker/MakeAuthenticator.php b/src/Maker/MakeAuthenticator.php index 089dca7c3..5d8f83d4a 100644 --- a/src/Maker/MakeAuthenticator.php +++ b/src/Maker/MakeAuthenticator.php @@ -37,10 +37,13 @@ final class MakeAuthenticator extends AbstractMaker private $configUpdater; - public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater) + private $generator; + + public function __construct(FileManager $fileManager, SecurityConfigUpdater $configUpdater, Generator $generator) { $this->fileManager = $fileManager; $this->configUpdater = $configUpdater; + $this->generator = $generator; } public static function getCommandName(): string @@ -69,8 +72,12 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma $securityData = $manipulator->getData(); $interactiveSecurityHelper = new InteractiveSecurityHelper(); + $command->addOption('firewall-name', null, InputOption::VALUE_OPTIONAL, ''); $interactiveSecurityHelper->guessFirewallName($input, $io, $securityData); + + $command->addOption('entry-point', null, InputOption::VALUE_OPTIONAL); + $interactiveSecurityHelper->guessEntryPoint($input, $io, $this->generator, $securityData); } public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) @@ -93,6 +100,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $newYaml = $this->configUpdater->updateForAuthenticator( $this->fileManager->getFileContents($path), $input->getOption('firewall-name'), + $input->getOption('entry-point'), $classNameDetails->getFullName() ); $generator->dumpFile($path, $newYaml); diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index b8ce52956..3037ce68e 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -10,6 +10,7 @@ + diff --git a/src/Security/InteractiveSecurityHelper.php b/src/Security/InteractiveSecurityHelper.php index 33e8dcec9..7ff0e2685 100644 --- a/src/Security/InteractiveSecurityHelper.php +++ b/src/Security/InteractiveSecurityHelper.php @@ -11,18 +11,25 @@ namespace Symfony\Bundle\MakerBundle\Security; -use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; +use Symfony\Bundle\MakerBundle\Generator; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; /** * @internal */ final class InteractiveSecurityHelper { - public function guessFirewallName(InputInterface $input, ConsoleStyle $io, array $securityData) + /** + * @param InputInterface $input + * @param SymfonyStyle $io + * @param array $securityData + */ + public function guessFirewallName(InputInterface $input, SymfonyStyle $io, array $securityData) { $firewalls = array_filter( - $securityData['security']['firewalls'], + $securityData['security']['firewalls'] ?? [], function ($item) { return !isset($item['security']) || true === $item['security']; } @@ -41,4 +48,47 @@ function ($item) { $firewallName = $io->choice('Which firewall you want to update ?', array_keys($firewalls), key($firewalls)); $input->setOption('firewall-name', $firewallName); } + + /** + * @param InputInterface $input + * @param SymfonyStyle $io + * @param Generator $generator + * @param array $securityData + */ + public function guessEntryPoint(InputInterface $input, SymfonyStyle $io, Generator $generator, array $securityData) + { + $firewallName = $input->getOption('firewall-name'); + if (!$firewallName) { + throw new RuntimeCommandException("Option \"firewall-name\" must be provided."); + } + + if (!isset($securityData['security'])) { + $securityData['security'] = []; + } + + if (!isset($securityData['security']['firewalls'])) { + $securityData['security']['firewalls'] = []; + } + + $firewalls = $securityData['security']['firewalls']; + if (!isset($firewalls[$firewallName])) { + throw new RuntimeCommandException("Firewall \"$firewallName\" does not exist"); + } + + if (!isset($firewalls[$firewallName]['guard']) + || !isset($firewalls[$firewallName]['guard']['authenticators']) + || !$firewalls[$firewallName]['guard']['authenticators']) { + return; + } + + $authenticators = $firewalls[$firewallName]['guard']['authenticators']; + $classNameDetails = $generator->createClassNameDetails( + $input->getArgument('authenticator-class'), + 'Security\\' + ); + $authenticators[] = $classNameDetails->getFullName(); + + $entryPoint = $io->choice('Which authenticator will be the entry point ?', $authenticators, current($authenticators)); + $input->setOption('entry-point', $entryPoint); + } } diff --git a/src/Security/SecurityConfigUpdater.php b/src/Security/SecurityConfigUpdater.php index a0ba21655..309ba54e7 100644 --- a/src/Security/SecurityConfigUpdater.php +++ b/src/Security/SecurityConfigUpdater.php @@ -48,7 +48,15 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u return $contents; } - public function updateForAuthenticator(string $yamlSource, string $firewallName, string $userClass) + /** + * @param string $yamlSource + * @param string $firewallName + * @param string $chosenEntryPoint + * @param string $authenticatorClass + * + * @return string + */ + public function updateForAuthenticator(string $yamlSource, string $firewallName, $chosenEntryPoint, string $authenticatorClass) { $this->manipulator = new YamlSourceManipulator($yamlSource); @@ -74,10 +82,10 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName, $firewall['guard']['authenticators'] = []; } - $firewall['guard']['authenticators'][] = $userClass; + $firewall['guard']['authenticators'][] = $authenticatorClass; if (count($firewall['guard']['authenticators']) > 1) { - $firewall['guard']['entry_point'] = current($firewall['guard']['authenticators']); + $firewall['guard']['entry_point'] = $chosenEntryPoint ?? current($firewall['guard']['authenticators']); } $newData['security']['firewalls'][$firewallName] = $firewall; diff --git a/tests/Maker/FunctionalTest.php b/tests/Maker/FunctionalTest.php index c91a48615..aec4d73dc 100644 --- a/tests/Maker/FunctionalTest.php +++ b/tests/Maker/FunctionalTest.php @@ -342,6 +342,67 @@ function (string $output, string $directory) { ), ]; + yield 'auth_empty_existing_authenticator' => [ + MakerTestDetails::createTest( + $this->getMakerInstance(MakeAuthenticator::class), + [ + // class name + 'AppCustomAuthenticator', + 1 + ] + ) + ->addExtraDependencies('security') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorExistingAuthenticator') + ->setRequiredPhpVersion(70100) + ->assert( + function (string $output, string $directory) { + $this->assertContains('Success', $output); + + $finder = new Finder(); + $finder->in($directory.'/src/Security') + ->name('AppCustomAuthenticator.php'); + $this->assertCount(1, $finder); + + $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $this->assertEquals( + 'App\\Security\\AppCustomAuthenticator', + $securityConfig['security']['firewalls']['main']['guard']['entry_point'] + ); + } + ), + ]; + + yield 'auth_empty_multiple_firewalls_existing_authenticator' => [ + MakerTestDetails::createTest( + $this->getMakerInstance(MakeAuthenticator::class), + [ + // class name + 'AppCustomAuthenticator', + 1, + 1 + ] + ) + ->addExtraDependencies('security') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewallsExistingAuthenticator') + ->setRequiredPhpVersion(70100) + ->assert( + function (string $output, string $directory) { + $this->assertContains('Success', $output); + + $finder = new Finder(); + $finder->in($directory.'/src/Security') + ->name('AppCustomAuthenticator.php'); + $this->assertCount(1, $finder); + + $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $this->assertEquals( + 'App\\Security\\AppCustomAuthenticator', + $securityConfig['security']['firewalls']['second']['guard']['entry_point'] + ); + } + ), + ]; + yield 'user_security_entity_with_password' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeUser::class), [ diff --git a/tests/Security/InteractiveSecurityHelperTest.php b/tests/Security/InteractiveSecurityHelperTest.php new file mode 100644 index 000000000..6fb46d702 --- /dev/null +++ b/tests/Security/InteractiveSecurityHelperTest.php @@ -0,0 +1,175 @@ +createMock(InputInterface::class); + $input->expects($this->once()) + ->method('setOption') + ->with('firewall-name', $expectedFirewallName); + + /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ + $io = $this->createMock(SymfonyStyle::class); + $io->expects($this->exactly(false === $multipleValues ? 0 : 1)) + ->method('choice') + ->willReturn($expectedFirewallName); + + $helper = new InteractiveSecurityHelper(); + $helper->guessFirewallName($input, $io, $securityData); + } + + public function getFirewallNameTests() + { + yield 'empty_security' => [ + [], + 'main', + ]; + + yield 'no_firewall' => [ + ['security' => ['firewalls' => []]], + 'main', + ]; + + yield 'no_secured_firewall' => [ + ['security' => ['firewalls' => ['dev' => ['security' => false]]]], + 'main', + ]; + + yield 'main_firewall' => [ + ['security' => ['firewalls' => ['dev' => ['security' => false], 'main' => null]]], + 'main', + ]; + + yield 'foo_firewall' => [ + ['security' => ['firewalls' => ['dev' => ['security' => false], 'foo' => null]]], + 'foo', + ]; + + yield 'foo_bar_firewalls_1' => [ + ['security' => ['firewalls' => ['dev' => ['security' => false], 'foo' => null, 'bar' => null]]], + 'foo', + true, + ]; + + yield 'foo_bar_firewalls_2' => [ + ['security' => ['firewalls' => ['dev' => ['security' => false], 'foo' => null, 'bar' => null]]], + 'bar', + true, + ]; + } + + /** + * @expectedException \Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException + */ + public function testGuessEntryPointWithNoFirewallNameThrowsException() + { + /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ + $input = $this->createMock(InputInterface::class); + + /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ + $io = $this->createMock(SymfonyStyle::class); + + /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ + $generator = $this->createMock(Generator::class); + + $helper = new InteractiveSecurityHelper(); + $helper->guessEntryPoint($input, $io, $generator, []); + } + + /** + * @expectedException \Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException + */ + public function testGuessEntryPointWithNonExistingFirewallThrowsException() + { + /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ + $input = $this->createMock(InputInterface::class); + $input->expects($this->once()) + ->method('getOption') + ->with('firewall-name') + ->willReturn('foo'); + + /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ + $io = $this->createMock(SymfonyStyle::class); + + /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ + $generator = $this->createMock(Generator::class); + + $helper = new InteractiveSecurityHelper(); + $helper->guessEntryPoint($input, $io, $generator, []); + } + + /** + * @dataProvider getEntryPointTests + */ + public function testGuestEntryPoint(array $securityData, string $firewallName, bool $multipleAuthenticators = false) + { + /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ + $input = $this->createMock(InputInterface::class); + $input->expects($this->once()) + ->method('getOption') + ->with('firewall-name') + ->willReturn($firewallName); + + $input->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) + ->method('getArgument') + ->with('authenticator-class') + ->willReturn('NewAuthenticator'); + + + /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ + $io = $this->createMock(SymfonyStyle::class); + $io->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) + ->method('choice'); + + /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ + $generator = $this->createMock(Generator::class); + $generator->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) + ->method('createClassNameDetails') + ->willReturn(new ClassNameDetails('App\\Security\\NewAuthenticator', 'App\\Security')); + + $helper = new InteractiveSecurityHelper(); + $helper->guessEntryPoint($input, $io, $generator, $securityData); + } + + public function getEntryPointTests() + { + yield 'no_guard' => [ + ['security' => ['firewalls' => ['main' => []]]], + 'main' + ]; + + yield 'no_authenticators_key' => [ + ['security' => ['firewalls' => ['main' => ['guard' => []]]]], + 'main' + ]; + + yield 'no_authenticator' => [ + ['security' => ['firewalls' => ['main' => ['guard' => ['authenticators' => []]]]]], + 'main' + ]; + + yield 'one_authenticator' => [ + ['security' => ['firewalls' => ['main' => ['guard' => ['authenticators' => ['App\\Security\\Authenticator']]]]]], + 'main', + true + ]; + } +} \ No newline at end of file diff --git a/tests/Security/SecurityConfigUpdaterTest.php b/tests/Security/SecurityConfigUpdaterTest.php index 9118104f6..528425618 100644 --- a/tests/Security/SecurityConfigUpdaterTest.php +++ b/tests/Security/SecurityConfigUpdaterTest.php @@ -71,11 +71,11 @@ public function getUserClassTests() /** * @dataProvider getAuthenticatorTests */ - public function testUpdateForAuthenticator(string $firewallName, string $expectedSourceFilename, string $startingSourceFilename) + public function testUpdateForAuthenticator(string $firewallName, $entryPoint, string $expectedSourceFilename, string $startingSourceFilename) { $updater = new SecurityConfigUpdater(); $source = file_get_contents(__DIR__.'/yaml_fixtures/source/'.$startingSourceFilename); - $actualSource = $updater->updateForAuthenticator($source, $firewallName, 'App\\Security\\AppCustomAuthenticator'); + $actualSource = $updater->updateForAuthenticator($source, $firewallName, $entryPoint, 'App\\Security\\AppCustomAuthenticator'); $expectedSource = file_get_contents(__DIR__.'/yaml_fixtures/expected_authenticator/'.$expectedSourceFilename); $this->assertSame($expectedSource, $actualSource); @@ -85,21 +85,37 @@ public function getAuthenticatorTests() { yield 'empty_source' => [ 'main', + null, 'empty_source.yaml', 'empty_security.yaml' ]; - // waiting for YamlSourceManipulator to be debugged -// yield 'empty_source' => [ -// 'main', -// 'simple_security_source.yaml', -// 'simple_security.yaml' -// ]; + yield 'empty_source' => [ + 'main', + null, + 'simple_security_source.yaml', + 'simple_security.yaml' + ]; + + yield 'simple_security_with_firewalls' => [ + 'main', + null, + 'simple_security_with_firewalls.yaml', + 'simple_security_with_firewalls.yaml' + ]; yield 'simple_security_with_firewalls' => [ 'main', + null, 'simple_security_with_firewalls.yaml', 'simple_security_with_firewalls.yaml' ]; + + yield 'simple_security_with_firewalls_and_authenticator' => [ + 'main', + 'App\\Security\\AppCustomAuthenticator', + 'simple_security_with_firewalls_and_authenticator.yaml', + 'simple_security_with_firewalls_and_authenticator.yaml' + ]; } } \ No newline at end of file diff --git a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml index 61b340dee..bcbbbef09 100644 --- a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml +++ b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml @@ -5,4 +5,7 @@ security: firewalls: dev: ~ - main: ~ + main: + guard: + authenticators: + - App\Security\AppCustomAuthenticator diff --git a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls_and_authenticator.yaml b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls_and_authenticator.yaml new file mode 100644 index 000000000..60428439f --- /dev/null +++ b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_with_firewalls_and_authenticator.yaml @@ -0,0 +1,18 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + guard: + authenticators: + - App\Security\Authenticator + - App\Security\AppCustomAuthenticator + entry_point: App\Security\AppCustomAuthenticator + foo: + anonymous: true diff --git a/tests/Security/yaml_fixtures/source/simple_security_with_firewalls_and_authenticator.yaml b/tests/Security/yaml_fixtures/source/simple_security_with_firewalls_and_authenticator.yaml new file mode 100644 index 000000000..58deab039 --- /dev/null +++ b/tests/Security/yaml_fixtures/source/simple_security_with_firewalls_and_authenticator.yaml @@ -0,0 +1,16 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + guard: + authenticators: + - App\Security\Authenticator + foo: + anonymous: true diff --git a/tests/fixtures/MakeAuthenticatorExistingAuthenticator/config/packages/security.yaml b/tests/fixtures/MakeAuthenticatorExistingAuthenticator/config/packages/security.yaml new file mode 100644 index 000000000..e92c5f287 --- /dev/null +++ b/tests/fixtures/MakeAuthenticatorExistingAuthenticator/config/packages/security.yaml @@ -0,0 +1,27 @@ +security: + # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers + providers: + in_memory: { memory: ~ } + firewalls: + dev: + pattern: ^/(_(profiler|wdt)|css|images|js)/ + security: false + main: + anonymous: true + guard: + authenticators: + - App\Security\Authenticator + + # activate different ways to authenticate + + # http_basic: true + # https://symfony.com/doc/current/security.html#a-configuring-how-your-users-will-authenticate + + # form_login: true + # https://symfony.com/doc/current/security/form_login_setup.html + + # Easy way to control access for large sections of your site + # Note: Only the *first* access control that matches will be used + access_control: + # - { path: ^/admin, roles: ROLE_ADMIN } + # - { path: ^/profile, roles: ROLE_USER } diff --git a/tests/fixtures/MakeAuthenticatorExistingAuthenticator/src/Security/Authenticator.php b/tests/fixtures/MakeAuthenticatorExistingAuthenticator/src/Security/Authenticator.php new file mode 100644 index 000000000..bb2a7859c --- /dev/null +++ b/tests/fixtures/MakeAuthenticatorExistingAuthenticator/src/Security/Authenticator.php @@ -0,0 +1,53 @@ + Date: Thu, 6 Sep 2018 18:24:26 +0200 Subject: [PATCH 4/5] fixes after PR --- src/Maker/MakeAuthenticator.php | 34 +++++--- src/Security/InteractiveSecurityHelper.php | 64 ++++++--------- src/Security/SecurityConfigUpdater.php | 14 +--- src/Util/YamlSourceManipulator.php | 6 +- tests/Maker/FunctionalTest.php | 38 +++------ .../InteractiveSecurityHelperTest.php | 82 ++++--------------- tests/Security/SecurityConfigUpdaterTest.php | 2 +- .../expected_authenticator/empty_source.yaml | 1 + .../simple_security_source.yaml | 1 + 9 files changed, 82 insertions(+), 160 deletions(-) diff --git a/src/Maker/MakeAuthenticator.php b/src/Maker/MakeAuthenticator.php index 5d8f83d4a..55169666b 100644 --- a/src/Maker/MakeAuthenticator.php +++ b/src/Maker/MakeAuthenticator.php @@ -20,16 +20,16 @@ use Symfony\Bundle\MakerBundle\Security\SecurityConfigUpdater; use Symfony\Bundle\MakerBundle\Util\YamlManipulationFailedException; use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator; +use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Exception\InvalidOptionException; use Symfony\Component\Console\Input\InputArgument; -use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Filesystem\Filesystem; /** * @author Ryan Weaver + * + * @internal */ final class MakeAuthenticator extends AbstractMaker { @@ -56,15 +56,12 @@ public function configureCommand(Command $command, InputConfiguration $inputConf $command ->setDescription('Creates an empty Guard authenticator') ->addArgument('authenticator-class', InputArgument::OPTIONAL, 'The class name of the authenticator to create (e.g. AppCustomAuthenticator)') - ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeAuth.txt')) - ; + ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeAuth.txt')); } public function interact(InputInterface $input, ConsoleStyle $io, Command $command) { - $fs = new Filesystem(); - - if (!$fs->exists($path = 'config/packages/security.yaml')) { + if (!$this->fileManager->fileExists($path = 'config/packages/security.yaml')) { return; } @@ -74,10 +71,19 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma $interactiveSecurityHelper = new InteractiveSecurityHelper(); $command->addOption('firewall-name', null, InputOption::VALUE_OPTIONAL, ''); - $interactiveSecurityHelper->guessFirewallName($input, $io, $securityData); + $input->setOption('firewall-name', $firewallName = $interactiveSecurityHelper->guessFirewallName($io, $securityData)); $command->addOption('entry-point', null, InputOption::VALUE_OPTIONAL); - $interactiveSecurityHelper->guessEntryPoint($input, $io, $this->generator, $securityData); + + $authenticatorClassNameDetails = $this->generator->createClassNameDetails( + $input->getArgument('authenticator-class'), + 'Security\\' + ); + + $input->setOption( + 'entry-point', + $interactiveSecurityHelper->guessEntryPoint($io, $securityData, $authenticatorClassNameDetails->getFullName(), $firewallName) + ); } public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) @@ -115,7 +121,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $text = ['Next: Customize your new authenticator.']; if (!$securityYamlUpdated) { - $text[] = 'Then, configure the "guard" key on your firewall to use it.'; + $yamlExample = $this->configUpdater->updateForAuthenticator( + 'security: {}', + 'main', + null, + $classNameDetails->getFullName() + ); + $text[] = "Your security.yaml could not be updated automatically. You'll need to add the following config manually:\n\n".$yamlExample; } $io->text($text); } diff --git a/src/Security/InteractiveSecurityHelper.php b/src/Security/InteractiveSecurityHelper.php index 7ff0e2685..8b46e65cf 100644 --- a/src/Security/InteractiveSecurityHelper.php +++ b/src/Security/InteractiveSecurityHelper.php @@ -12,8 +12,6 @@ namespace Symfony\Bundle\MakerBundle\Security; use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; -use Symfony\Bundle\MakerBundle\Generator; -use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Style\SymfonyStyle; /** @@ -21,47 +19,28 @@ */ final class InteractiveSecurityHelper { - /** - * @param InputInterface $input - * @param SymfonyStyle $io - * @param array $securityData - */ - public function guessFirewallName(InputInterface $input, SymfonyStyle $io, array $securityData) + public function guessFirewallName(SymfonyStyle $io, array $securityData) { - $firewalls = array_filter( + $realFirewalls = array_filter( $securityData['security']['firewalls'] ?? [], function ($item) { return !isset($item['security']) || true === $item['security']; } ); - if (count($firewalls) < 2) { - if ($firewalls) { - $input->setOption('firewall-name', key($firewalls)); - } else { - $input->setOption('firewall-name', 'main'); - } + if (0 === \count($realFirewalls)) { + return 'main'; + } - return; + if (1 === \count($realFirewalls)) { + return key($realFirewalls); } - $firewallName = $io->choice('Which firewall you want to update ?', array_keys($firewalls), key($firewalls)); - $input->setOption('firewall-name', $firewallName); + return $io->choice('Which firewall do you want to update ?', array_keys($realFirewalls), key($realFirewalls)); } - /** - * @param InputInterface $input - * @param SymfonyStyle $io - * @param Generator $generator - * @param array $securityData - */ - public function guessEntryPoint(InputInterface $input, SymfonyStyle $io, Generator $generator, array $securityData) + public function guessEntryPoint(SymfonyStyle $io, array $securityData, string $authenticatorClass, string $firewallName) { - $firewallName = $input->getOption('firewall-name'); - if (!$firewallName) { - throw new RuntimeCommandException("Option \"firewall-name\" must be provided."); - } - if (!isset($securityData['security'])) { $securityData['security'] = []; } @@ -72,23 +51,28 @@ public function guessEntryPoint(InputInterface $input, SymfonyStyle $io, Generat $firewalls = $securityData['security']['firewalls']; if (!isset($firewalls[$firewallName])) { - throw new RuntimeCommandException("Firewall \"$firewallName\" does not exist"); + throw new RuntimeCommandException(sprintf('Firewall "%s" does not exist', $firewallName)); } if (!isset($firewalls[$firewallName]['guard']) || !isset($firewalls[$firewallName]['guard']['authenticators']) - || !$firewalls[$firewallName]['guard']['authenticators']) { - return; + || !$firewalls[$firewallName]['guard']['authenticators'] + || isset($firewalls[$firewallName]['guard']['entry_point'])) { + return null; } $authenticators = $firewalls[$firewallName]['guard']['authenticators']; - $classNameDetails = $generator->createClassNameDetails( - $input->getArgument('authenticator-class'), - 'Security\\' - ); - $authenticators[] = $classNameDetails->getFullName(); + $authenticators[] = $authenticatorClass; - $entryPoint = $io->choice('Which authenticator will be the entry point ?', $authenticators, current($authenticators)); - $input->setOption('entry-point', $entryPoint); + return $io->choice( + 'The entry point for your firewall is what should happen when an anonymous user tries to access +a protected page. For example, a common "entry point" behavior is to redirect to the login page. +The "entry point" behavior is controlled by the start() method on your authenticator. +However, you will now have multiple authenticators. You need to choose which authenticator\'s +start() method should be used as the entry point (the start() method on all other +authenticators will be ignored, and can be blank.', + $authenticators, + current($authenticators) + ); } } diff --git a/src/Security/SecurityConfigUpdater.php b/src/Security/SecurityConfigUpdater.php index 309ba54e7..bf0d84da3 100644 --- a/src/Security/SecurityConfigUpdater.php +++ b/src/Security/SecurityConfigUpdater.php @@ -48,15 +48,7 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u return $contents; } - /** - * @param string $yamlSource - * @param string $firewallName - * @param string $chosenEntryPoint - * @param string $authenticatorClass - * - * @return string - */ - public function updateForAuthenticator(string $yamlSource, string $firewallName, $chosenEntryPoint, string $authenticatorClass) + public function updateForAuthenticator(string $yamlSource, string $firewallName, $chosenEntryPoint, string $authenticatorClass): string { $this->manipulator = new YamlSourceManipulator($yamlSource); @@ -69,7 +61,7 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName, } if (!isset($newData['security']['firewalls'][$firewallName])) { - $newData['security']['firewalls'][$firewallName] = []; + $newData['security']['firewalls'][$firewallName] = ['anonymous' => true]; } $firewall = $newData['security']['firewalls'][$firewallName]; @@ -84,7 +76,7 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName, $firewall['guard']['authenticators'][] = $authenticatorClass; - if (count($firewall['guard']['authenticators']) > 1) { + if (\count($firewall['guard']['authenticators']) > 1) { $firewall['guard']['entry_point'] = $chosenEntryPoint ?? current($firewall['guard']['authenticators']); } diff --git a/src/Util/YamlSourceManipulator.php b/src/Util/YamlSourceManipulator.php index 65dcae266..2f0585122 100644 --- a/src/Util/YamlSourceManipulator.php +++ b/src/Util/YamlSourceManipulator.php @@ -450,7 +450,7 @@ private function changeValueInYaml($value) $endValuePosition = $this->findEndPositionOfValue($originalVal); $newYamlValue = $this->convertToYaml($value); - if (!is_array($originalVal) && is_array($value)) { + if (!\is_array($originalVal) && \is_array($value)) { // we're converting from a scalar to a (multiline) array // this means we need to break onto the next line @@ -798,7 +798,7 @@ private function advanceCurrentPosition(int $newPosition) * to look for indentation. */ if ($this->isCharLineBreak(substr($this->contents, $originalPosition - 1, 1))) { - $originalPosition--; + --$originalPosition; } // look for empty lines and track the current indentation @@ -890,7 +890,7 @@ private function guessNextArrayTypeAndAdvance(): string // because we are either moving along one line until [ { // or we are finding a line break and stopping: indentation // should not be calculated - $this->currentPosition++; + ++$this->currentPosition; if ($this->isCharLineBreak($nextCharacter)) { return self::ARRAY_FORMAT_MULTILINE; diff --git a/tests/Maker/FunctionalTest.php b/tests/Maker/FunctionalTest.php index aec4d73dc..7c39e97bf 100644 --- a/tests/Maker/FunctionalTest.php +++ b/tests/Maker/FunctionalTest.php @@ -32,6 +32,7 @@ use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Finder\Finder; use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\Routing\RouteCollectionBuilder; @@ -293,17 +294,14 @@ public function getCommandTests() ) ->addExtraDependencies('security') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticator') - ->setRequiredPhpVersion(70100) ->assert( function (string $output, string $directory) { $this->assertContains('Success', $output); - $finder = new Finder(); - $finder->in($directory.'/src/Security') - ->name('AppCustomAuthenticator.php'); - $this->assertCount(1, $finder); + $fs = new Filesystem(); + $this->assertTrue($fs->exists(sprintf('%s/src/Security/AppCustomAuthenticator.php', $directory))); - $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory))); $this->assertEquals( 'App\\Security\\AppCustomAuthenticator', $securityConfig['security']['firewalls']['main']['guard']['authenticators'][0] @@ -318,22 +316,17 @@ function (string $output, string $directory) { [ // class name 'AppCustomAuthenticator', + // firewall name 1 ] ) ->addExtraDependencies('security') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewalls') - ->setRequiredPhpVersion(70100) ->assert( function (string $output, string $directory) { $this->assertContains('Success', $output); - $finder = new Finder(); - $finder->in($directory.'/src/Security') - ->name('AppCustomAuthenticator.php'); - $this->assertCount(1, $finder); - - $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory))); $this->assertEquals( 'App\\Security\\AppCustomAuthenticator', $securityConfig['security']['firewalls']['second']['guard']['authenticators'][0] @@ -348,22 +341,17 @@ function (string $output, string $directory) { [ // class name 'AppCustomAuthenticator', + // firewall name 1 ] ) ->addExtraDependencies('security') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorExistingAuthenticator') - ->setRequiredPhpVersion(70100) ->assert( function (string $output, string $directory) { $this->assertContains('Success', $output); - $finder = new Finder(); - $finder->in($directory.'/src/Security') - ->name('AppCustomAuthenticator.php'); - $this->assertCount(1, $finder); - - $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory))); $this->assertEquals( 'App\\Security\\AppCustomAuthenticator', $securityConfig['security']['firewalls']['main']['guard']['entry_point'] @@ -378,23 +366,19 @@ function (string $output, string $directory) { [ // class name 'AppCustomAuthenticator', + // firewall name 1, + // entry point 1 ] ) ->addExtraDependencies('security') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeAuthenticatorMultipleFirewallsExistingAuthenticator') - ->setRequiredPhpVersion(70100) ->assert( function (string $output, string $directory) { $this->assertContains('Success', $output); - $finder = new Finder(); - $finder->in($directory.'/src/Security') - ->name('AppCustomAuthenticator.php'); - $this->assertCount(1, $finder); - - $securityConfig = Yaml::parse(file_get_contents("$directory/config/packages/security.yaml")); + $securityConfig = Yaml::parse(file_get_contents(sprintf("%s/config/packages/security.yaml", $directory))); $this->assertEquals( 'App\\Security\\AppCustomAuthenticator', $securityConfig['security']['firewalls']['second']['guard']['entry_point'] diff --git a/tests/Security/InteractiveSecurityHelperTest.php b/tests/Security/InteractiveSecurityHelperTest.php index 6fb46d702..180a21d34 100644 --- a/tests/Security/InteractiveSecurityHelperTest.php +++ b/tests/Security/InteractiveSecurityHelperTest.php @@ -3,29 +3,16 @@ namespace Symfony\Bundle\MakerBundle\Tests\Security; use PHPUnit\Framework\TestCase; -use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\Security\InteractiveSecurityHelper; -use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; -use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Style\SymfonyStyle; class InteractiveSecurityHelperTest extends TestCase { /** * @dataProvider getFirewallNameTests - * - * @param array $securityData - * @param string $expectedFirewallName - * @param bool $multipleValues */ public function testGuessFirewallName(array $securityData, string $expectedFirewallName, $multipleValues = false) { - /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ - $input = $this->createMock(InputInterface::class); - $input->expects($this->once()) - ->method('setOption') - ->with('firewall-name', $expectedFirewallName); - /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ $io = $this->createMock(SymfonyStyle::class); $io->expects($this->exactly(false === $multipleValues ? 0 : 1)) @@ -33,7 +20,10 @@ public function testGuessFirewallName(array $securityData, string $expectedFirew ->willReturn($expectedFirewallName); $helper = new InteractiveSecurityHelper(); - $helper->guessFirewallName($input, $io, $securityData); + $this->assertEquals( + $expectedFirewallName, + $helper->guessFirewallName($io, $securityData) + ); } public function getFirewallNameTests() @@ -76,44 +66,16 @@ public function getFirewallNameTests() ]; } - /** - * @expectedException \Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException - */ - public function testGuessEntryPointWithNoFirewallNameThrowsException() - { - /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ - $input = $this->createMock(InputInterface::class); - - /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ - $io = $this->createMock(SymfonyStyle::class); - - /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ - $generator = $this->createMock(Generator::class); - - $helper = new InteractiveSecurityHelper(); - $helper->guessEntryPoint($input, $io, $generator, []); - } - /** * @expectedException \Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException */ public function testGuessEntryPointWithNonExistingFirewallThrowsException() { - /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ - $input = $this->createMock(InputInterface::class); - $input->expects($this->once()) - ->method('getOption') - ->with('firewall-name') - ->willReturn('foo'); - /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ $io = $this->createMock(SymfonyStyle::class); - /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ - $generator = $this->createMock(Generator::class); - $helper = new InteractiveSecurityHelper(); - $helper->guessEntryPoint($input, $io, $generator, []); + $helper->guessEntryPoint($io, [], '', 'foo'); } /** @@ -121,55 +83,41 @@ public function testGuessEntryPointWithNonExistingFirewallThrowsException() */ public function testGuestEntryPoint(array $securityData, string $firewallName, bool $multipleAuthenticators = false) { - /** @var InputInterface|\PHPUnit_Framework_MockObject_MockObject $input */ - $input = $this->createMock(InputInterface::class); - $input->expects($this->once()) - ->method('getOption') - ->with('firewall-name') - ->willReturn($firewallName); - - $input->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) - ->method('getArgument') - ->with('authenticator-class') - ->willReturn('NewAuthenticator'); - - /** @var SymfonyStyle|\PHPUnit_Framework_MockObject_MockObject $io */ $io = $this->createMock(SymfonyStyle::class); $io->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) ->method('choice'); - /** @var Generator|\PHPUnit_Framework_MockObject_MockObject $generator */ - $generator = $this->createMock(Generator::class); - $generator->expects($this->exactly(false === $multipleAuthenticators ? 0 : 1)) - ->method('createClassNameDetails') - ->willReturn(new ClassNameDetails('App\\Security\\NewAuthenticator', 'App\\Security')); - $helper = new InteractiveSecurityHelper(); - $helper->guessEntryPoint($input, $io, $generator, $securityData); + $helper->guessEntryPoint($io, $securityData, 'App\\Security\\NewAuthenticator', $firewallName); } public function getEntryPointTests() { yield 'no_guard' => [ ['security' => ['firewalls' => ['main' => []]]], - 'main' + 'main', ]; yield 'no_authenticators_key' => [ ['security' => ['firewalls' => ['main' => ['guard' => []]]]], - 'main' + 'main', ]; yield 'no_authenticator' => [ ['security' => ['firewalls' => ['main' => ['guard' => ['authenticators' => []]]]]], - 'main' + 'main', ]; yield 'one_authenticator' => [ ['security' => ['firewalls' => ['main' => ['guard' => ['authenticators' => ['App\\Security\\Authenticator']]]]]], 'main', - true + true, + ]; + + yield 'one_authenticator' => [ + ['security' => ['firewalls' => ['main' => ['guard' => ['entry_point' => 'App\\Security\\Authenticator', 'authenticators' => ['App\\Security\\Authenticator']]]]]], + 'main', ]; } } \ No newline at end of file diff --git a/tests/Security/SecurityConfigUpdaterTest.php b/tests/Security/SecurityConfigUpdaterTest.php index 528425618..ec5379f8f 100644 --- a/tests/Security/SecurityConfigUpdaterTest.php +++ b/tests/Security/SecurityConfigUpdaterTest.php @@ -90,7 +90,7 @@ public function getAuthenticatorTests() 'empty_security.yaml' ]; - yield 'empty_source' => [ + yield 'simple_security' => [ 'main', null, 'simple_security_source.yaml', diff --git a/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml b/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml index 6b78efaa4..6ad70689c 100644 --- a/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml +++ b/tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml @@ -1,5 +1,6 @@ security: firewalls: main: + anonymous: true guard: authenticators: [App\Security\AppCustomAuthenticator] \ No newline at end of file diff --git a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml index bcbbbef09..f84a7ce38 100644 --- a/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml +++ b/tests/Security/yaml_fixtures/expected_authenticator/simple_security_source.yaml @@ -6,6 +6,7 @@ security: firewalls: dev: ~ main: + anonymous: true guard: authenticators: - App\Security\AppCustomAuthenticator From 3a26d9859a990d8963159c998eba572bbebb2bd6 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Fri, 7 Sep 2018 10:33:41 +0200 Subject: [PATCH 5/5] add return type to InteractiveSecurityHelper::guessFirewallName --- src/Security/InteractiveSecurityHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/InteractiveSecurityHelper.php b/src/Security/InteractiveSecurityHelper.php index 8b46e65cf..346fd0148 100644 --- a/src/Security/InteractiveSecurityHelper.php +++ b/src/Security/InteractiveSecurityHelper.php @@ -19,7 +19,7 @@ */ final class InteractiveSecurityHelper { - public function guessFirewallName(SymfonyStyle $io, array $securityData) + public function guessFirewallName(SymfonyStyle $io, array $securityData): string { $realFirewalls = array_filter( $securityData['security']['firewalls'] ?? [],