From 0359885baf842f2443424036efb2913e684cc0cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 11 Nov 2026 16:07:41 +0100 Subject: [PATCH] Read TARGET and a boolean renew in debug mode handleDebugMode() validated the ticket it had just issued against the service from a lowercase 'target' parameter, which no client sends: login() and the validation endpoints read 'TARGET'. With only TARGET given, debug mode therefore answered "Missing service parameter". It also passed the 'renew' query string straight to validate(), which takes a bool, so renew=true failed with a TypeError under strict types. Read 'TARGET', convert 'renew' with FILTER_VALIDATE_BOOL, and test debug mode with service and with TARGET, each with and without renew. Assisted-by: Claude Code (claude-opus-5-5, implementation & unit tests) Assisted-by: Google Antigravity (gemini-3.8-flash-high, review) --- docs/ChangeLog.md | 6 ++ src/Controller/LoginController.php | 7 ++- tests/src/Controller/LoginControllerTest.php | 61 ++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/docs/ChangeLog.md b/docs/ChangeLog.md index 8cb504b..2a892f0 100644 --- a/docs/ChangeLog.md +++ b/docs/ChangeLog.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- Debug mode (`debugMode=true`) now validates the ticket for a service given as `TARGET`, and no longer fails with a type error when `renew` is set (Marko Ivančić) + ## [7.0.6] - 2026-11-10 ### Security diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 161fa8b..ad118fb 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -28,6 +28,7 @@ use Symfony\Component\HttpKernel\Attribute\AsController; use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; +use function filter_var; use function http_build_query; use function in_array; use function var_export; @@ -311,8 +312,10 @@ public function handleDebugMode( $xmlResponse = $this->validate( request: $request, method: 'serviceValidate', - renew: $request->get('renew', false), - target: $request->get('target'), + // The query parameter is a string, validate() takes a bool + renew: filter_var($request->get('renew', false), FILTER_VALIDATE_BOOL), + // The same parameter login() reads the service from when there is no 'service' + target: $request->get('TARGET'), ticket: $serviceTicket['id'], service: $request->get('service'), pgtUrl: $request->get('pgtUrl'), diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php index 07b3592..1a44093 100644 --- a/tests/src/Controller/LoginControllerTest.php +++ b/tests/src/Controller/LoginControllerTest.php @@ -16,6 +16,7 @@ use SimpleSAML\Module\casserver\Controller\LoginController; use SimpleSAML\Session; use SimpleSAML\Utils; +use SimpleSAML\XHTML\Template; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -848,4 +849,64 @@ public function testAuthenticatedPostSubmitsViaPostWithTicket(): void $this->assertArrayHasKey('ticket', $params); $this->assertStringStartsWith('ST-', $params['ticket']); } + + + /** + * @return array}> + */ + public static function debugModeServiceValidateProvider(): array + { + $service = 'https://example.com/ssp/module.php/cas/linkback.php'; + return [ + 'service' => [['service' => $service, 'debugMode' => 'true']], + 'TARGET' => [['TARGET' => $service, 'debugMode' => 'true']], + 'service with renew' => [['service' => $service, 'debugMode' => 'true', 'renew' => 'true']], + 'TARGET with renew' => [['TARGET' => $service, 'debugMode' => 'true', 'renew' => 'true']], + ]; + } + + + /** + * Debug mode validates the ticket it has just issued against the service it was issued for, whichever query + * parameter named the service, and whether or not renew was requested. + * + * @param array $requestParams + * + * @throws \Exception + */ + #[DataProvider('debugModeServiceValidateProvider')] + public function testDebugModeValidatesTheIssuedTicket(array $requestParams): void + { + $moduleConfig = $this->moduleConfig; + $moduleConfig['debugMode'] = true; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->exactly(2))->method('getSessionId')->willReturn(session_create_id()); + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(true); + $this->authSimpleMock->expects($this->once())->method('getAuthData')->with('Expire')->willReturn(9999999999); + $this->authSimpleMock->expects($this->once())->method('getAuthDataArray')->willReturn([ + 'Attributes' => ['eduPersonPrincipalName' => ['testuser@example.com']], + 'Expire' => 9999999999, + ]); + + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $requestParams, + ); + $response = $this->callLogin($controllerMock, $loginRequest, $requestParams); + + $this->assertInstanceOf(Template::class, $response); + $this->assertSame(Response::HTTP_OK, $response->data['statusCode']); + $xml = simplexml_load_string((string)$response->data['DebugModeXml']); + $xml->registerXPathNamespace('cas', 'serviceResponse'); + $this->assertSame( + 'testuser@example.com', + (string)$xml->xpath('//cas:authenticationSuccess/cas:user')[0], + ); + } }