From 3971bbf8f4e2d660df30c52c3cd862dd18b55793 Mon Sep 17 00:00:00 2001
From: Thijs Kinkhorst <thijs@kinkhorst.com>
Date: Mon, 21 Feb 2022 20:43:25 +0000
Subject: [PATCH] Make ReturnTo parameter work again for logout.

For this move the detection and validtion of a ReturnTo parameter
from the cleardiscochoies method into a helper method so it can
be reused. Also update the dos for this change.
---
 docs/simplesamlphp-sp-api.md          |  4 ++--
 modules/core/lib/Controller/Login.php | 32 ++++++++++++++++++---------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/docs/simplesamlphp-sp-api.md b/docs/simplesamlphp-sp-api.md
index 91391200a..2182edac1 100644
--- a/docs/simplesamlphp-sp-api.md
+++ b/docs/simplesamlphp-sp-api.md
@@ -256,7 +256,7 @@ The URL returned by this function is static, and will not change.
 You can easily create your own links without using this function.
 The URL should be:
 
-     .../simplesaml/module.php/core/login/<authentication source>/?AuthId=<authentication source>&ReturnTo=<return URL>
+     .../simplesaml/module.php/core/login/<authentication source>?ReturnTo=<return URL>
 
 
 `getLogoutURL`
@@ -285,4 +285,4 @@ The URL returned by this function is static, and will not change.
 You can easily create your own links without using this function.
 The URL should be:
 
-     .../simplesaml/module.php/core/as_logout.php?AuthId=<authentication source>&ReturnTo=<return URL>
+     .../simplesaml/module.php/core/logout/<authentication source>?ReturnTo=<return URL>
diff --git a/modules/core/lib/Controller/Login.php b/modules/core/lib/Controller/Login.php
index 0cda6e748..2dfa37bec 100644
--- a/modules/core/lib/Controller/Login.php
+++ b/modules/core/lib/Controller/Login.php
@@ -51,21 +51,40 @@ class Login
     /**
      * Log the user out of a given authentication source.
      *
+     * @param Request $request The request that lead to this logout operation.
      * @param string $as The name of the auth source.
      *
      * @return \SimpleSAML\HTTP\RunnableResponse A runnable response which will actually perform logout.
      *
      * @throws \SimpleSAML\Error\CriticalConfigurationError
      */
-    public function logout(string $as): Response
+    public function logout(Request $request, string $as): RunnableResponse
     {
         $auth = new Auth\Simple($as);
+        $returnTo = $this->getReturnPath($request);
         return new RunnableResponse(
             [$auth, 'logout'],
-            [$this->config->getBasePath()]
+            [$returnTo]
         );
     }
 
+    /**
+     * Searches for a valid and allowed ReturnTo URL parameter,
+     * otherwise give the base installation page as a return point.
+     */
+    private function getReturnPath(Request $request): string
+    {
+        $httpUtils = new Utils\HTTP();
+
+        $returnTo = $request->query->get('ReturnTo', false);
+        if ($returnTo !== false) {
+            $returnTo = $httpUtils->checkURLAllowed($returnTo);
+        }
+        if (empty($returnTo)) {
+            return $this->config->getBasePath();
+        }
+        return $returnTo;
+    }
 
     /**
      * This clears the user's IdP discovery choices.
@@ -89,14 +108,7 @@ class Login
             $httpUtils->setCookie($cookieName, null, ['path' => $cookiePath, 'httponly' => false], false);
         }
 
-        // Find where we should go now.
-        $returnTo = $request->request->get('ReturnTo', false);
-        if ($returnTo !== false) {
-            $returnTo = $httpUtils->checkURLAllowed($returnTo);
-        } else {
-            // Return to the front page if no other destination is given. This is the same as the base cookie path.
-            $returnTo = $cookiePath;
-        }
+        $returnTo = $this->getReturnPath($request);
 
         // Redirect to destination.
         $httpUtils->redirectTrustedURL($returnTo);
-- 
GitLab