From 64c2efd8cbabdf88f72352b399676ef914f5e191 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 22 Jun 2017 00:05:03 +0200 Subject: [PATCH 1/6] [Security] Fix authentication.failure event not dispatched on AccountStatusException --- .../AuthenticationProviderManager.php | 4 +- .../AuthenticationProviderManagerTest.php | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php b/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php index 16de8daaeda93..4c0a7459d070c 100644 --- a/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php +++ b/src/Symfony/Component/Security/Core/Authentication/AuthenticationProviderManager.php @@ -83,9 +83,9 @@ public function authenticate(TokenInterface $token) break; } } catch (AccountStatusException $e) { - $e->setToken($token); + $lastException = $e; - throw $e; + break; } catch (AuthenticationException $e) { $lastException = $e; } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php index 9b8105012c3d8..373369d455959 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php @@ -13,6 +13,9 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager; +use Symfony\Component\Security\Core\AuthenticationEvents; +use Symfony\Component\Security\Core\Event\AuthenticationEvent; +use Symfony\Component\Security\Core\Event\AuthenticationFailureEvent; use Symfony\Component\Security\Core\Exception\ProviderNotFoundException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AccountStatusException; @@ -124,6 +127,50 @@ public function testEraseCredentialFlag() $this->assertEquals('bar', $token->getCredentials()); } + public function testAuthenticateDispatchesAuthenticationFailureEvent() + { + $token = new UsernamePasswordToken('foo', 'bar', 'key'); + $provider = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface')->getMock(); + $provider->expects($this->once())->method('supports')->willReturn(true); + $provider->expects($this->once())->method('authenticate')->willThrowException($exception = new AuthenticationException()); + + $dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); + $dispatcher + ->expects($this->once()) + ->method('dispatch') + ->with(AuthenticationEvents::AUTHENTICATION_FAILURE, $this->equalTo(new AuthenticationFailureEvent($token, $exception))); + + $manager = new AuthenticationProviderManager(array($provider)); + $manager->setEventDispatcher($dispatcher); + + try { + $manager->authenticate($token); + $this->fail('->authenticate() should rethrow exceptions'); + } catch (AuthenticationException $e) { + $this->assertSame($token, $exception->getToken()); + } + } + + public function testAuthenticateDispatchesAuthenticationSuccessEvent() + { + $token = new UsernamePasswordToken('foo', 'bar', 'key'); + + $provider = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface')->getMock(); + $provider->expects($this->once())->method('supports')->willReturn(true); + $provider->expects($this->once())->method('authenticate')->willReturn($token); + + $dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); + $dispatcher + ->expects($this->once()) + ->method('dispatch') + ->with(AuthenticationEvents::AUTHENTICATION_SUCCESS, $this->equalTo(new AuthenticationEvent($token))); + + $manager = new AuthenticationProviderManager(array($provider)); + $manager->setEventDispatcher($dispatcher); + + $this->assertSame($token, $manager->authenticate($token)); + } + protected function getAuthenticationProvider($supports, $token = null, $exception = null) { $provider = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface')->getMock(); From 025dfff675a5b4b266bd5d500e6baa6b519ff8a8 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Sun, 9 Jul 2017 20:05:54 +0200 Subject: [PATCH 2/6] Use rawurlencode() to transform the Cookie into a string --- src/Symfony/Component/BrowserKit/Cookie.php | 2 +- .../Component/BrowserKit/Tests/CookieTest.php | 15 +++++++++++++++ src/Symfony/Component/HttpFoundation/Cookie.php | 2 +- .../Component/HttpFoundation/Tests/CookieTest.php | 3 +++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/BrowserKit/Cookie.php b/src/Symfony/Component/BrowserKit/Cookie.php index 42f184d532e02..c042c6a525295 100644 --- a/src/Symfony/Component/BrowserKit/Cookie.php +++ b/src/Symfony/Component/BrowserKit/Cookie.php @@ -62,7 +62,7 @@ public function __construct($name, $value, $expires = null, $path = null, $domai $this->rawValue = $value; } else { $this->value = $value; - $this->rawValue = urlencode($value); + $this->rawValue = rawurlencode($value); } $this->name = $name; $this->path = empty($path) ? '/' : $path; diff --git a/src/Symfony/Component/BrowserKit/Tests/CookieTest.php b/src/Symfony/Component/BrowserKit/Tests/CookieTest.php index 38ea81220bb2c..2f5a08d104143 100644 --- a/src/Symfony/Component/BrowserKit/Tests/CookieTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/CookieTest.php @@ -16,6 +16,21 @@ class CookieTest extends TestCase { + public function testToString() + { + $cookie = new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); + $this->assertEquals('foo=bar; expires=Fri, 20 May 2011 15:25:52 GMT; domain=.myfoodomain.com; path=/; secure; httponly', (string) $cookie, '->__toString() returns string representation of the cookie'); + + $cookie = new Cookie('foo', 'bar with white spaces', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); + $this->assertEquals('foo=bar%20with%20white%20spaces; expires=Fri, 20 May 2011 15:25:52 GMT; domain=.myfoodomain.com; path=/; secure; httponly', (string) $cookie, '->__toString() encodes the value of the cookie according to RFC 3986 (white space = %20)'); + + $cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com'); + $this->assertEquals('foo=; expires=Thu, 01 Jan 1970 00:00:01 GMT; domain=.myfoodomain.com; path=/admin/; httponly', (string) $cookie, '->__toString() returns string representation of a cleared cookie if value is NULL'); + + $cookie = new Cookie('foo', 'bar', 0, '/', ''); + $this->assertEquals('foo=bar; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/; httponly', (string) $cookie); + } + /** * @dataProvider getTestsForToFromString */ diff --git a/src/Symfony/Component/HttpFoundation/Cookie.php b/src/Symfony/Component/HttpFoundation/Cookie.php index 91783a6ad2b50..fb1e7dfd74ea4 100644 --- a/src/Symfony/Component/HttpFoundation/Cookie.php +++ b/src/Symfony/Component/HttpFoundation/Cookie.php @@ -82,7 +82,7 @@ public function __toString() if ('' === (string) $this->getValue()) { $str .= 'deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001); } else { - $str .= urlencode($this->getValue()); + $str .= rawurlencode($this->getValue()); if (0 !== $this->getExpiresTime()) { $str .= '; expires='.gmdate('D, d-M-Y H:i:s T', $this->getExpiresTime()); diff --git a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php index f3f74f635eb40..2d9fb09d3d4b6 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php @@ -160,6 +160,9 @@ public function testToString() $cookie = new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); $this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() returns string representation of the cookie'); + $cookie = new Cookie('foo', 'bar with white spaces', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true); + $this->assertEquals('foo=bar%20with%20white%20spaces; expires=Fri, 20-May-2011 15:25:52 GMT; path=/; domain=.myfoodomain.com; secure; httponly', (string) $cookie, '->__toString() encodes the value of the cookie according to RFC 3986 (white space = %20)'); + $cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com'); $this->assertEquals('foo=deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; path=/admin/; domain=.myfoodomain.com; httponly', (string) $cookie, '->__toString() returns string representation of a cleared cookie if value is NULL'); From d8f6ab1d6aadd716cf24087506b53ccd96e630ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20R=C3=B6=C3=9Fner?= Date: Wed, 12 Jul 2017 10:36:00 +0200 Subject: [PATCH 3/6] [Config] extracted the xml parsing from XmlUtils::loadFile into XmlUtils::load, so the parsing can be used with xml strings, as well. --- .../Config/Tests/Util/XmlUtilsTest.php | 17 ++++++- .../Component/Config/Util/XmlUtils.php | 45 +++++++++++++++---- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php index 29d64dba84205..3355a0359b56f 100644 --- a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php +++ b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php @@ -55,13 +55,28 @@ public function testLoadFile() XmlUtils::loadFile($fixtures.'valid.xml', array($mock, 'validate')); $this->fail(); } catch (\InvalidArgumentException $e) { - $this->assertContains('is not valid', $e->getMessage()); + $this->assertRegExp('/The XML file "[\w\/\\\.]+" is not valid\./', $e->getMessage()); } $this->assertInstanceOf('DOMDocument', XmlUtils::loadFile($fixtures.'valid.xml', array($mock, 'validate'))); $this->assertSame(array(), libxml_get_errors()); } + public function testLoad() + { + $fixtures = __DIR__.'/../Fixtures/Util/'; + + $mock = $this->getMockBuilder(__NAMESPACE__.'\Validator')->getMock(); + $mock->expects($this->once())->method('validate')->will($this->onConsecutiveCalls(false, true)); + + try { + XmlUtils::load(file_get_contents($fixtures.'valid.xml'), array($mock, 'validate')); + $this->fail(); + } catch (\InvalidArgumentException $e) { + $this->assertContains('The XML is not valid', $e->getMessage()); + } + } + public function testLoadFileWithInternalErrorsEnabled() { $internalErrors = libxml_use_internal_errors(true); diff --git a/src/Symfony/Component/Config/Util/XmlUtils.php b/src/Symfony/Component/Config/Util/XmlUtils.php index 25d9b0a0abe5d..34b7f5dcca462 100644 --- a/src/Symfony/Component/Config/Util/XmlUtils.php +++ b/src/Symfony/Component/Config/Util/XmlUtils.php @@ -18,9 +18,12 @@ * * @author Fabien Potencier * @author Martin Hasoň + * @author Ole Rößner */ class XmlUtils { + const XML_IS_NOT_VALID_MESSAGE = 'The XML is not valid.'; + /** * This class should not be instantiated. */ @@ -29,22 +32,17 @@ private function __construct() } /** - * Loads an XML file. + * Parses an XML string. * - * @param string $file An XML file path + * @param string $content An XML string * @param string|callable|null $schemaOrCallable An XSD schema file path, a callable, or null to disable validation * * @return \DOMDocument * * @throws \InvalidArgumentException When loading of XML file returns error */ - public static function loadFile($file, $schemaOrCallable = null) + public static function load($content, $schemaOrCallable = null) { - $content = @file_get_contents($file); - if ('' === trim($content)) { - throw new \InvalidArgumentException(sprintf('File %s does not contain valid XML, it is empty.', $file)); - } - $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); @@ -91,7 +89,7 @@ public static function loadFile($file, $schemaOrCallable = null) if (!$valid) { $messages = static::getXmlErrors($internalErrors); if (empty($messages)) { - $messages = array(sprintf('The XML file "%s" is not valid.', $file)); + $messages = array(self::XML_IS_NOT_VALID_MESSAGE); } throw new \InvalidArgumentException(implode("\n", $messages), 0, $e); } @@ -103,6 +101,35 @@ public static function loadFile($file, $schemaOrCallable = null) return $dom; } + /** + * Loads an XML file. + * + * @param string $file An XML file path + * @param string|callable|null $schemaOrCallable An XSD schema file path, a callable, or null to disable validation + * + * @return \DOMDocument + * + * @throws \InvalidArgumentException When loading of XML file returns error + */ + public static function loadFile($file, $schemaOrCallable = null) + { + $content = @file_get_contents($file); + if ('' === trim($content)) { + throw new \InvalidArgumentException(sprintf('File %s does not contain valid XML, it is empty.', $file)); + } + + try { + return static::load($content, $schemaOrCallable); + } + catch (\InvalidArgumentException $ex) { + throw new \InvalidArgumentException( + str_replace(self::XML_IS_NOT_VALID_MESSAGE, sprintf('The XML file "%s" is not valid.', $file), $ex->getMessage()), + $ex->getCode(), + $ex->getPrevious() + ); + } + } + /** * Converts a \DomElement object to a PHP array. * From 450348a206ff8752965bba9c3795eec1e3756811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20R=C3=B6=C3=9Fner?= Date: Wed, 12 Jul 2017 10:39:37 +0200 Subject: [PATCH 4/6] code style fixes. --- src/Symfony/Component/Config/Util/XmlUtils.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Symfony/Component/Config/Util/XmlUtils.php b/src/Symfony/Component/Config/Util/XmlUtils.php index 34b7f5dcca462..94f0ced752f7c 100644 --- a/src/Symfony/Component/Config/Util/XmlUtils.php +++ b/src/Symfony/Component/Config/Util/XmlUtils.php @@ -120,8 +120,7 @@ public static function loadFile($file, $schemaOrCallable = null) try { return static::load($content, $schemaOrCallable); - } - catch (\InvalidArgumentException $ex) { + } catch (\InvalidArgumentException $ex) { throw new \InvalidArgumentException( str_replace(self::XML_IS_NOT_VALID_MESSAGE, sprintf('The XML file "%s" is not valid.', $file), $ex->getMessage()), $ex->getCode(), From 43b2c2f5aadb79ee3d370bd43abc0c8b8719e2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20R=C3=B6=C3=9Fner?= Date: Wed, 12 Jul 2017 10:44:19 +0200 Subject: [PATCH 5/6] renamed load to parse, that's more specific. --- src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php | 4 ++-- src/Symfony/Component/Config/Util/XmlUtils.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php index 3355a0359b56f..15499928f7798 100644 --- a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php +++ b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php @@ -62,7 +62,7 @@ public function testLoadFile() $this->assertSame(array(), libxml_get_errors()); } - public function testLoad() + public function testParse() { $fixtures = __DIR__.'/../Fixtures/Util/'; @@ -70,7 +70,7 @@ public function testLoad() $mock->expects($this->once())->method('validate')->will($this->onConsecutiveCalls(false, true)); try { - XmlUtils::load(file_get_contents($fixtures.'valid.xml'), array($mock, 'validate')); + XmlUtils::parse(file_get_contents($fixtures.'valid.xml'), array($mock, 'validate')); $this->fail(); } catch (\InvalidArgumentException $e) { $this->assertContains('The XML is not valid', $e->getMessage()); diff --git a/src/Symfony/Component/Config/Util/XmlUtils.php b/src/Symfony/Component/Config/Util/XmlUtils.php index 94f0ced752f7c..fd0b004111770 100644 --- a/src/Symfony/Component/Config/Util/XmlUtils.php +++ b/src/Symfony/Component/Config/Util/XmlUtils.php @@ -41,7 +41,7 @@ private function __construct() * * @throws \InvalidArgumentException When loading of XML file returns error */ - public static function load($content, $schemaOrCallable = null) + public static function parse($content, $schemaOrCallable = null) { $internalErrors = libxml_use_internal_errors(true); $disableEntities = libxml_disable_entity_loader(true); @@ -119,7 +119,7 @@ public static function loadFile($file, $schemaOrCallable = null) } try { - return static::load($content, $schemaOrCallable); + return static::parse($content, $schemaOrCallable); } catch (\InvalidArgumentException $ex) { throw new \InvalidArgumentException( str_replace(self::XML_IS_NOT_VALID_MESSAGE, sprintf('The XML file "%s" is not valid.', $file), $ex->getMessage()), From 533dbb6d978f1035a7c23c9ea1feda29323cd839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20R=C3=B6=C3=9Fner?= Date: Wed, 12 Jul 2017 10:53:36 +0200 Subject: [PATCH 6/6] make the file test windows compatible --- src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php index 15499928f7798..395f44c0ce9f5 100644 --- a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php +++ b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php @@ -55,7 +55,7 @@ public function testLoadFile() XmlUtils::loadFile($fixtures.'valid.xml', array($mock, 'validate')); $this->fail(); } catch (\InvalidArgumentException $e) { - $this->assertRegExp('/The XML file "[\w\/\\\.]+" is not valid\./', $e->getMessage()); + $this->assertRegExp('/The XML file "[\w:\/\\\.]+" is not valid\./', $e->getMessage()); } $this->assertInstanceOf('DOMDocument', XmlUtils::loadFile($fixtures.'valid.xml', array($mock, 'validate')));