From 6717f8ff9ff85294ed973daebca6c7bc8e8a75ed Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 25 Jan 2022 17:47:58 +0100 Subject: [PATCH 1/5] Add direct arg to login flow Signed-off-by: Vincent Petry --- core/Controller/ClientFlowLoginController.php | 9 +++++++-- core/templates/loginflow/authpicker.php | 2 +- core/templates/loginflow/grant.php | 15 +++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 2ba26deb0e736..341a8bdc31603 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -164,10 +164,11 @@ private function stateTokenForbiddenResponse() { * @UseSession * * @param string $clientIdentifier + * @param int $direct * * @return StandaloneTemplateResponse */ - public function showAuthPickerPage($clientIdentifier = '') { + public function showAuthPickerPage($clientIdentifier = '', $direct = 0) { $clientName = $this->getClientName(); $client = null; if ($clientIdentifier !== '') { @@ -218,6 +219,7 @@ public function showAuthPickerPage($clientIdentifier = '') { 'stateToken' => $stateToken, 'serverHost' => $this->getServerPath(), 'oauthState' => $this->session->get('oauth.state'), + 'direct' => $direct, ], 'guest' ); @@ -234,10 +236,12 @@ public function showAuthPickerPage($clientIdentifier = '') { * * @param string $stateToken * @param string $clientIdentifier + * @param int $direct * @return StandaloneTemplateResponse */ public function grantPage($stateToken = '', - $clientIdentifier = '') { + $clientIdentifier = '', + $direct = 0) { if (!$this->isValidToken($stateToken)) { return $this->stateTokenForbiddenResponse(); } @@ -267,6 +271,7 @@ public function grantPage($stateToken = '', 'stateToken' => $stateToken, 'serverHost' => $this->getServerPath(), 'oauthState' => $this->session->get('oauth.state'), + 'direct' => $direct, ], 'guest' ); diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php index 02b4b9cc003b4..975039f0d92a9 100644 --- a/core/templates/loginflow/authpicker.php +++ b/core/templates/loginflow/authpicker.php @@ -46,7 +46,7 @@
diff --git a/core/templates/loginflow/grant.php b/core/templates/loginflow/grant.php index 0f1b9235a8971..c537c47ea6488 100644 --- a/core/templates/loginflow/grant.php +++ b/core/templates/loginflow/grant.php @@ -39,14 +39,17 @@
- - - - + + + + + + + +
-
+

From f48df63bb66436f9e5197e7bb81c13fa2f28cea0 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 25 Jan 2022 17:59:37 +0100 Subject: [PATCH 2/5] Also add direct outside the redirect url Signed-off-by: Vincent Petry --- core/templates/loginflow/authpicker.php | 1 + 1 file changed, 1 insertion(+) diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php index 975039f0d92a9..2982a1e096901 100644 --- a/core/templates/loginflow/authpicker.php +++ b/core/templates/loginflow/authpicker.php @@ -62,6 +62,7 @@

+ From b32c7e6926e2bb2ae0f1ed5bce7b4eda564e2685 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 25 Jan 2022 18:07:17 +0100 Subject: [PATCH 3/5] Don't double absolute url Signed-off-by: Carl Schwan --- core/Controller/LoginController.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 8a96db97c9e25..7957aa53fb8bd 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -267,7 +267,12 @@ private function canResetPassword(?string $passwordLink, ?IUser $user): bool { private function generateRedirect(?string $redirectUrl): RedirectResponse { if ($redirectUrl !== null && $this->userSession->isLoggedIn()) { - $location = $this->urlGenerator->getAbsoluteURL($redirectUrl); + $location = null; + if (str_starts_with($redirectUrl, 'http')) { + $location = $redirectUrl; + } else { + $location = $this->urlGenerator->getAbsoluteURL($redirectUrl); + } // Deny the redirect if the URL contains a @ // This prevents unvalidated redirects like ?redirect_url=:user@domain.com if (strpos($location, '@') === false) { From 36e74d2188b5a18addbd1f4d60deb6e19cea3e25 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 27 Jan 2022 16:26:37 +0100 Subject: [PATCH 4/5] Add more direct Signed-off-by: Carl Schwan --- core/Controller/ClientFlowLoginController.php | 18 ++++-------------- .../Middleware/Security/SecurityMiddleware.php | 3 +++ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 341a8bdc31603..d67a065a14eff 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -162,13 +162,8 @@ private function stateTokenForbiddenResponse() { * @PublicPage * @NoCSRFRequired * @UseSession - * - * @param string $clientIdentifier - * @param int $direct - * - * @return StandaloneTemplateResponse */ - public function showAuthPickerPage($clientIdentifier = '', $direct = 0) { + public function showAuthPickerPage(string $clientIdentifier = '', int $direct = 0): StandaloneTemplateResponse { $clientName = $this->getClientName(); $client = null; if ($clientIdentifier !== '') { @@ -233,15 +228,10 @@ public function showAuthPickerPage($clientIdentifier = '', $direct = 0) { * @NoCSRFRequired * @NoSameSiteCookieRequired * @UseSession - * - * @param string $stateToken - * @param string $clientIdentifier - * @param int $direct - * @return StandaloneTemplateResponse */ - public function grantPage($stateToken = '', - $clientIdentifier = '', - $direct = 0) { + public function grantPage(string $stateToken = '', + string $clientIdentifier = '', + int $direct = 0): StandaloneTemplateResponse { if (!$this->isValidToken($stateToken)) { return $this->stateTokenForbiddenResponse(); } diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index bd75118360488..3413eec8f43f9 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -223,6 +223,9 @@ public function afterException($controller, $methodName, \Exception $exception): if (isset($this->request->server['REQUEST_URI'])) { $params['redirect_url'] = $this->request->server['REQUEST_URI']; } + if ($this->request->getParam('direct')) { + $params['direct'] = 1; + } $url = $this->urlGenerator->linkToRoute('core.login.showLoginForm', $params); $response = new RedirectResponse($url); } else { From 2d49f72cbf51a7431399017a56457f13d2aa888f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 24 Feb 2022 14:05:41 +0100 Subject: [PATCH 5/5] Remove useless (and dangerous) redirection change Signed-off-by: Carl Schwan --- core/Controller/LoginController.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 7957aa53fb8bd..8a96db97c9e25 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -267,12 +267,7 @@ private function canResetPassword(?string $passwordLink, ?IUser $user): bool { private function generateRedirect(?string $redirectUrl): RedirectResponse { if ($redirectUrl !== null && $this->userSession->isLoggedIn()) { - $location = null; - if (str_starts_with($redirectUrl, 'http')) { - $location = $redirectUrl; - } else { - $location = $this->urlGenerator->getAbsoluteURL($redirectUrl); - } + $location = $this->urlGenerator->getAbsoluteURL($redirectUrl); // Deny the redirect if the URL contains a @ // This prevents unvalidated redirects like ?redirect_url=:user@domain.com if (strpos($location, '@') === false) {