diff options
25 files changed, 669 insertions, 98 deletions
diff --git a/Core/Authentication/Provider/AuthenticationProviderInterface.php b/Core/Authentication/Provider/AuthenticationProviderInterface.php index adad258..f3e1590 100644 --- a/Core/Authentication/Provider/AuthenticationProviderInterface.php +++ b/Core/Authentication/Provider/AuthenticationProviderInterface.php @@ -25,6 +25,13 @@ use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterfac interface AuthenticationProviderInterface extends AuthenticationManagerInterface { /** + * Use this constant for not provided username. + * + * @var string + */ + const USERNAME_NONE_PROVIDED = 'NONE_PROVIDED'; + + /** * Checks whether this provider supports the given token. * * @param TokenInterface $token A TokenInterface instance diff --git a/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/Core/Authentication/Provider/LdapBindAuthenticationProvider.php index e887f99..5ebb09a 100644 --- a/Core/Authentication/Provider/LdapBindAuthenticationProvider.php +++ b/Core/Authentication/Provider/LdapBindAuthenticationProvider.php @@ -17,7 +17,7 @@ use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -use Symfony\Component\Ldap\LdapClientInterface; +use Symfony\Component\Ldap\LdapInterface; use Symfony\Component\Ldap\Exception\ConnectionException; /** @@ -40,11 +40,11 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider * @param UserProviderInterface $userProvider A UserProvider * @param UserCheckerInterface $userChecker A UserChecker * @param string $providerKey The provider key - * @param LdapClientInterface $ldap An Ldap client + * @param LdapInterface $ldap A Ldap client * @param string $dnString A string used to create the bind DN * @param bool $hideUserNotFoundExceptions Whether to hide user not found exception or not */ - public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, $providerKey, LdapClientInterface $ldap, $dnString = '{username}', $hideUserNotFoundExceptions = true) + public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, $providerKey, LdapInterface $ldap, $dnString = '{username}', $hideUserNotFoundExceptions = true) { parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions); @@ -58,7 +58,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider */ protected function retrieveUser($username, UsernamePasswordToken $token) { - if ('NONE_PROVIDED' === $username) { + if (AuthenticationProviderInterface::USERNAME_NONE_PROVIDED === $username) { throw new UsernameNotFoundException('Username can not be null'); } @@ -78,7 +78,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider } try { - $username = $this->ldap->escape($username, '', LDAP_ESCAPE_DN); + $username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN); $dn = str_replace('{username}', $username, $this->dnString); $this->ldap->bind($dn, $password); diff --git a/Core/Authentication/Provider/UserAuthenticationProvider.php b/Core/Authentication/Provider/UserAuthenticationProvider.php index 2674088..9dc4751 100644 --- a/Core/Authentication/Provider/UserAuthenticationProvider.php +++ b/Core/Authentication/Provider/UserAuthenticationProvider.php @@ -63,7 +63,7 @@ abstract class UserAuthenticationProvider implements AuthenticationProviderInter $username = $token->getUsername(); if ('' === $username || null === $username) { - $username = 'NONE_PROVIDED'; + $username = AuthenticationProviderInterface::USERNAME_NONE_PROVIDED; } try { diff --git a/Core/AuthenticationEvents.php b/Core/AuthenticationEvents.php index 13bce30..dfbd903 100644 --- a/Core/AuthenticationEvents.php +++ b/Core/AuthenticationEvents.php @@ -17,10 +17,7 @@ final class AuthenticationEvents * The AUTHENTICATION_SUCCESS event occurs after a user is authenticated * by one provider. * - * The event listener method receives a - * Symfony\Component\Security\Core\Event\AuthenticationEvent instance. - * - * @Event + * @Event("Symfony\Component\Security\Core\Event\AuthenticationEvent") * * @var string */ @@ -30,11 +27,7 @@ final class AuthenticationEvents * The AUTHENTICATION_FAILURE event occurs after a user cannot be * authenticated by any of the providers. * - * The event listener method receives a - * Symfony\Component\Security\Core\Event\AuthenticationFailureEvent - * instance. - * - * @Event + * @Event("Symfony\Component\Security\Core\Event\AuthenticationFailureEvent") * * @var string */ diff --git a/Core/Authorization/DebugAccessDecisionManager.php b/Core/Authorization/DebugAccessDecisionManager.php new file mode 100644 index 0000000..aa15443 --- /dev/null +++ b/Core/Authorization/DebugAccessDecisionManager.php @@ -0,0 +1,134 @@ +<?php + +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier <fabien@symfony.com> + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization; + +use Doctrine\Common\Util\ClassUtils; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; + +/** + * Decorates the original AccessDecisionManager class to log information + * about the security voters and the decisions made by them. + * + * @author Javier Eguiluz <javier.eguiluz@gmail.com> + * + * @internal + */ +class DebugAccessDecisionManager implements AccessDecisionManagerInterface +{ + private $manager; + private $strategy; + private $voters = array(); + private $decisionLog = array(); + + public function __construct(AccessDecisionManagerInterface $manager) + { + $this->manager = $manager; + + if ($this->manager instanceof AccessDecisionManager) { + // The strategy is stored in a private property of the decorated service + $reflection = new \ReflectionProperty(AccessDecisionManager::class, 'strategy'); + $reflection->setAccessible(true); + $this->strategy = $reflection->getValue($manager); + } + } + + /** + * {@inheritdoc} + */ + public function decide(TokenInterface $token, array $attributes, $object = null) + { + $result = $this->manager->decide($token, $attributes, $object); + + $this->decisionLog[] = array( + 'attributes' => $attributes, + 'object' => $this->getStringRepresentation($object), + 'result' => $result, + ); + + return $result; + } + + /** + * {@inheritdoc} + */ + public function setVoters(array $voters) + { + if (!method_exists($this->manager, 'setVoters')) { + return; + } + + $this->voters = $voters; + $this->manager->setVoters($voters); + } + + /** + * @return string + */ + public function getStrategy() + { + // The $strategy property is misleading because it stores the name of its + // method (e.g. 'decideAffirmative') instead of the original strategy name + // (e.g. 'affirmative') + return null === $this->strategy ? '-' : strtolower(substr($this->strategy, 6)); + } + + /** + * @return array + */ + public function getVoters() + { + return $this->voters; + } + + /** + * @return array + */ + public function getDecisionLog() + { + return $this->decisionLog; + } + + /** + * @param mixed $object + * + * @return string + */ + private function getStringRepresentation($object) + { + if (null === $object) { + return 'NULL'; + } + + if (!is_object($object)) { + if (is_bool($object)) { + return sprintf('%s (%s)', gettype($object), $object ? 'true' : 'false'); + } + if (is_scalar($object)) { + return sprintf('%s (%s)', gettype($object), $object); + } + + return gettype($object); + } + + $objectClass = class_exists('Doctrine\Common\Util\ClassUtils') ? ClassUtils::getClass($object) : get_class($object); + + if (method_exists($object, 'getId')) { + $objectAsString = sprintf('ID: %s', $object->getId()); + } elseif (method_exists($object, '__toString')) { + $objectAsString = (string) $object; + } else { + $objectAsString = sprintf('object hash: %s', spl_object_hash($object)); + } + + return sprintf('%s (%s)', $objectClass, $objectAsString); + } +} diff --git a/Core/Tests/Authentication/Provider/LdapBindAuthenticationProviderTest.php b/Core/Tests/Authentication/Provider/LdapBindAuthenticationProviderTest.php index fbb4d73..da3068f 100644 --- a/Core/Tests/Authentication/Provider/LdapBindAuthenticationProviderTest.php +++ b/Core/Tests/Authentication/Provider/LdapBindAuthenticationProviderTest.php @@ -11,10 +11,13 @@ namespace Symfony\Component\Security\Core\Tests\Authentication\Provider; +use Symfony\Component\Ldap\LdapInterface; use Symfony\Component\Security\Core\Authentication\Provider\LdapBindAuthenticationProvider; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\User\User; use Symfony\Component\Ldap\Exception\ConnectionException; +use Symfony\Component\Security\Core\User\UserCheckerInterface; +use Symfony\Component\Security\Core\User\UserProviderInterface; /** * @requires extension ldap @@ -44,14 +47,14 @@ class LdapBindAuthenticationProviderTest extends \PHPUnit_Framework_TestCase */ public function testBindFailureShouldThrowAnException() { - $userProvider = $this->getMock('Symfony\Component\Security\Core\User\UserProviderInterface'); - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $userProvider = $this->getMock(UserProviderInterface::class); + $ldap = $this->getMock(LdapInterface::class); $ldap ->expects($this->once()) ->method('bind') ->will($this->throwException(new ConnectionException())) ; - $userChecker = $this->getMock('Symfony\Component\Security\Core\User\UserCheckerInterface'); + $userChecker = $this->getMock(UserCheckerInterface::class); $provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap); $reflection = new \ReflectionMethod($provider, 'checkAuthentication'); @@ -62,15 +65,15 @@ class LdapBindAuthenticationProviderTest extends \PHPUnit_Framework_TestCase public function testRetrieveUser() { - $userProvider = $this->getMock('Symfony\Component\Security\Core\User\UserProviderInterface'); + $userProvider = $this->getMock(UserProviderInterface::class); $userProvider ->expects($this->once()) ->method('loadUserByUsername') ->with('foo') ; - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $ldap = $this->getMock(LdapInterface::class); - $userChecker = $this->getMock('Symfony\Component\Security\Core\User\UserCheckerInterface'); + $userChecker = $this->getMock(UserCheckerInterface::class); $provider = new LdapBindAuthenticationProvider($userProvider, $userChecker, 'key', $ldap); $reflection = new \ReflectionMethod($provider, 'retrieveUser'); diff --git a/Core/Tests/Authorization/DebugAccessDecisionManagerTest.php b/Core/Tests/Authorization/DebugAccessDecisionManagerTest.php new file mode 100644 index 0000000..f90f776 --- /dev/null +++ b/Core/Tests/Authorization/DebugAccessDecisionManagerTest.php @@ -0,0 +1,43 @@ +<?php + +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier <fabien@symfony.com> + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Authorization; + +use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; + +class DebugAccessDecisionManagerTest extends \PHPUnit_Framework_TestCase +{ + /** + * @dataProvider provideObjectsAndLogs + */ + public function testDecideLog($expectedLog, $object) + { + $adm = new DebugAccessDecisionManager(new AccessDecisionManager()); + $adm->decide($this->getMock(TokenInterface::class), array('ATTRIBUTE_1'), $object); + + $this->assertSame($expectedLog, $adm->getDecisionLog()); + } + + public function provideObjectsAndLogs() + { + $object = new \stdClass(); + + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'NULL', 'result' => false)), null); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'boolean (true)', 'result' => false)), true); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'string (jolie string)', 'result' => false)), 'jolie string'); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'integer (12345)', 'result' => false)), 12345); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'resource', 'result' => false)), fopen(__FILE__, 'r')); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => 'array', 'result' => false)), array()); + yield array(array(array('attributes' => array('ATTRIBUTE_1'), 'object' => sprintf('stdClass (object hash: %s)', spl_object_hash($object)), 'result' => false)), $object); + } +} diff --git a/Core/Tests/User/LdapUserProviderTest.php b/Core/Tests/User/LdapUserProviderTest.php index 9b126e9..b942e76 100644 --- a/Core/Tests/User/LdapUserProviderTest.php +++ b/Core/Tests/User/LdapUserProviderTest.php @@ -11,6 +11,10 @@ namespace Symfony\Component\Security\Core\Tests\User; +use Symfony\Component\Ldap\Adapter\CollectionInterface; +use Symfony\Component\Ldap\Adapter\QueryInterface; +use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\LdapInterface; use Symfony\Component\Security\Core\User\LdapUserProvider; use Symfony\Component\Ldap\Exception\ConnectionException; @@ -24,7 +28,7 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase */ public function testLoadUserByUsernameFailsIfCantConnectToLdap() { - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $ldap = $this->getMock(LdapInterface::class); $ldap ->expects($this->once()) ->method('bind') @@ -40,12 +44,29 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase */ public function testLoadUserByUsernameFailsIfNoLdapEntries() { - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(0)) + ; + $ldap = $this->getMock(LdapInterface::class); $ldap ->expects($this->once()) ->method('escape') ->will($this->returnValue('foo')) ; + $ldap + ->expects($this->once()) + ->method('query') + ->will($this->returnValue($query)) + ; $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com'); $provider->loadUserByUsername('foo'); @@ -56,7 +77,19 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase */ public function testLoadUserByUsernameFailsIfMoreThanOneLdapEntry() { - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(2)) + ; + $ldap = $this->getMock(LdapInterface::class); $ldap ->expects($this->once()) ->method('escape') @@ -64,21 +97,42 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase ; $ldap ->expects($this->once()) - ->method('find') - ->will($this->returnValue(array( - array(), - array(), - 'count' => 2, - ))) + ->method('query') + ->will($this->returnValue($query)) ; $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com'); $provider->loadUserByUsername('foo'); } - public function testSuccessfulLoadUserByUsername() + /** + * @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException + */ + public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry() { - $ldap = $this->getMock('Symfony\Component\Ldap\LdapClientInterface'); + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $ldap = $this->getMock(LdapInterface::class); + $result + ->expects($this->once()) + ->method('offsetGet') + ->with(0) + ->will($this->returnValue(new Entry('foo', array( + 'sAMAccountName' => array('foo'), + 'userpassword' => array('bar', 'baz'), + ) + ))) + ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(1)) + ; $ldap ->expects($this->once()) ->method('escape') @@ -86,15 +140,96 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase ; $ldap ->expects($this->once()) - ->method('find') - ->will($this->returnValue(array( - array( - 'sAMAccountName' => 'foo', - 'userpassword' => 'bar', - ), - 'count' => 1, + ->method('query') + ->will($this->returnValue($query)) + ; + + $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, array(), 'sAMAccountName', '({uid_key}={username})', 'userpassword'); + $this->assertInstanceOf( + 'Symfony\Component\Security\Core\User\User', + $provider->loadUserByUsername('foo') + ); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException + */ + public function testLoadUserByUsernameFailsIfEntryHasNoPasswordAttribute() + { + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $ldap = $this->getMock(LdapInterface::class); + $result + ->expects($this->once()) + ->method('offsetGet') + ->with(0) + ->will($this->returnValue(new Entry('foo', array( + 'sAMAccountName' => array('foo'), + ) ))) ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(1)) + ; + $ldap + ->expects($this->once()) + ->method('escape') + ->will($this->returnValue('foo')) + ; + $ldap + ->expects($this->once()) + ->method('query') + ->will($this->returnValue($query)) + ; + + $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, array(), 'sAMAccountName', '({uid_key}={username})', 'userpassword'); + $this->assertInstanceOf( + 'Symfony\Component\Security\Core\User\User', + $provider->loadUserByUsername('foo') + ); + } + + public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute() + { + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $ldap = $this->getMock(LdapInterface::class); + $result + ->expects($this->once()) + ->method('offsetGet') + ->with(0) + ->will($this->returnValue(new Entry('foo', array( + 'sAMAccountName' => array('foo'), + ) + ))) + ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(1)) + ; + $ldap + ->expects($this->once()) + ->method('escape') + ->will($this->returnValue('foo')) + ; + $ldap + ->expects($this->once()) + ->method('query') + ->will($this->returnValue($query)) + ; $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com'); $this->assertInstanceOf( @@ -102,4 +237,47 @@ class LdapUserProviderTest extends \PHPUnit_Framework_TestCase $provider->loadUserByUsername('foo') ); } + + public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute() + { + $result = $this->getMock(CollectionInterface::class); + $query = $this->getMock(QueryInterface::class); + $query + ->expects($this->once()) + ->method('execute') + ->will($this->returnValue($result)) + ; + $ldap = $this->getMock(LdapInterface::class); + $result + ->expects($this->once()) + ->method('offsetGet') + ->with(0) + ->will($this->returnValue(new Entry('foo', array( + 'sAMAccountName' => array('foo'), + 'userpassword' => array('bar'), + ) + ))) + ; + $result + ->expects($this->once()) + ->method('count') + ->will($this->returnValue(1)) + ; + $ldap + ->expects($this->once()) + ->method('escape') + ->will($this->returnValue('foo')) + ; + $ldap + ->expects($this->once()) + ->method('query') + ->will($this->returnValue($query)) + ; + + $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, array(), 'sAMAccountName', '({uid_key}={username})', 'userpassword'); + $this->assertInstanceOf( + 'Symfony\Component\Security\Core\User\User', + $provider->loadUserByUsername('foo') + ); + } } diff --git a/Core/User/LdapUserProvider.php b/Core/User/LdapUserProvider.php index 1593564..fc42419 100644 --- a/Core/User/LdapUserProvider.php +++ b/Core/User/LdapUserProvider.php @@ -11,10 +11,12 @@ namespace Symfony\Component\Security\Core\User; +use Symfony\Component\Ldap\Entry; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Ldap\Exception\ConnectionException; -use Symfony\Component\Ldap\LdapClientInterface; +use Symfony\Component\Ldap\LdapInterface; /** * LdapUserProvider is a simple user provider on top of ldap. @@ -30,17 +32,19 @@ class LdapUserProvider implements UserProviderInterface private $searchPassword; private $defaultRoles; private $defaultSearch; + private $passwordAttribute; /** - * @param LdapClientInterface $ldap - * @param string $baseDn - * @param string $searchDn - * @param string $searchPassword - * @param array $defaultRoles - * @param string $uidKey - * @param string $filter + * @param LdapInterface $ldap + * @param string $baseDn + * @param string $searchDn + * @param string $searchPassword + * @param array $defaultRoles + * @param string $uidKey + * @param string $filter + * @param string $passwordAttribute */ - public function __construct(LdapClientInterface $ldap, $baseDn, $searchDn = null, $searchPassword = null, array $defaultRoles = array(), $uidKey = 'sAMAccountName', $filter = '({uid_key}={username})') + public function __construct(LdapInterface $ldap, $baseDn, $searchDn = null, $searchPassword = null, array $defaultRoles = array(), $uidKey = 'sAMAccountName', $filter = '({uid_key}={username})', $passwordAttribute = null) { $this->ldap = $ldap; $this->baseDn = $baseDn; @@ -48,6 +52,7 @@ class LdapUserProvider implements UserProviderInterface $this->searchPassword = $searchPassword; $this->defaultRoles = $defaultRoles; $this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter); + $this->passwordAttribute = $passwordAttribute; } /** @@ -57,33 +62,25 @@ class LdapUserProvider implements UserProviderInterface { try { $this->ldap->bind($this->searchDn, $this->searchPassword); - $username = $this->ldap->escape($username, '', LDAP_ESCAPE_FILTER); + $username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_FILTER); $query = str_replace('{username}', $username, $this->defaultSearch); - $search = $this->ldap->find($this->baseDn, $query); + $search = $this->ldap->query($this->baseDn, $query); } catch (ConnectionException $e) { throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username), 0, $e); } - if (!$search) { + $entries = $search->execute(); + $count = count($entries); + + if (!$count) { throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username)); } - if ($search['count'] > 1) { + if ($count > 1) { throw new UsernameNotFoundException('More than one user found'); } - $user = $search[0]; - - return $this->loadUser($username, $user); - } - - public function loadUser($username, $user) - { - $password = isset($user['userpassword']) ? $user['userpassword'] : null; - - $roles = $this->defaultRoles; - - return new User($username, $password, $roles); + return $this->loadUser($username, $entries[0]); } /** @@ -105,4 +102,43 @@ class LdapUserProvider implements UserProviderInterface { return $class === 'Symfony\Component\Security\Core\User\User'; } + + /** + * Loads a user from an LDAP entry. + * + * @param string $username + * @param Entry $entry + * + * @return User + */ + protected function loadUser($username, Entry $entry) + { + $password = $this->getPassword($entry); + + return new User($username, $password, $this->defaultRoles); + } + + /** + * Fetches the password from an LDAP entry. + * + * @param null|Entry $entry + */ + private function getPassword(Entry $entry) + { + if (null === $this->passwordAttribute) { + return; + } + + if (!$entry->hasAttribute($this->passwordAttribute)) { + throw new InvalidArgumentException(sprintf('Missing attribute "%s" for user "%s".', $this->passwordAttribute, $entry->getDn())); + } + + $values = $entry->getAttribute($this->passwordAttribute); + + if (1 !== count($values)) { + throw new InvalidArgumentException(sprintf('Attribute "%s" has multiple values.', $this->passwordAttribute)); + } + + return $values[0]; + } } diff --git a/Core/composer.json b/Core/composer.json index 8e7931e..e2915b0 100644 --- a/Core/composer.json +++ b/Core/composer.json @@ -24,7 +24,7 @@ "symfony/event-dispatcher": "~2.8|~3.0", "symfony/expression-language": "~2.8|~3.0", "symfony/http-foundation": "~2.8|~3.0", - "symfony/ldap": "~2.8|~3.0", + "symfony/ldap": "~3.1", "symfony/validator": "~2.8|~3.0", "psr/log": "~1.0" }, @@ -44,7 +44,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "3.0-dev" + "dev-master": "3.1-dev" } } } diff --git a/Csrf/composer.json b/Csrf/composer.json index 341a860..d111fa1 100644 --- a/Csrf/composer.json +++ b/Csrf/composer.json @@ -36,7 +36,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "3.0-dev" + "dev-master": "3.1-dev" } } } diff --git a/Guard/Authenticator/AbstractFormLoginAuthenticator.php b/Guard/Authenticator/AbstractFormLoginAuthenticator.php index 6d6d14e..f99900b 100644 --- a/Guard/Authenticator/AbstractFormLoginAuthenticator.php +++ b/Guard/Authenticator/AbstractFormLoginAuthenticator.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Security; +use Symfony\Component\Security\Http\Util\TargetPathTrait; /** * A base class to make form login authentication easier! @@ -26,6 +27,8 @@ use Symfony\Component\Security\Core\Security; */ abstract class AbstractFormLoginAuthenticator extends AbstractGuardAuthenticator { + use TargetPathTrait; + /** * Return the URL to the login page. * @@ -34,16 +37,6 @@ abstract class AbstractFormLoginAuthenticator extends AbstractGuardAuthenticator abstract protected function getLoginUrl(); /** - * The user will be redirected to the secure page they originally tried - * to access. But if no such page exists (i.e. the user went to the - * login page directly), this returns the URL the user should be redirected - * to after logging in successfully (e.g. your homepage). - * - * @return string - */ - abstract protected function getDefaultSuccessRedirectUrl(); - - /** * Override to change what happens after a bad username/password is submitted. * * @param Request $request @@ -73,12 +66,18 @@ abstract class AbstractFormLoginAuthenticator extends AbstractGuardAuthenticator */ public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey) { + @trigger_error(sprintf('The AbstractFormLoginAuthenticator::onAuthenticationSuccess() implementation was deprecated in Symfony 3.1 and will be removed in Symfony 4.0. You should implement this method yourself in %s and remove getDefaultSuccessRedirectUrl().', get_class($this)), E_USER_DEPRECATED); + + if (!method_exists($this, 'getDefaultSuccessRedirectUrl')) { + throw new \Exception(sprintf('You must implement onAuthenticationSuccess() or getDefaultSuccessRedirectUrl() in %s.', get_class($this))); + } + $targetPath = null; // if the user hit a secure page and start() was called, this was // the URL they were on, and probably where you want to redirect to if ($request->getSession() instanceof SessionInterface) { - $targetPath = $request->getSession()->get('_security.'.$providerKey.'.target_path'); + $targetPath = $this->getTargetPath($request->getSession(), $providerKey); } if (!$targetPath) { diff --git a/Guard/Firewall/GuardAuthenticationListener.php b/Guard/Firewall/GuardAuthenticationListener.php index ed0a36e..59d5d29 100644 --- a/Guard/Firewall/GuardAuthenticationListener.php +++ b/Guard/Firewall/GuardAuthenticationListener.php @@ -78,7 +78,7 @@ class GuardAuthenticationListener implements ListenerInterface if ($event->hasResponse()) { if (null !== $this->logger) { - $this->logger->debug(sprintf('The "%s" authenticator set the response. Any later authenticator will not be called', get_class($guardAuthenticator))); + $this->logger->debug('The "{authenticator}" authenticator set the response. Any later authenticator will not be called', array('authenticator' => get_class($guardAuthenticator))); } break; diff --git a/Guard/Tests/Authenticator/FormLoginAuthenticatorTest.php b/Guard/Tests/Authenticator/FormLoginAuthenticatorTest.php index 3dbbf84..e35564b 100644 --- a/Guard/Tests/Authenticator/FormLoginAuthenticatorTest.php +++ b/Guard/Tests/Authenticator/FormLoginAuthenticatorTest.php @@ -50,6 +50,9 @@ class FormLoginAuthenticatorTest extends \PHPUnit_Framework_TestCase $this->assertEquals(self::LOGIN_URL, $failureResponse->getTargetUrl()); } + /** + * @group legacy + */ public function testAuthenticationSuccessWithoutSession() { $token = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface') @@ -62,6 +65,9 @@ class FormLoginAuthenticatorTest extends \PHPUnit_Framework_TestCase $this->assertEquals(self::DEFAULT_SUCCESS_URL, $redirectResponse->getTargetUrl()); } + /** + * @group legacy + */ public function testAuthenticationSuccessWithSessionButEmpty() { $token = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface') @@ -78,6 +84,9 @@ class FormLoginAuthenticatorTest extends \PHPUnit_Framework_TestCase $this->assertEquals(self::DEFAULT_SUCCESS_URL, $redirectResponse->getTargetUrl()); } + /** + * @group legacy + */ public function testAuthenticationSuccessWithSessionAndTarget() { $token = $this->getMockBuilder('Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface') diff --git a/Guard/composer.json b/Guard/composer.json index 99dff9c..7adb774 100644 --- a/Guard/composer.json +++ b/Guard/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=5.5.9", "symfony/security-core": "~2.8|~3.0", - "symfony/security-http": "~2.8|~3.0" + "symfony/security-http": "~3.1" }, "require-dev": { "psr/log": "~1.0" @@ -32,7 +32,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "3.0-dev" + "dev-master": "3.1-dev" } } } diff --git a/Http/Authentication/DefaultAuthenticationSuccessHandler.php b/Http/Authentication/DefaultAuthenticationSuccessHandler.php index bfc0c8b..4cb4bb6 100644 --- a/Http/Authentication/DefaultAuthenticationSuccessHandler.php +++ b/Http/Authentication/DefaultAuthenticationSuccessHandler.php @@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Http\Authentication; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Http\Util\TargetPathTrait; use Symfony\Component\Security\Http\HttpUtils; use Symfony\Component\Security\Http\ParameterBagUtils; @@ -25,6 +26,8 @@ use Symfony\Component\Security\Http\ParameterBagUtils; */ class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandlerInterface { + use TargetPathTrait; + protected $httpUtils; protected $options; protected $providerKey; @@ -113,8 +116,8 @@ class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandle return $targetUrl; } - if (null !== $this->providerKey && $targetUrl = $request->getSession()->get('_security.'.$this->providerKey.'.target_path')) { - $request->getSession()->remove('_security.'.$this->providerKey.'.target_path'); + if (null !== $this->providerKey && $targetUrl = $this->getTargetPath($request->getSession(), $this->providerKey)) { + $this->removeTargetPath($request->getSession(), $this->providerKey); return $targetUrl; } diff --git a/Http/Firewall/ContextListener.php b/Http/Firewall/ContextListener.php index 9ac37cd..4d6f3f8 100644 --- a/Http/Firewall/ContextListener.php +++ b/Http/Firewall/ContextListener.php @@ -15,7 +15,10 @@ use Psr\Log\LoggerInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; +use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; +use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; @@ -39,8 +42,9 @@ class ContextListener implements ListenerInterface private $userProviders; private $dispatcher; private $registered; + private $trustResolver; - public function __construct(TokenStorageInterface $tokenStorage, array $userProviders, $contextKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null) + public function __construct(TokenStorageInterface $tokenStorage, array $userProviders, $contextKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, AuthenticationTrustResolverInterface $trustResolver = null) { if (empty($contextKey)) { throw new \InvalidArgumentException('$contextKey must not be empty.'); @@ -58,6 +62,7 @@ class ContextListener implements ListenerInterface $this->sessionKey = '_security_'.$contextKey; $this->logger = $logger; $this->dispatcher = $dispatcher; + $this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver(AnonymousToken::class, RememberMeToken::class); } /** @@ -121,7 +126,7 @@ class ContextListener implements ListenerInterface $request = $event->getRequest(); $session = $request->getSession(); - if ((null === $token = $this->tokenStorage->getToken()) || ($token instanceof AnonymousToken)) { + if ((null === $token = $this->tokenStorage->getToken()) || $this->trustResolver->isAnonymous($token)) { if ($request->hasPreviousSession()) { $session->remove($this->sessionKey); } diff --git a/Http/Firewall/ExceptionListener.php b/Http/Firewall/ExceptionListener.php index a1cae2a..98f5ac0 100644 --- a/Http/Firewall/ExceptionListener.php +++ b/Http/Firewall/ExceptionListener.php @@ -22,6 +22,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\InsufficientAuthenticationException; use Symfony\Component\Security\Core\Exception\LogoutException; +use Symfony\Component\Security\Http\Util\TargetPathTrait; use Symfony\Component\Security\Http\HttpUtils; use Symfony\Component\HttpFoundation\Request; use Psr\Log\LoggerInterface; @@ -39,6 +40,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; */ class ExceptionListener { + use TargetPathTrait; + private $tokenStorage; private $providerKey; private $accessDeniedHandler; @@ -200,7 +203,15 @@ class ExceptionListener } } - return $this->authenticationEntryPoint->start($request, $authException); + $response = $this->authenticationEntryPoint->start($request, $authException); + + if (!$response instanceof Response) { + $given = is_object($response) ? get_class($response) : gettype($response); + + throw new \LogicException(sprintf('The %s::start() method must return a Response object (%s returned)', get_class($this->authenticationEntryPoint), $given)); + } + + return $response; } /** @@ -210,7 +221,7 @@ class ExceptionListener { // session isn't required when using HTTP basic authentication mechanism for example if ($request->hasSession() && $request->isMethodSafe() && !$request->isXmlHttpRequest()) { - $request->getSession()->set('_security.'.$this->providerKey.'.target_path', $request->getUri()); + $this->saveTargetPath($request->getSession(), $this->providerKey, $request->getUri()); } } } diff --git a/Http/SecurityEvents.php b/Http/SecurityEvents.php index 46c8257..550acb4 100644 --- a/Http/SecurityEvents.php +++ b/Http/SecurityEvents.php @@ -17,10 +17,7 @@ final class SecurityEvents * The INTERACTIVE_LOGIN event occurs after a user is logged in * interactively for authentication based on http, cookies or X509. * - * The event listener method receives a - * Symfony\Component\Security\Http\Event\InteractiveLoginEvent instance. - * - * @Event + * @Event("Symfony\Component\Security\Http\Event\InteractiveLoginEvent") * * @var string */ @@ -30,10 +27,7 @@ final class SecurityEvents * The SWITCH_USER event occurs before switch to another user and * before exit from an already switched user. * - * The event listener method receives a - * Symfony\Component\Security\Http\Event\SwitchUserEvent instance. - * - * @Event + * @Event("Symfony\Component\Security\Http\Event\SwitchUserEvent") * * @var string */ diff --git a/Http/Tests/Firewall/ContextListenerTest.php b/Http/Tests/Firewall/ContextListenerTest.php index ae1199a..0213330 100644 --- a/Http/Tests/Firewall/ContextListenerTest.php +++ b/Http/Tests/Firewall/ContextListenerTest.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Http\Firewall\ContextListener; @@ -85,6 +86,13 @@ class ContextListenerTest extends \PHPUnit_Framework_TestCase $this->assertFalse($session->has('_security_session')); } + public function testOnKernelResponseWillRemoveSessionOnAnonymousToken() + { + $session = $this->runSessionOnKernelResponse(new AnonymousToken('secret', 'anon.'), 'C:10:"serialized"'); + + $this->assertFalse($session->has('_security_session')); + } + public function testOnKernelResponseWithoutSession() { $tokenStorage = new TokenStorage(); diff --git a/Http/Tests/Firewall/ExceptionListenerTest.php b/Http/Tests/Firewall/ExceptionListenerTest.php index 3d409e5..db0a242 100644 --- a/Http/Tests/Firewall/ExceptionListenerTest.php +++ b/Http/Tests/Firewall/ExceptionListenerTest.php @@ -65,6 +65,20 @@ class ExceptionListenerTest extends \PHPUnit_Framework_TestCase ); } + public function testExceptionWhenEntryPointReturnsBadValue() + { + $event = $this->createEvent(new AuthenticationException()); + + $entryPoint = $this->getMock('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface'); + $entryPoint->expects($this->once())->method('start')->will($this->returnValue('NOT A RESPONSE')); + + $listener = $this->createExceptionListener(null, null, null, $entryPoint); + $listener->onKernelException($event); + // the exception has been replaced by our LogicException + $this->assertInstanceOf('LogicException', $event->getException()); + $this->assertStringEndsWith('start() method must return a Response object (string returned)', $event->getException()->getMessage()); + } + /** * @dataProvider getAccessDeniedExceptionProvider */ diff --git a/Http/Tests/Util/TargetPathTraitTest.php b/Http/Tests/Util/TargetPathTraitTest.php new file mode 100644 index 0000000..b2c4dc7 --- /dev/null +++ b/Http/Tests/Util/TargetPathTraitTest.php @@ -0,0 +1,76 @@ +<?php + +namespace Symfony\Component\Security\Http\Tests\Util; + +use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\Security\Http\Util\TargetPathTrait; + +class TargetPathTraitTest extends \PHPUnit_Framework_TestCase +{ + public function testSetTargetPath() + { + $obj = new TestClassWithTargetPathTrait(); + + $session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface') + ->getMock(); + + $session->expects($this->once()) + ->method('set') + ->with('_security.firewall_name.target_path', '/foo'); + + $obj->doSetTargetPath($session, 'firewall_name', '/foo'); + } + + public function testGetTargetPath() + { + $obj = new TestClassWithTargetPathTrait(); + + $session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface') + ->getMock(); + + $session->expects($this->once()) + ->method('get') + ->with('_security.cool_firewall.target_path') + ->willReturn('/bar'); + + $actualUri = $obj->doGetTargetPath($session, 'cool_firewall'); + $this->assertEquals( + '/bar', + $actualUri + ); + } + + public function testRemoveTargetPath() + { + $obj = new TestClassWithTargetPathTrait(); + + $session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface') + ->getMock(); + + $session->expects($this->once()) + ->method('remove') + ->with('_security.best_firewall.target_path'); + + $obj->doRemoveTargetPath($session, 'best_firewall'); + } +} + +class TestClassWithTargetPathTrait +{ + use TargetPathTrait; + + public function doSetTargetPath(SessionInterface $session, $providerKey, $uri) + { + $this->saveTargetPath($session, $providerKey, $uri); + } + + public function doGetTargetPath(SessionInterface $session, $providerKey) + { + return $this->getTargetPath($session, $providerKey); + } + + public function doRemoveTargetPath(SessionInterface $session, $providerKey) + { + $this->removeTargetPath($session, $providerKey); + } +} diff --git a/Http/Util/TargetPathTrait.php b/Http/Util/TargetPathTrait.php new file mode 100644 index 0000000..986adb0 --- /dev/null +++ b/Http/Util/TargetPathTrait.php @@ -0,0 +1,58 @@ +<?php + +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier <fabien@symfony.com> + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Util; + +use Symfony\Component\HttpFoundation\Session\SessionInterface; + +/** + * Trait to get (and set) the URL the user last visited before being forced to authenticate. + */ +trait TargetPathTrait +{ + /** + * Sets the target path the user should be redirected to after authentication. + * + * Usually, you do not need to set this directly. + * + * @param SessionInterface $session + * @param string $providerKey The name of your firewall + * @param string $uri The URI to set as the target path + */ + private function saveTargetPath(SessionInterface $session, $providerKey, $uri) + { + $session->set('_security.'.$providerKey.'.target_path', $uri); + } + + /** + * Returns the URL (if any) the user visited that forced them to login. + * + * @param SessionInterface $session + * @param string $providerKey The name of your firewall + * + * @return string + */ + private function getTargetPath(SessionInterface $session, $providerKey) + { + return $session->get('_security.'.$providerKey.'.target_path'); + } + + /** + * Removes the target path from the session. + * + * @param SessionInterface $session + * @param string $providerKey The name of your firewall + */ + private function removeTargetPath(SessionInterface $session, $providerKey) + { + $session->remove('_security.'.$providerKey.'.target_path'); + } +} diff --git a/Http/composer.json b/Http/composer.json index a3e008a..f19d0e4 100644 --- a/Http/composer.json +++ b/Http/composer.json @@ -43,7 +43,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "3.0-dev" + "dev-master": "3.1-dev" } } } diff --git a/composer.json b/composer.json index e4aff16..7b3801f 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "symfony/routing": "~2.8|~3.0", "symfony/validator": "~2.8|~3.0", "symfony/expression-language": "~2.8|~3.0", - "symfony/ldap": "~2.8|~3.0", + "symfony/ldap": "~3.1", "psr/log": "~1.0" }, "suggest": { @@ -56,7 +56,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "3.0-dev" + "dev-master": "3.1-dev" } } } |