diff --git a/composer.json b/composer.json index 434e1b452..0e03236f2 100644 --- a/composer.json +++ b/composer.json @@ -96,6 +96,7 @@ "php": "^8.1", "symfony/deprecation-contracts": "^3.0", "symfony/framework-bundle": "^5.4 || ^6.3 || ^7.0", + "symfony/http-foundation": "^5.4 || ^6.3 || ^7.0", "symfony/security-bundle": "^5.4 || ^6.3 || ^7.0", "symfony/options-resolver": "^5.4 || ^6.3 || ^7.0", "symfony/form": "^5.4 || ^6.3 || ^7.0", diff --git a/src/Security/OAuthUtils.php b/src/Security/OAuthUtils.php index 28e6aeac3..099ccb718 100644 --- a/src/Security/OAuthUtils.php +++ b/src/Security/OAuthUtils.php @@ -77,7 +77,7 @@ public function getAuthorizationUrl(Request $request, string $name, ?string $red } if ($request->query->has('state')) { - $this->addQueryParameterToState($request->query->get('state'), $resourceOwner); + $this->addQueryParameterToState($request->query->all()['state'] ?? [], $resourceOwner); } return $resourceOwner->getAuthorizationUrl($redirectUrl, $extraParameters); @@ -93,7 +93,7 @@ public function getServiceAuthUrl(Request $request, ResourceOwnerInterface $reso $request->attributes->set('service', $resourceOwner->getName()); if ($request->query->has('state')) { - $this->addQueryParameterToState($request->query->get('state'), $resourceOwner); + $this->addQueryParameterToState($request->query->all()['state'] ?? [], $resourceOwner); } return $this->httpUtils->generateUri($request, 'hwi_oauth_connect_service'); diff --git a/tests/Security/OAuthUtilsTest.php b/tests/Security/OAuthUtilsTest.php index 0c63894fe..d8950e615 100644 --- a/tests/Security/OAuthUtilsTest.php +++ b/tests/Security/OAuthUtilsTest.php @@ -67,15 +67,12 @@ public function testGetAuthorizationUrlWithConnectAndUserToken(): void ); } - public function testGetAuthorizationUrlWithStateQueryParameters(): void + /** + * @dataProvider provideAuthorizationUrlsWithState + */ + public function testGetAuthorizationUrlWithStateQueryParameters(string $url, string $urlWithState, string $redirect): void { - $parameters = ['foo' => 'bar', 'foobar' => 'foobaz']; - $state = new State($parameters); - - $url = 'http://localhost:8080/login/check-instagram'; - $redirect = 'https://api.instagram.com/oauth/authorize?redirect='.rawurlencode($url); - - $request = $this->getRequest($url.'?state='.$state->encode()); + $request = $this->getRequest($urlWithState); $resource = $this->getMockBuilder(ResourceOwnerInterface::class)->getMock(); $utils = new OAuthUtils($this->getHttpUtils($url), $this->getAuthorizationChecker(false, $this->grantRule), $this->createFirewallMapMock(), true, $this->grantRule); @@ -131,14 +128,12 @@ public function testGetAuthorizationUrlWithAuthenticatedFullyRule(): void $this->assertNull($request->attributes->get('service')); } - public function testGetServiceAuthUrlWithStateQueryParameters(): void + /** + * @dataProvider provideServiceAuthUrlsWithState + */ + public function testGetServiceAuthUrlWithStateQueryParameters(string $url, string $expectedResult): void { - $parameters = ['foo' => 'bar', 'foobar' => 'foobaz']; - $state = new State($parameters); - - $url = 'http://localhost:8080/service/instagram'; - - $request = $this->getRequest($url.'?state='.$state->encode()); + $request = $this->getRequest($url); $resource = $this->getMockBuilder(ResourceOwnerInterface::class)->getMock(); $resource ->expects($this->any()) @@ -166,7 +161,7 @@ public function testGetServiceAuthUrlWithStateQueryParameters(): void ->withConsecutive(['foo', 'bar'], ['foobar', 'foobaz']); $this->assertEquals( - $url, + $expectedResult, $utils->getServiceAuthUrl($request, $resource) ); } @@ -261,6 +256,41 @@ public function provideInvalidData(): iterable yield 'missing "oauth_signature_method"' => [['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_version' => '']]; } + public function provideServiceAuthUrlsWithState(): iterable + { + $parameters = ['foo' => 'bar', 'foobar' => 'foobaz']; + $state = new State($parameters); + + $url = 'http://localhost:8080/service/instagram'; + + yield 'state as an encoded string' => [$url.'?state='.$state->encode(), $url]; + + $stateAsArray = []; + foreach ($parameters as $key => $value) { + $stateAsArray[] = sprintf('state[%s]=%s', $key, rawurlencode($value)); + } + + yield 'state as an array' => [$url.'?'.implode('&', $stateAsArray), $url]; + } + + public function provideAuthorizationUrlsWithState(): iterable + { + $parameters = ['foo' => 'bar', 'foobar' => 'foobaz']; + $state = new State($parameters); + + $url = 'http://localhost:8080/login/check-instagram'; + $redirect = 'https://api.instagram.com/oauth/authorize?redirect='.rawurlencode($url); + + yield 'state as an encoded string' => [$url, $url.'?state='.$state->encode(), $redirect]; + + $stateAsArray = []; + foreach ($parameters as $key => $value) { + $stateAsArray[] = sprintf('state[%s]=%s', $key, rawurlencode($value)); + } + + yield 'state as an array' => [$url, $url.'?'.implode('&', $stateAsArray), $redirect]; + } + private function getRequest(string $url): Request { return Request::create($url, 'get', [], [], [], ['SERVER_PORT' => 8080]);