user RunnableResponse

simplesamlphp · Jan 2, 2025 · fb66648 · fb66648
1 parent 7a8e7bb
commit fb66648
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 49 deletions.
diff --git a/config/module_casserver.php.dist b/config/module_casserver.php.dist
@@ -26,10 +26,11 @@ $config = [
         'https://host2.domain:5678/path2/path3',
         // So is regex
         '|^https://.*\.domain.com/|',
-        // Some configuration options can be overridden
+        // The FOLLOWING configuration options can be overridden
         'https://override.example.com' => [
             'attrname' => 'uid',
             'attributes_to_transfer' => ['cn'],
+            //'authproc' => []
         ],
     ],
 

diff --git a/src/Controller/Cas20Controller.php b/src/Controller/Cas20Controller.php
@@ -76,7 +76,7 @@ public function __construct(
 
     /**
      * @param   Request      $request
-     * @param   string|null  $TARGET
+     * @param   string|null  $TARGET   Query parameter name for "service" used by older CAS clients'
      * @param   bool         $renew    [OPTIONAL] - if this parameter is set, ticket validation will only succeed
      *                                 if the service ticket was issued from the presentation of the user’s primary
      *                                 credentials. It will fail if the ticket was issued from a single sign-on session.
@@ -195,7 +195,7 @@ public function proxy(
 
     /**
      * @param   Request      $request
-     * @param   string|null  $TARGET
+     * @param   string|null  $TARGET   Query parameter name for "service" used by older CAS clients'
      * @param   bool         $renew    [OPTIONAL] - if this parameter is set, ticket validation will only succeed
      *                                 if the service ticket was issued from the presentation of the user’s primary
      *                                 credentials. It will fail if the ticket was issued from a single sign-on session.

diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php
@@ -7,6 +7,7 @@
 use SimpleSAML\Auth\ProcessingChain;
 use SimpleSAML\Auth\Simple;
 use SimpleSAML\Configuration;
+use SimpleSAML\HTTP\RunnableResponse;
 use SimpleSAML\Logger;
 use SimpleSAML\Module;
 use SimpleSAML\Module\casserver\Cas\AttributeExtractor;
@@ -20,7 +21,6 @@
 use SimpleSAML\Module\casserver\Http\XmlResponse;
 use SimpleSAML\Session;
 use SimpleSAML\Utils;
-use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Attribute\AsController;
@@ -103,16 +103,14 @@ public function __construct(
      * @param   bool         $renew
      * @param   bool         $gateway
      * @param   string|null  $service
-     * @param   string|null  $TARGET
+     * @param   string|null  $TARGET  Query parameter name for "service" used by older CAS clients'
      * @param   string|null  $scope
      * @param   string|null  $language
      * @param   string|null  $entityId
      * @param   string|null  $debugMode
      * @param   string|null  $method
      *
-     * @return RedirectResponse|XmlResponse|null
-     * @throws NoState
-     * @throws \Exception
+     * @return RunnableResponse
      */
     public function login(
         Request $request,
@@ -125,13 +123,15 @@ public function login(
         #[MapQueryParameter] ?string $entityId = null,
         #[MapQueryParameter] ?string $debugMode = null,
         #[MapQueryParameter] ?string $method = null,
-    ): RedirectResponse|XmlResponse|null {
+    ): RunnableResponse {
         $forceAuthn = $renew;
         $serviceUrl = $service ?? $TARGET ?? null;
         $redirect = !(isset($method) && $method === 'POST');
 
         // Set initial configurations, or fail
         $this->handleServiceConfiguration($serviceUrl);
+        // Instantiate the classes that rely on the override configuration.
+        // We do not do this in the constructor since we do not have the correct values yet.
         $this->instantiateClassDependencies();
         $this->handleScope($scope);
         $this->handleLanguage($language);
@@ -177,9 +177,10 @@ public function login(
             /*
              *  REDIRECT TO AUTHSOURCE LOGIN
              * */
-            $this->authSource->login($params);
-            // We should never get here.This is to facilitate testing.
-            return null;
+            return new RunnableResponse(
+                [$this->authSource, 'login'],
+                [$params],
+            );
         }
 
         // We are Authenticated.
@@ -195,13 +196,11 @@ public function login(
          *  We are done. REDIRECT TO LOGGEDIN
          * */
         if (!isset($serviceUrl) && $this->authProcId === null) {
-            $urlParameters = $this->httpUtils->addURLParameters(
-                Module::getModuleURL('casserver/loggedIn'),
-                $this->postAuthUrlParameters,
+            $loggedInUrl = Module::getModuleURL('casserver/loggedIn');
+            return new RunnableResponse(
+                [$this->httpUtils, 'redirectTrustedURL'],
+                [$loggedInUrl, $this->postAuthUrlParameters],
             );
-            $this->httpUtils->redirectTrustedURL($urlParameters);
-            // We should never get here.This is to facilitate testing.
-            return null;
         }
 
         // Get the state.
@@ -232,16 +231,16 @@ public function login(
 
         // GET
         if ($redirect) {
-            $this->httpUtils->redirectTrustedURL(
-                $this->httpUtils->addURLParameters($serviceUrl, $this->postAuthUrlParameters),
+            return new RunnableResponse(
+                [$this->httpUtils, 'redirectTrustedURL'],
+                [$serviceUrl, $this->postAuthUrlParameters],
             );
-            // We should never get here.This is to facilitate testing.
-            return null;
         }
         // POST
-        $this->httpUtils->submitPOSTData($serviceUrl, $this->postAuthUrlParameters);
-        // We should never get here.This is to facilitate testing.
-        return null;
+        return new RunnableResponse(
+            [$this->httpUtils, 'submitPOSTData'],
+            [$serviceUrl, $this->postAuthUrlParameters],
+        );
     }
 
     /**

diff --git a/src/Controller/LogoutController.php b/src/Controller/LogoutController.php
@@ -81,12 +81,12 @@ public function __construct(
      * @param   Request      $request
      * @param   string|null  $url
      *
-     * @return RunnableResponse|null
+     * @return RunnableResponse
      */
     public function logout(
         Request $request,
         #[MapQueryParameter] ?string $url = null,
-    ): RunnableResponse|null {
+    ): RunnableResponse {
         if (!$this->casConfig->getOptionalValue('enable_logout', false)) {
             $this->handleExceptionThrown('Logout not allowed');
         }
@@ -120,10 +120,10 @@ public function logout(
         }
 
         // Logout and redirect
-        $this->authSource->logout($logoutRedirectUrl);
-
-        // We should never get here
-        return null;
+        return new RunnableResponse(
+            [$this->authSource, 'logout'],
+            [$logoutRedirectUrl],
+        );
     }
 
     /**

diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php
@@ -10,6 +10,7 @@
 use SimpleSAML\Auth\Simple;
 use SimpleSAML\Compat\SspContainer;
 use SimpleSAML\Configuration;
+use SimpleSAML\HTTP\RunnableResponse;
 use SimpleSAML\Module;
 use SimpleSAML\Module\casserver\Controller\LoginController;
 use SimpleSAML\Session;
@@ -209,11 +210,13 @@ public function testAuthSourceLogin(array $requestParameters, array $loginParame
             ->getMock();
         $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock);
         $this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(false);
-        $this->authSimpleMock->expects($this->once())->method('login')->with($loginParameters);
         $sessionId = session_create_id();
         $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn($sessionId);
 
-        $controllerMock->login($loginRequest, ...$requestParameters);
+        $response = $controllerMock->login($loginRequest, ...$requestParameters);
+        $this->assertInstanceOf(RunnableResponse::class, $response);
+        $callable = (array)$response->getCallable();
+        $this->assertEquals('login', $callable[1] ?? '');
     }
 
     /**
@@ -233,14 +236,16 @@ public function testIsAuthenticatedRedirectsToLoggedIn(): void
         $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock);
         $this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(true);
         $this->authSimpleMock->expects($this->once())->method('getAuthData')->with('Expire')->willReturn(9999999999);
-        $this->httpUtils->expects($this->once())->method('redirectTrustedURL')
-            ->with('http://localhost/module.php/casserver/loggedIn?');
+
         $loginRequest = Request::create(
             uri:        Module::getModuleURL('casserver/login'),
             parameters: [],
         );
 
-        $controllerMock->login($loginRequest);
+        $response = $controllerMock->login($loginRequest);
+        $this->assertInstanceOf(RunnableResponse::class, $response);
+        $callable = (array)$response->getCallable();
+        $this->assertEquals('redirectTrustedURL', $callable[1] ?? '');
     }
 
     public static function validServiceUrlProvider(): array
@@ -300,22 +305,19 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b
 
         $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock);
         $this->authSimpleMock->expects($this->any())->method('isAuthenticated')->willReturn(true);
-        $this->httpUtils->expects($this->once())->method('redirectTrustedURL')
-            ->withAnyParameters()
-            ->willReturnCallback(function ($url) use ($redirectURL) {
-                $this->assertStringStartsWith(
-                    $redirectURL,
-                    $url,
-                    'Ticket should be part of the redirect.',
-                );
-            });
         $queryParameters = [$serviceParam => 'https://example.com/ssp/module.php/cas/linkback.php'];
         $loginRequest = Request::create(
             uri:        Module::getModuleURL('casserver/login'),
             parameters: $queryParameters,
         );
 
         /** @psalm-suppress InvalidArgument */
-        $controllerMock->login($loginRequest, ...$queryParameters);
+        $response = $controllerMock->login($loginRequest, ...$queryParameters);
+        $this->assertInstanceOf(RunnableResponse::class, $response);
+        $arguments = $response->getArguments();
+        $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]);
+        $this->assertStringStartsWith('ST-', array_values($arguments[1])[0] ?? []);
+        $callable = (array)$response->getCallable();
+        $this->assertEquals('redirectTrustedURL', $callable[1] ?? '');
     }
 }
diff --git a/tests/src/Controller/LogoutControllerTest.php b/tests/src/Controller/LogoutControllerTest.php
@@ -8,6 +8,7 @@
 use PHPUnit\Framework\TestCase;
 use SimpleSAML\Auth\Simple;
 use SimpleSAML\Configuration;
+use SimpleSAML\HTTP\RunnableResponse;
 use SimpleSAML\Module;
 use SimpleSAML\Module\casserver\Controller\LogoutController;
 use SimpleSAML\Session;
@@ -160,11 +161,19 @@ public function testLogoutNoRedirectUrlOnNoSkipLogoutAuthenticated(): void
 
         // Unauthenticated
         $this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(true);
-        $this->authSimpleMock->expects($this->once())->method('logout')
-            ->with('http://localhost/module.php/casserver/loggedOut');
 
         $controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils);
-        $controller->logout(Request::create('/'));
+        $queryParameters = ['url' => 'http://localhost/module.php/casserver/loggedOut'];
+        $logoutRequest = Request::create(
+            uri:        Module::getModuleURL('casserver/loggedOut'),
+            parameters: $queryParameters,
+        );
+
+        $response = $controller->logout($logoutRequest, ...$queryParameters);
+
+        $this->assertInstanceOf(RunnableResponse::class, $response);
+        $callable = (array)$response->getCallable();
+        $this->assertEquals('logout', $callable[1] ?? '');
     }
 
     public function testTicketIdGetsDeletedOnLogout(): void