From 92582a350d295f123d08f16f09e06baa4bc4733c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 31 Oct 2018 23:06:08 +0100 Subject: [PATCH] Use the proper server for the apptoken flow login If a user can't authenticate normally (because they have 2FA that is not available on their devices for example). The redirect that is generated should be of the proper format. This means 1. Include the protocol 2. Include the possible subfolder Signed-off-by: Roeland Jago Douma --- core/Controller/ClientFlowLoginController.php | 48 ++++++++++--------- .../ClientFlowLoginControllerTest.php | 10 +++- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 088a6a9869..2e8216c2ba 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -197,7 +197,7 @@ class ClientFlowLoginController extends Controller { 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, - 'serverHost' => $this->request->getServerHost(), + 'serverHost' => $this->getServerPath(), 'oauthState' => $this->session->get('oauth.state'), ], 'guest' @@ -235,7 +235,7 @@ class ClientFlowLoginController extends Controller { 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, - 'serverHost' => $this->request->getServerHost(), + 'serverHost' => $this->getServerPath(), 'oauthState' => $this->session->get('oauth.state'), ], 'guest' @@ -345,27 +345,7 @@ class ClientFlowLoginController extends Controller { ); $this->session->remove('oauth.state'); } else { - $serverPostfix = ''; - - if (strpos($this->request->getRequestUri(), '/index.php') !== false) { - $serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/index.php')); - } else if (strpos($this->request->getRequestUri(), '/login/flow') !== false) { - $serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/login/flow')); - } - - $protocol = $this->request->getServerProtocol(); - - if ($protocol !== "https") { - $xForwardedProto = $this->request->getHeader('X-Forwarded-Proto'); - $xForwardedSSL = $this->request->getHeader('X-Forwarded-Ssl'); - if ($xForwardedProto === 'https' || $xForwardedSSL === 'on') { - $protocol = 'https'; - } - } - - - $serverPath = $protocol . "://" . $this->request->getServerHost() . $serverPostfix; - $redirectUri = 'nc://login/server:' . $serverPath . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); + $redirectUri = 'nc://login/server:' . $this->getServerPath() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); // Clear the token from the login here $this->tokenProvider->invalidateToken($sessionId); @@ -373,4 +353,26 @@ class ClientFlowLoginController extends Controller { return new Http\RedirectResponse($redirectUri); } + + private function getServerPath(): string { + $serverPostfix = ''; + + if (strpos($this->request->getRequestUri(), '/index.php') !== false) { + $serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/index.php')); + } else if (strpos($this->request->getRequestUri(), '/login/flow') !== false) { + $serverPostfix = substr($this->request->getRequestUri(), 0, strpos($this->request->getRequestUri(), '/login/flow')); + } + + $protocol = $this->request->getServerProtocol(); + + if ($protocol !== "https") { + $xForwardedProto = $this->request->getHeader('X-Forwarded-Proto'); + $xForwardedSSL = $this->request->getHeader('X-Forwarded-Ssl'); + if ($xForwardedProto === 'https' || $xForwardedSSL === 'on') { + $protocol = 'https'; + } + } + + return $protocol . "://" . $this->request->getServerHost() . $serverPostfix; + } } diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index 7fe87df026..b54897ddc4 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -162,6 +162,9 @@ class ClientFlowLoginControllerTest extends TestCase { ->expects($this->once()) ->method('getServerHost') ->willReturn('example.com'); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $expected = new TemplateResponse( 'core', @@ -172,7 +175,7 @@ class ClientFlowLoginControllerTest extends TestCase { 'instanceName' => 'ExampleCloud', 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'StateToken', - 'serverHost' => 'example.com', + 'serverHost' => 'https://example.com', 'oauthState' => 'OauthStateToken', ], 'guest' @@ -218,6 +221,9 @@ class ClientFlowLoginControllerTest extends TestCase { ->expects($this->once()) ->method('getServerHost') ->willReturn('example.com'); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $expected = new TemplateResponse( 'core', @@ -228,7 +234,7 @@ class ClientFlowLoginControllerTest extends TestCase { 'instanceName' => 'ExampleCloud', 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'StateToken', - 'serverHost' => 'example.com', + 'serverHost' => 'https://example.com', 'oauthState' => 'OauthStateToken', ], 'guest'