From cbfb3652d203bc23dcf77191c2367c95a97596ea Mon Sep 17 00:00:00 2001 From: DerManoMann Date: Mon, 22 Feb 2016 14:50:01 +1300 Subject: [PATCH] Refactor code to expect the logger as optional constructor arg and update tests --- src/LdapAuthenticationServiceProvider.php | 28 +++++---- .../Provider/LdapAuthenticationProvider.php | 2 +- ...ilex1LdapAuthenticationServiceProvider.php | 28 +++++---- .../LdapAuthenticationServiceProviderTest.php | 57 ++++++++++++------- tests/LdapTest.php | 31 +++++----- 5 files changed, 88 insertions(+), 58 deletions(-) diff --git a/src/LdapAuthenticationServiceProvider.php b/src/LdapAuthenticationServiceProvider.php index 2bd7cd9..1a05656 100755 --- a/src/LdapAuthenticationServiceProvider.php +++ b/src/LdapAuthenticationServiceProvider.php @@ -11,6 +11,7 @@ namespace Radebatz\Silex\LdapAuth; +use Psr\Log\LoggerInterface; use Pimple\Container; use Pimple\ServiceProviderInterface; use Zend\Ldap\Exception\LdapException; @@ -24,15 +25,18 @@ class LdapAuthenticationServiceProvider implements ServiceProviderInterface { protected $serviceName; + protected $logger; /** * Create new instance. * * @param string $serviceName Service name. + * @param Psr\Log\LoggerInterface $logger Optional logger. */ - public function __construct($serviceName = 'ldap') + public function __construct($serviceName = 'ldap', LoggerInterface $logger = null) { $this->serviceName = $serviceName; + $this->logger = $logger; } /** @@ -42,6 +46,8 @@ public function register(Container $app) { // our name $serviceName = $this->serviceName; + // a logger (or not); + $logger = $this->logger; $defaults = array( // authentication defaults @@ -75,7 +81,7 @@ public function register(Container $app) // the actual Ldap resource if (!isset($app['security.ldap.'.$serviceName.'.ldap'])) { - $app['security.ldap.'.$serviceName.'.ldap'] = function () use ($app, $serviceName) { + $app['security.ldap.'.$serviceName.'.ldap'] = function () use ($app, $serviceName, $logger) { // ldap options $options = $app['security.ldap.config']($serviceName)['ldap']; @@ -97,15 +103,15 @@ public function register(Container $app) return $ldap; } catch (LdapException $le) { - if ($app->offsetExists('logger')) { - $app['logger']->warning(sprintf('LDAP: Failed connecting to host: %s', $host)); + if ($logger) { + $logger->warning(sprintf('LDAP: Failed connecting to host: %s', $host)); } } } } - if ($app->offsetExists('logger')) { - $app['logger']->info(sprintf('LDAP: Using default host: %s', $options['host'])); + if ($logger) { + $logger->info(sprintf('LDAP: Using default host: %s', $options['host'])); } // just pass through all options using configured (single) host @@ -115,13 +121,13 @@ public function register(Container $app) // ready made user provider if (!isset($app['security.ldap.'.$serviceName.'.user_provider'])) { - $app['security.ldap.'.$serviceName.'.user_provider'] = $app->protect(function ($options = array()) use ($app, $serviceName) { - return new LdapUserProvider($serviceName, $app['security.ldap.'.$serviceName.'.ldap'], $app['logger'], $options); + $app['security.ldap.'.$serviceName.'.user_provider'] = $app->protect(function ($options = array()) use ($app, $serviceName, $logger) { + return new LdapUserProvider($serviceName, $app['security.ldap.'.$serviceName.'.ldap'], $logger, $options); }); } // set up authentication provider factory and user provider - $app['security.authentication_listener.factory.'.$serviceName] = $app->protect(function ($name, $options) use ($app, $serviceName) { + $app['security.authentication_listener.factory.'.$serviceName] = $app->protect(function ($name, $options) use ($app, $serviceName, $logger) { $serviceOptions = $app['security.ldap.config']($serviceName); $entryPoint = $serviceOptions['auth']['entryPoint']; @@ -130,12 +136,12 @@ public function register(Container $app) } // define the authentication provider object - $app['security.authentication_provider.'.$name.'.'.$serviceName] = function () use ($app, $name, $serviceOptions, $serviceName) { + $app['security.authentication_provider.'.$name.'.'.$serviceName] = function () use ($app, $name, $serviceOptions, $serviceName, $logger) { return new LdapAuthenticationProvider( $serviceName, $app['security.user_provider.'.$name], $app['security.ldap.'.$serviceName.'.ldap'], - $app['logger'], + $logger, $serviceOptions['auth'] ); }; diff --git a/src/Security/Core/Authentication/Provider/LdapAuthenticationProvider.php b/src/Security/Core/Authentication/Provider/LdapAuthenticationProvider.php index 20a27c9..80d12e8 100644 --- a/src/Security/Core/Authentication/Provider/LdapAuthenticationProvider.php +++ b/src/Security/Core/Authentication/Provider/LdapAuthenticationProvider.php @@ -36,7 +36,7 @@ class LdapAuthenticationProvider implements AuthenticationProviderInterface * @param string $providerKey The provider key. * @param UserProviderInterface $userProvider A user provider. * @param Zend\Ldap\Ldap $ldap Ldap serivce. - * @param Logger $logger Optional logger. + * @param Psr\Log\LoggerInterface $logger Optional logger. * @param array $options Options. */ public function __construct($providerKey, UserProviderInterface $userProvider, $ldap, LoggerInterface $logger = null, array $options = array()) diff --git a/src/Silex1LdapAuthenticationServiceProvider.php b/src/Silex1LdapAuthenticationServiceProvider.php index 9b33781..0b8afd1 100755 --- a/src/Silex1LdapAuthenticationServiceProvider.php +++ b/src/Silex1LdapAuthenticationServiceProvider.php @@ -11,6 +11,7 @@ namespace Radebatz\Silex\LdapAuth; +use Psr\Log\LoggerInterface; use Silex\Application; use Silex\ServiceProviderInterface; use Zend\Ldap\Exception\LdapException; @@ -24,15 +25,18 @@ class Silex1LdapAuthenticationServiceProvider implements ServiceProviderInterface { protected $serviceName; + protected $logger; /** * Create new instance. * * @param string $serviceName Service name. + * @param Psr\Log\LoggerInterface $logger Optional logger. */ - public function __construct($serviceName = 'ldap') + public function __construct($serviceName = 'ldap', LoggerInterface $logger = null) { $this->serviceName = $serviceName; + $this->logger = $logger; } /** @@ -42,6 +46,8 @@ public function register(Application $app) { // our name $serviceName = $this->serviceName; + // a logger (or not); + $logger = $this->logger; $defaults = array( // authentication defaults @@ -75,7 +81,7 @@ public function register(Application $app) // the actual Ldap resource if (!isset($app['security.ldap.'.$serviceName.'.ldap'])) { - $app['security.ldap.'.$serviceName.'.ldap'] = function () use ($app, $serviceName) { + $app['security.ldap.'.$serviceName.'.ldap'] = function () use ($app, $serviceName, $logger) { // ldap options $options = $app['security.ldap.config']($serviceName)['ldap']; @@ -97,15 +103,15 @@ public function register(Application $app) return $ldap; } catch (LdapException $le) { - if ($app->offsetExists('logger')) { - $app['logger']->warning(sprintf('LDAP: Failed connecting to host: %s', $host)); + if ($logger) { + $logger->warning(sprintf('LDAP: Failed connecting to host: %s', $host)); } } } } - if ($app->offsetExists('logger')) { - $app['logger']->info(sprintf('LDAP: Using default host: %s', $options['host'])); + if ($logger) { + $logger->info(sprintf('LDAP: Using default host: %s', $options['host'])); } // just pass through all options using configured (single) host @@ -115,13 +121,13 @@ public function register(Application $app) // ready made user provider if (!isset($app['security.ldap.'.$serviceName.'.user_provider'])) { - $app['security.ldap.'.$serviceName.'.user_provider'] = $app->protect(function ($options = array()) use ($app, $serviceName) { - return new LdapUserProvider($serviceName, $app['security.ldap.'.$serviceName.'.ldap'], $app['logger'], $options); + $app['security.ldap.'.$serviceName.'.user_provider'] = $app->protect(function ($options = array()) use ($app, $serviceName, $logger) { + return new LdapUserProvider($serviceName, $app['security.ldap.'.$serviceName.'.ldap'], $logger, $options); }); } // set up authentication provider factory and user provider - $app['security.authentication_listener.factory.'.$serviceName] = $app->protect(function ($name, $options) use ($app, $serviceName) { + $app['security.authentication_listener.factory.'.$serviceName] = $app->protect(function ($name, $options) use ($app, $serviceName, $logger) { $serviceOptions = $app['security.ldap.config']($serviceName); $entryPoint = $serviceOptions['auth']['entryPoint']; @@ -130,12 +136,12 @@ public function register(Application $app) } // define the authentication provider object - $app['security.authentication_provider.'.$name.'.'.$serviceName] = function () use ($app, $name, $serviceOptions, $serviceName) { + $app['security.authentication_provider.'.$name.'.'.$serviceName] = function () use ($app, $name, $serviceOptions, $serviceName, $logger) { return new LdapAuthenticationProvider( $serviceName, $app['security.user_provider.'.$name], $app['security.ldap.'.$serviceName.'.ldap'], - $app['logger'], + $logger, $serviceOptions['auth'] ); }; diff --git a/tests/LdapAuthenticationServiceProviderTest.php b/tests/LdapAuthenticationServiceProviderTest.php index 13d6f30..56522a3 100644 --- a/tests/LdapAuthenticationServiceProviderTest.php +++ b/tests/LdapAuthenticationServiceProviderTest.php @@ -27,9 +27,23 @@ */ class LdapAuthenticationServiceProviderTest extends LdapAuthTestCase { - public function testLdapHttpAuthentication() + public function loggerProvider() { - $app = $this->createApplication('http'); + $logger = new Logger('CLI'); + $logger->pushHandler(new StreamHandler('php://stdout', Logger::DEBUG)); + + return [ + 'null' => [null], + 'psr' => [$logger], + ]; + } + + /** + * @dataProvider loggerProvider + */ + public function testLdapHttpAuthentication($logger) + { + $app = $this->createApplication('http', $logger); $client = new Client($app); @@ -54,9 +68,12 @@ public function testLdapHttpAuthentication() $this->assertEquals('admin', $client->getResponse()->getContent()); } - public function testLdapFormAuthentication() + /** + * @dataProvider loggerProvider + */ + public function testLdapFormAuthentication($logger) { - $app = $this->createApplication('form'); + $app = $this->createApplication('form', $logger); $client = new Client($app); @@ -102,20 +119,15 @@ public function testLdapFormAuthentication() $this->assertEquals('admin', $client->getResponse()->getContent()); } - public function createApplication($authenticationMethod) + public function createApplication($authenticationMethod, $logger) { $app = new Application(); $app['debug'] = true; $app->register(new SessionServiceProvider()); - /* - $app['logger'] = new Logger('CLI'); - $app['logger']->pushHandler(new StreamHandler('php://stdout', Logger::DEBUG)); - */ - // ********* // $serviceName = 'ldap-'.$authenticationMethod; - $this->registerLdapAuthenticationServiceProvider($app, $authenticationMethod, $serviceName); + $this->registerLdapAuthenticationServiceProvider($app, $authenticationMethod, $serviceName, $logger); $app = call_user_func(array($this, 'add'.ucfirst($authenticationMethod ?: 'null').'Authentication'), $app, $serviceName); $app['session.test'] = true; @@ -123,18 +135,21 @@ public function createApplication($authenticationMethod) return $app; } - protected function registerLdapAuthenticationServiceProvider($app, $authenticationMethod, $serviceName) + protected function registerLdapAuthenticationServiceProvider($app, $authenticationMethod, $serviceName, $logger) { - $app->register(new LdapAuthenticationServiceProvider($serviceName), array( - 'security.ldap.'.$serviceName.'.options' => array_merge( - $this->getOptions(), - array( - 'auth' => array( - 'entryPoint' => $authenticationMethod, - ), + $app->register(new LdapAuthenticationServiceProvider($serviceName, $logger), + array( + 'security.ldap.'.$serviceName.'.options' => array_merge( + $this->getOptions(), + array( + 'auth' => array( + 'entryPoint' => $authenticationMethod, + ), + ) ) - ) - )); + ), + $app['logger'] + ); // need this before the firewall is configured $app['security.ldap.'.$serviceName.'.ldap'] = function () { diff --git a/tests/LdapTest.php b/tests/LdapTest.php index 9761acd..7bad590 100755 --- a/tests/LdapTest.php +++ b/tests/LdapTest.php @@ -23,43 +23,46 @@ class LdapTest extends LdapAuthTestCase { + public function loggerProvider() + { + $logger = new Logger('CLI'); + $logger->pushHandler(new StreamHandler('php://stdout', Logger::DEBUG)); + + return [ + 'null' => [null], + 'psr' => [$logger], + ]; + } + /** + * @dataProvider loggerProvider * @expectedException Symfony\Component\Security\Core\Exception\UsernameNotFoundException */ - public function testLdapExceptionSimple() + public function testLdapExceptionSimple($logger) { $app = new Application(); $app['debug'] = true; $app->register(new SessionServiceProvider()); - $app['logger'] = new Logger('CLI'); - $app['logger']->pushHandler(new StreamHandler('php://stdout', Logger::DEBUG)); - /* - */ - $serviceName = 'ldap-form'; - $app->register(new LdapAuthenticationServiceProvider($serviceName)); + $app->register(new LdapAuthenticationServiceProvider($serviceName, $logger)); // try the user provider with invalid Ldap configuration $app['security.ldap.'.$serviceName.'.user_provider']()->loadUserByUsername('mano'); } /** + * @dataProvider loggerProvider * @expectedException Symfony\Component\Security\Core\Exception\UsernameNotFoundException */ - public function testLdapExceptionHosts() + public function testLdapExceptionHosts($logger) { $app = new Application(); $app['debug'] = true; $app->register(new SessionServiceProvider()); - $app['logger'] = new Logger('CLI'); - $app['logger']->pushHandler(new StreamHandler('php://stdout', Logger::DEBUG)); - /* - */ - $serviceName = 'ldap-form'; - $app->register(new LdapAuthenticationServiceProvider($serviceName), array( + $app->register(new LdapAuthenticationServiceProvider($serviceName, $logger), array( 'security.ldap.'.$serviceName.'.options' => array( 'ldap' => array( 'hosts' => array(