From 5678f432a43c00920c0e76056702f0609ac34ed1 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 | 44 ++++++++++--------- .../ClientFlowLoginControllerTest.php | 10 ++++- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 088a6a9869967..2e8216c2ba52a 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -197,7 +197,7 @@ public function showAuthPickerPage($clientIdentifier = '') { '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 @@ public function grantPage($stateToken = '', 'instanceName' => $this->defaults->getName(), 'urlGenerator' => $this->urlGenerator, 'stateToken' => $stateToken, - 'serverHost' => $this->request->getServerHost(), + 'serverHost' => $this->getServerPath(), 'oauthState' => $this->session->get('oauth.state'), ], 'guest' @@ -345,32 +345,34 @@ public function generateAppPassword($stateToken, ); $this->session->remove('oauth.state'); } else { - $serverPostfix = ''; + $redirectUri = 'nc://login/server:' . $this->getServerPath() . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); - 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')); - } + // Clear the token from the login here + $this->tokenProvider->invalidateToken($sessionId); + } - $protocol = $this->request->getServerProtocol(); + return new Http\RedirectResponse($redirectUri); + } - if ($protocol !== "https") { - $xForwardedProto = $this->request->getHeader('X-Forwarded-Proto'); - $xForwardedSSL = $this->request->getHeader('X-Forwarded-Ssl'); - if ($xForwardedProto === 'https' || $xForwardedSSL === 'on') { - $protocol = 'https'; - } - } + 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')); + } - $serverPath = $protocol . "://" . $this->request->getServerHost() . $serverPostfix; - $redirectUri = 'nc://login/server:' . $serverPath . '&user:' . urlencode($loginName) . '&password:' . urlencode($token); + $protocol = $this->request->getServerProtocol(); - // Clear the token from the login here - $this->tokenProvider->invalidateToken($sessionId); + 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 new Http\RedirectResponse($redirectUri); + return $protocol . "://" . $this->request->getServerHost() . $serverPostfix; } } diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index 7fe87df026f8f..b54897ddc4485 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -162,6 +162,9 @@ public function testShowAuthPickerPageWithOcsHeader() { ->expects($this->once()) ->method('getServerHost') ->willReturn('example.com'); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $expected = new TemplateResponse( 'core', @@ -172,7 +175,7 @@ public function testShowAuthPickerPageWithOcsHeader() { 'instanceName' => 'ExampleCloud', 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'StateToken', - 'serverHost' => 'example.com', + 'serverHost' => 'https://example.com', 'oauthState' => 'OauthStateToken', ], 'guest' @@ -218,6 +221,9 @@ public function testShowAuthPickerPageWithOauth() { ->expects($this->once()) ->method('getServerHost') ->willReturn('example.com'); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $expected = new TemplateResponse( 'core', @@ -228,7 +234,7 @@ public function testShowAuthPickerPageWithOauth() { 'instanceName' => 'ExampleCloud', 'urlGenerator' => $this->urlGenerator, 'stateToken' => 'StateToken', - 'serverHost' => 'example.com', + 'serverHost' => 'https://example.com', 'oauthState' => 'OauthStateToken', ], 'guest'