From 5e3a44c2a15b012927b88856a65c9f1d079434ad Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 25 Feb 2016 14:48:42 +0100 Subject: [PATCH 1/3] Updated the authentication configuration to support credentials --- DependencyInjection/Configuration.php | 70 ++++++++++++++++--- DependencyInjection/HttplugExtension.php | 48 ++++++++++++- README.md | 30 +++++++- Resources/config/plugins.xml | 3 - .../DependencyInjection/ConfigurationTest.php | 8 +-- 5 files changed, 135 insertions(+), 24 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 8db08e1c..e9402aa3 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -120,17 +120,7 @@ protected function configurePlugins(ArrayNodeDefinition $root) ->arrayNode('plugins') ->addDefaultsIfNotSet() ->children() - - ->arrayNode('authentication') - ->canBeEnabled() - ->children() - ->scalarNode('authentication') - ->info('This must be a service id to a service implementing Http\Message\Authentication') - ->isRequired() - ->cannotBeEmpty() - ->end() - ->end() - ->end() // End authentication plugin + ->append($this->addAuthenticationPluiginNode()) ->arrayNode('cache') ->canBeEnabled() @@ -235,4 +225,62 @@ protected function configurePlugins(ArrayNodeDefinition $root) ->end() ->end(); } + + /** + * Add configuration for authentication plugin. + * + * @return ArrayNodeDefinition|\Symfony\Component\Config\Definition\Builder\NodeDefinition + */ + private function addAuthenticationPluiginNode() + { + $builder = new TreeBuilder(); + $node = $builder->root('authentication'); + $node + ->useAttributeAsKey('name') + ->prototype('array') + ->validate() + ->always() + ->then(function ($config) { + switch ($config['type']) { + case 'basic': + if (empty($config['username']) || empty($config['password'])) { + throw new InvalidConfigurationException('Authentication "basic" requires both "username" and "password".'); + } + break; + case 'bearer': + if (empty($config['token'])) { + throw new InvalidConfigurationException('Authentication "bearer" requires a "token".'); + } + break; + case 'service': + if (empty($config['service'])) { + throw new InvalidConfigurationException('Authentication "service" requires a "service".'); + } + break; + case 'wsse': + if (empty($config['username']) || empty($config['password'])) { + throw new InvalidConfigurationException('Authentication "wsse" requires both "username" and "password".'); + } + break; + } + + return $config; + }) + ->end() + ->children() + ->enumNode('type') + ->values(['basic', 'bearer', 'wsse', 'service']) + ->isRequired() + ->cannotBeEmpty() + ->end() + ->scalarNode('username')->end() + ->scalarNode('password')->end() + ->scalarNode('token')->end() + ->scalarNode('service')->end() + ->end() + ->end() + ->end(); // End authentication plugin + + return $node; + } } diff --git a/DependencyInjection/HttplugExtension.php b/DependencyInjection/HttplugExtension.php index 8909a91f..77b23e92 100644 --- a/DependencyInjection/HttplugExtension.php +++ b/DependencyInjection/HttplugExtension.php @@ -2,8 +2,12 @@ namespace Http\HttplugBundle\DependencyInjection; +use Http\Client\Plugin\AuthenticationPlugin; use Http\Client\Plugin\PluginClient; use Http\HttplugBundle\ClientFactory\DummyClient; +use Http\Message\Authentication\BasicAuth; +use Http\Message\Authentication\Bearer; +use Http\Message\Authentication\Wsse; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -106,6 +110,11 @@ private function configureClients(ContainerBuilder $container, array $config) */ private function configurePlugins(ContainerBuilder $container, array $config) { + if (!empty($config['authentication'])) { + $this->configureAuthentication($container, $config['authentication']); + } + unset($config['authentication']); + foreach ($config as $name => $pluginConfig) { $pluginId = 'httplug.plugin.'.$name; if ($pluginConfig['enabled']) { @@ -125,9 +134,6 @@ private function configurePlugins(ContainerBuilder $container, array $config) private function configurePluginByName($name, Definition $definition, array $config) { switch ($name) { - case 'authentication': - $definition->replaceArgument(0, new Reference($config['authentication'])); - break; case 'cache': $definition ->replaceArgument(0, new Reference($config['cache_pool'])) @@ -162,4 +168,40 @@ private function configurePluginByName($name, Definition $definition, array $con break; } } + + /** + * @param ContainerBuilder $container + * @param Definition $parent + * @param array $config + */ + private function configureAuthentication(ContainerBuilder $container, array $config) + { + foreach ($config as $name => $values) { + $authServiceKey = sprintf('httplug.plugin.authentication.%s.auth', $name); + switch ($values['type']) { + case 'bearer': + $container->register($authServiceKey, Bearer::class) + ->addArgument($values['token']); + break; + case 'basic': + $container->register($authServiceKey, BasicAuth::class) + ->addArgument($values['username']) + ->addArgument($values['password']); + break; + case 'wsse': + $container->register($authServiceKey, Wsse::class) + ->addArgument($values['username']) + ->addArgument($values['password']); + break; + case 'service': + $authServiceKey = $values['service']; + break; + default: + throw new \LogicException(sprintf('Unknown authentication type: "%s"', $values['type'])); + } + + $container->register('httplug.plugin.authentication.'.$name, AuthenticationPlugin::class) + ->addArgument(new Reference($authServiceKey)); + } + } } diff --git a/README.md b/README.md index d373aa44..fe965bf8 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ For information how to write applications with the services provided by this bun | httplug.client.[name] | This is your Httpclient that you have configured. With the configuration below the name would be `acme_client`. | httplug.client | This is the first client configured or a client named `default`. | httplug.plugin.content_length
httplug.plugin.decoder
httplug.plugin.error
httplug.plugin.logger
httplug.plugin.redirect
httplug.plugin.retry
httplug.plugin.stopwatch | These are plugins that are enabled by default. These services are not public and may only be used when configure HttpClients or other services. -| httplug.plugin.authentication
httplug.plugin.cache
httplug.plugin.cookie
httplug.plugin.history | These are plugins that are disabled by default. They need to be configured before they can be used. These services are not public and may only be used when configure HttpClients or other services. +| httplug.plugin.cache
httplug.plugin.cookie
httplug.plugin.history | These are plugins that are disabled by default. They need to be configured before they can be used. These services are not public and may only be used when configure HttpClients or other services. \* *These services are always an alias to another service. You can specify your own service or leave the default, which is the same name with `.default` appended. The default services in turn use the service discovery mechanism to provide the best available implementation. You can specify a class for each of the default services to use instead of discovery, as long as those classes can be instantiated without arguments.* @@ -134,6 +134,34 @@ httpug: base_uri: 'http://google.se/' ``` +#### Authentication + +```yaml +// config.yml +httpug: + plugins: + authentication: + my_basic: + type: 'basic' + username: 'my_username' + password: 'p4ssw0rd' + my_wsse: + type: 'wsse' + username: 'my_username' + password: 'p4ssw0rd' + my_brearer: + type: 'bearer' + token: 'authentication_token_hash' + my_service: + type: 'service' + service: 'my_authentication_service' + + clients: + acme: + factory: 'httplug.factory.guzzle6' + plugins: ['httplug.plugin.authentication.my_wsse'] +``` + ### Use for Reusable Bundles diff --git a/Resources/config/plugins.xml b/Resources/config/plugins.xml index e99fbe2e..3c7debac 100644 --- a/Resources/config/plugins.xml +++ b/Resources/config/plugins.xml @@ -4,9 +4,6 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - - diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index a1d37aa9..eef811e5 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -42,9 +42,7 @@ public function testEmptyConfiguration() 'formatter' => null, ], 'plugins' => [ - 'authentication' => [ - 'enabled' => false, - ], + 'authentication' => [], 'cache' => [ 'enabled' => false, 'stream_factory' => 'httplug.stream_factory', @@ -118,9 +116,7 @@ public function testSupportsAllConfigFormats() 'formatter' => null, ], 'plugins' => [ - 'authentication' => [ - 'enabled' => false, - ], + 'authentication' => [], 'cache' => [ 'enabled' => false, 'stream_factory' => 'httplug.stream_factory', From 31107960077657447ac348e5a57340cf5b163321 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Fri, 26 Feb 2016 19:30:18 +0100 Subject: [PATCH 2/3] validate type specific options --- DependencyInjection/Configuration.php | 44 +++++++++---- Tests/Resources/Fixtures/config/full.php | 54 ++++++++++++++++ Tests/Resources/Fixtures/config/full.xml | 19 ++++++ Tests/Resources/Fixtures/config/full.yml | 41 +++++++++++- .../Fixtures/config/invalid_auth.yml | 8 +++ .../config/{invalid.yml => invalid_class.yml} | 0 .../DependencyInjection/ConfigurationTest.php | 62 ++++++++++++++----- 7 files changed, 200 insertions(+), 28 deletions(-) create mode 100644 Tests/Resources/Fixtures/config/invalid_auth.yml rename Tests/Resources/Fixtures/config/{invalid.yml => invalid_class.yml} (100%) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index e9402aa3..c4b793bc 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -243,24 +243,16 @@ private function addAuthenticationPluiginNode() ->then(function ($config) { switch ($config['type']) { case 'basic': - if (empty($config['username']) || empty($config['password'])) { - throw new InvalidConfigurationException('Authentication "basic" requires both "username" and "password".'); - } + $this->validateAuthenticationType(['username', 'password'], $config, 'basic'); break; case 'bearer': - if (empty($config['token'])) { - throw new InvalidConfigurationException('Authentication "bearer" requires a "token".'); - } + $this->validateAuthenticationType(['token'], $config, 'bearer'); break; case 'service': - if (empty($config['service'])) { - throw new InvalidConfigurationException('Authentication "service" requires a "service".'); - } + $this->validateAuthenticationType(['service'], $config, 'service'); break; case 'wsse': - if (empty($config['username']) || empty($config['password'])) { - throw new InvalidConfigurationException('Authentication "wsse" requires both "username" and "password".'); - } + $this->validateAuthenticationType(['username', 'password'], $config, 'wsse'); break; } @@ -283,4 +275,32 @@ private function addAuthenticationPluiginNode() return $node; } + + /** + * Validate that the configuration fragment has the specified keys and none other. + * + * @param array $expected Fields that must exist + * @param array $actual Actual configuration hashmap + * @param string $authName Name of authentication method for error messages + * + * @throws InvalidConfigurationException If $actual does not have exactly the keys specified in $expected (plus 'type') + */ + private function validateAuthenticationType(array $expected, array $actual, $authName) + { + unset($actual['type']); + $actual = array_keys($actual); + sort($actual); + sort($expected); + + if ($expected === $actual) { + return; + } + + throw new InvalidConfigurationException(sprintf( + 'Authentication "%s" requires %s but got %s', + $authName, + implode(', ', $expected), + implode(', ', $actual) + )); + } } diff --git a/Tests/Resources/Fixtures/config/full.php b/Tests/Resources/Fixtures/config/full.php index 2883da4d..547d0548 100644 --- a/Tests/Resources/Fixtures/config/full.php +++ b/Tests/Resources/Fixtures/config/full.php @@ -13,4 +13,58 @@ 'uri_factory' => 'Http\Message\UriFactory\GuzzleUriFactory', 'stream_factory' => 'Http\Message\StreamFactory\GuzzleStreamFactory', ], + 'toolbar' => [ + 'enabled' => true, + 'formatter' => 'my_toolbar_formatter', + ], + 'plugins' => [ + 'authentication' => [ + 'my_basic' => [ + 'type' => 'basic', + 'username' => 'foo', + 'password' => 'bar', + ], + 'my_wsse' => [ + 'type' => 'wsse', + 'username' => 'foo', + 'password' => 'bar', + ], + 'my_brearer' => [ + 'type' => 'bearer', + 'token' => 'foo', + ], + 'my_service' => [ + 'type' => 'service', + 'service' => 'my_auth_serivce', + ], + ], + 'cache' => [ + 'stream_factory' => 'my_other_stream_factory', + 'config' => [ + 'default_ttl' => 42, + 'respect_cache_headers' => false, + ], + ], + 'cookie' => [ + 'cookie_jar' => 'my_cookie_jar', + ], + 'decoder' => [ + 'enabled' => false, + ], + 'history' => [ + 'journal' => 'my_journal', + ], + 'logger' => [ + 'enabled' => false, + ], + 'redirect' => [ + 'enabled' => false, + ], + 'retry' => [ + 'enabled' => false, + ], + 'stopwatch' => [ + 'enabled' => false, + ], + ], ]); diff --git a/Tests/Resources/Fixtures/config/full.xml b/Tests/Resources/Fixtures/config/full.xml index f3553a6f..69d1d507 100644 --- a/Tests/Resources/Fixtures/config/full.xml +++ b/Tests/Resources/Fixtures/config/full.xml @@ -14,6 +14,25 @@ Http\Message\UriFactory\GuzzleUriFactory Http\Message\StreamFactory\GuzzleStreamFactory + + + + + + + + + + + + + + + + + + + diff --git a/Tests/Resources/Fixtures/config/full.yml b/Tests/Resources/Fixtures/config/full.yml index 881b62a9..7b886b27 100644 --- a/Tests/Resources/Fixtures/config/full.yml +++ b/Tests/Resources/Fixtures/config/full.yml @@ -8,4 +8,43 @@ httplug: client: Http\Adapter\Guzzle6\Client message_factory: Http\Message\MessageFactory\GuzzleMessageFactory uri_factory: Http\Message\UriFactory\GuzzleUriFactory - stream_factory: Http\Message\StreamFactory\GuzzleStreamFactory \ No newline at end of file + stream_factory: Http\Message\StreamFactory\GuzzleStreamFactory + toolbar: + enabled: true + formatter: my_toolbar_formatter + plugins: + authentication: + my_basic: + type: basic + username: foo + password: bar + my_wsse: + type: wsse + username: foo + password: bar + my_brearer: + type: bearer + token: foo + my_service: + type: service + service: my_auth_serivce + cache: + cache_pool: my_cache_pool + stream_factory: my_other_stream_factory + config: + default_ttl: 42 + respect_cache_headers: false + cookie: + cookie_jar: my_cookie_jar + decoder: + enabled: false + history: + journal: my_journal + logger: + enabled: false + redirect: + enabled: false + retry: + enabled: false + stopwatch: + enabled: false diff --git a/Tests/Resources/Fixtures/config/invalid_auth.yml b/Tests/Resources/Fixtures/config/invalid_auth.yml new file mode 100644 index 00000000..185bcebe --- /dev/null +++ b/Tests/Resources/Fixtures/config/invalid_auth.yml @@ -0,0 +1,8 @@ +httplug: + plugins: + authentication: + my_auth: + type: service + service: foobar + username: user + password: pass diff --git a/Tests/Resources/Fixtures/config/invalid.yml b/Tests/Resources/Fixtures/config/invalid_class.yml similarity index 100% rename from Tests/Resources/Fixtures/config/invalid.yml rename to Tests/Resources/Fixtures/config/invalid_class.yml diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index eef811e5..1d64927d 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -112,45 +112,67 @@ public function testSupportsAllConfigFormats() ], 'clients' => [], 'toolbar' => [ - 'enabled' => 'auto', - 'formatter' => null, + 'enabled' => true, + 'formatter' => 'my_toolbar_formatter', ], 'plugins' => [ - 'authentication' => [], + 'authentication' => [ + 'my_basic' => [ + 'type' => 'basic', + 'username' => 'foo', + 'password' => 'bar', + ], + 'my_wsse' => [ + 'type' => 'wsse', + 'username' => 'foo', + 'password' => 'bar', + ], + 'my_brearer' => [ + 'type' => 'bearer', + 'token' => 'foo', + ], + 'my_service' => [ + 'type' => 'service', + 'service' => 'my_auth_serivce', + ], + ], 'cache' => [ - 'enabled' => false, - 'stream_factory' => 'httplug.stream_factory', + 'enabled' => true, + 'cache_pool' => 'my_cache_pool', + 'stream_factory' => 'my_other_stream_factory', 'config' => [ - 'default_ttl' => null, - 'respect_cache_headers' => true, + 'default_ttl' => 42, + 'respect_cache_headers' => false, ], ], 'cookie' => [ - 'enabled' => false, + 'enabled' => true, + 'cookie_jar' => 'my_cookie_jar', ], 'decoder' => [ - 'enabled' => true, + 'enabled' => false, 'use_content_encoding' => true, ], 'history' => [ - 'enabled' => false, + 'enabled' => true, + 'journal' => 'my_journal', ], 'logger' => [ - 'enabled' => true, + 'enabled' => false, 'logger' => 'logger', 'formatter' => null, ], 'redirect' => [ - 'enabled' => true, + 'enabled' => false, 'preserve_header' => true, 'use_default_for_multiple' => true, ], 'retry' => [ - 'enabled' => true, + 'enabled' => false, 'retry' => 1, ], 'stopwatch' => [ - 'enabled' => true, + 'enabled' => false, 'stopwatch' => 'debug.stopwatch', ], ], @@ -175,7 +197,17 @@ public function testSupportsAllConfigFormats() */ public function testMissingClass() { - $file = __DIR__.'/../../Resources/Fixtures/config/invalid.yml'; + $file = __DIR__.'/../../Resources/Fixtures/config/invalid_class.yml'; + $this->assertProcessedConfigurationEquals([], [$file]); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage password, service, username + */ + public function testInvalidAuthentication() + { + $file = __DIR__.'/../../Resources/Fixtures/config/invalid_auth.yml'; $this->assertProcessedConfigurationEquals([], [$file]); } } From 57de525d1cc098ef67f141c6ce3af8b7f0b8a0e1 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sun, 28 Feb 2016 09:24:09 +0100 Subject: [PATCH 3/3] Fixed typos in the test config --- Tests/Resources/Fixtures/config/full.php | 5 +++-- Tests/Resources/Fixtures/config/full.yml | 4 ++-- Tests/Unit/DependencyInjection/ConfigurationTest.php | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Tests/Resources/Fixtures/config/full.php b/Tests/Resources/Fixtures/config/full.php index 547d0548..5207ae33 100644 --- a/Tests/Resources/Fixtures/config/full.php +++ b/Tests/Resources/Fixtures/config/full.php @@ -29,16 +29,17 @@ 'username' => 'foo', 'password' => 'bar', ], - 'my_brearer' => [ + 'my_bearer' => [ 'type' => 'bearer', 'token' => 'foo', ], 'my_service' => [ 'type' => 'service', - 'service' => 'my_auth_serivce', + 'service' => 'my_auth_service', ], ], 'cache' => [ + 'cache_pool' => 'my_cache_pool', 'stream_factory' => 'my_other_stream_factory', 'config' => [ 'default_ttl' => 42, diff --git a/Tests/Resources/Fixtures/config/full.yml b/Tests/Resources/Fixtures/config/full.yml index 7b886b27..dbc85f6a 100644 --- a/Tests/Resources/Fixtures/config/full.yml +++ b/Tests/Resources/Fixtures/config/full.yml @@ -22,12 +22,12 @@ httplug: type: wsse username: foo password: bar - my_brearer: + my_bearer: type: bearer token: foo my_service: type: service - service: my_auth_serivce + service: my_auth_service cache: cache_pool: my_cache_pool stream_factory: my_other_stream_factory diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php index 1d64927d..bb709196 100644 --- a/Tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/Tests/Unit/DependencyInjection/ConfigurationTest.php @@ -127,13 +127,13 @@ public function testSupportsAllConfigFormats() 'username' => 'foo', 'password' => 'bar', ], - 'my_brearer' => [ + 'my_bearer' => [ 'type' => 'bearer', 'token' => 'foo', ], 'my_service' => [ 'type' => 'service', - 'service' => 'my_auth_serivce', + 'service' => 'my_auth_service', ], ], 'cache' => [