From 4e2e4fc0232e3d4a55fc9f885e32671fa8881265 Mon Sep 17 00:00:00 2001 From: John Noel Date: Thu, 28 Jul 2016 10:11:40 +0100 Subject: [PATCH 1/2] getRandomState only returns alphanumeric states Before, this was using the default character set for RandomLib/Generator's generateString which is the base 64 character set that includes + and /. While / wasn't causing any problems, using + in a URL parameters (e.g. when the OAuth 2 server sends back the state), the + was getting interpretted as a space, which means when a straight string comparison to stored state was being done, it was returning false. This changes getRandomState to use the Generator::CHAR_ALNUM constant as its character set which solves this problem. --- src/Provider/AbstractProvider.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Provider/AbstractProvider.php b/src/Provider/AbstractProvider.php index 4c80dfe9..9cd27cd5 100644 --- a/src/Provider/AbstractProvider.php +++ b/src/Provider/AbstractProvider.php @@ -27,6 +27,7 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use RandomLib\Factory as RandomFactory; +use RandomLib\Generator as RandomGenerator; use UnexpectedValueException; /** @@ -302,7 +303,7 @@ protected function getRandomState($length = 32) ->getRandomFactory() ->getMediumStrengthGenerator(); - return $generator->generateString($length); + return $generator->generateString($length, RandomGenerator::CHAR_ALNUM); } /** @@ -358,7 +359,7 @@ protected function getAuthorizationParameters(array $options) $options['client_id'] = $this->clientId; $options['redirect_uri'] = $this->redirectUri; $options['state'] = $this->state; - + return $options; } From c916abbd7bd52492f76f8179f08a2fcab59bb19e Mon Sep 17 00:00:00 2001 From: John Noel Date: Thu, 28 Jul 2016 10:18:00 +0100 Subject: [PATCH 2/2] Update testRandomGeneratorCreatesRandomState test Changes the Generator mock to expect a character set integer (in this case 7 which is the Generator::CHAR_ALNUM constant). --- test/src/Provider/AbstractProviderTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/src/Provider/AbstractProviderTest.php b/test/src/Provider/AbstractProviderTest.php index a68cac50..56a8e593 100644 --- a/test/src/Provider/AbstractProviderTest.php +++ b/test/src/Provider/AbstractProviderTest.php @@ -64,7 +64,7 @@ public function testAuthorizationUrlStateParam() 'state' => 'XXX' ])); } - + /** * Tests https://github.com/thephpleague/oauth2-client/pull/485 */ @@ -75,7 +75,7 @@ public function testCustomAuthorizationUrlOptions() ]); $query = parse_url($url, PHP_URL_QUERY); $this->assertNotEmpty($query); - + parse_str($query, $params); $this->assertArrayHasKey('foo', $params); $this->assertSame('BAR', $params['foo']); @@ -307,7 +307,7 @@ public function testRandomGeneratorCreatesRandomState() $xstate = str_repeat('x', 32); $generator = m::mock(RandomGenerator::class); - $generator->shouldReceive('generateString')->with(32)->times(1)->andReturn($xstate); + $generator->shouldReceive('generateString')->with(32, 7)->times(1)->andReturn($xstate); $factory = m::mock(RandomFactory::class); $factory->shouldReceive('getMediumStrengthGenerator')->times(1)->andReturn($generator);