From 19791b2460bf7cdf17437d9ebab4d83b60270a3f Mon Sep 17 00:00:00 2001
From: RussellAult <RussellAult@users.noreply.github.com>
Date: Sun, 10 Nov 2019 15:49:57 -0700
Subject: Check getRedirectUri() for queries

Resolves Issue #17885

Check getRedirectUri() for queries, and add a '&' instead of a '?' to $redirectUri if it already has them; otherwise, $redirectUri might end up with two '?'.

Signed-off-by: RussellAult <russellault@users.noreply.github.com>
---
 core/Controller/ClientFlowLoginController.php           | 14 +++++++++++---
 tests/Core/Controller/ClientFlowLoginControllerTest.php | 15 ++++++++++++---
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php
index bffedf19224..ba594469a7f 100644
--- a/core/Controller/ClientFlowLoginController.php
+++ b/core/Controller/ClientFlowLoginController.php
@@ -6,6 +6,7 @@
  * @author Lukas Reschke <lukas@statuscode.ch>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
+ * @author Russell Ault <russell@auksnest.ca>
  *
  * @license GNU AGPL version 3 or any later version
  *
@@ -337,9 +338,16 @@ class ClientFlowLoginController extends Controller {
 			$accessToken->setTokenId($generatedToken->getId());
 			$this->accessTokenMapper->insert($accessToken);
 
-			$redirectUri = sprintf(
-				'%s?state=%s&code=%s',
-				$client->getRedirectUri(),
+			$redirectUri = $client->getRedirectUri();
+			
+			if (parse_url($redirectUri, PHP_URL_QUERY)) {
+				$redirectUri .= '&';
+			} else {
+				$redirectUri .= '?';
+			}
+
+			$redirectUri .= sprintf(
+				'state=%s&code=%s',
 				urlencode($this->session->get('oauth.state')),
 				urlencode($code)
 			);
diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php
index 50280e18371..1401eac67d8 100644
--- a/tests/Core/Controller/ClientFlowLoginControllerTest.php
+++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php
@@ -398,7 +398,16 @@ class ClientFlowLoginControllerTest extends TestCase {
 		$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken'));
 	}
 
-	public function testGeneratePasswordWithPasswordForOauthClient() {
+	/**
+	 * @param string $redirectUri
+	 * @param string $redirectUrl
+	 *
+	 * @testWith
+	 * ["https://example.com/redirect.php", "https://example.com/redirect.php?state=MyOauthState&code=MyAccessCode"]
+	 * ["https://example.com/redirect.php?hello=world", "https://example.com/redirect.php?hello=world&state=MyOauthState&code=MyAccessCode"]
+	 *
+	 */
+	public function testGeneratePasswordWithPasswordForOauthClient($redirectUri, $redirectUrl) {		
 		$this->session
 			->expects($this->at(0))
 			->method('get')
@@ -471,7 +480,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->willReturn($token);
 		$client = new Client();
 		$client->setName('My OAuth client');
-		$client->setRedirectUri('https://example.com/redirect.php');
+		$client->setRedirectUri($redirectUri);
 		$this->clientMapper
 			->expects($this->once())
 			->method('getByIdentifier')
@@ -481,7 +490,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 		$this->eventDispatcher->expects($this->once())
 			->method('dispatch');
 
-		$expected = new Http\RedirectResponse('https://example.com/redirect.php?state=MyOauthState&code=MyAccessCode');
+		$expected = new Http\RedirectResponse($redirectUrl);
 		$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken', 'MyClientIdentifier'));
 	}
 
-- 
cgit v1.2.3