From 06e51f782781f283f688bbfb05e6f6d2a47035bf Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sat, 25 Oct 2025 15:32:42 +0300 Subject: [PATCH 1/4] Improve gateway parameter handling to properly match the protocol description --- config/module_casserver.php.dist | 4 +- src/Controller/LoginController.php | 178 +++++++++-- tests/src/Controller/LoginControllerTest.php | 316 +++++++++++++++++++ 3 files changed, 469 insertions(+), 29 deletions(-) diff --git a/config/module_casserver.php.dist b/config/module_casserver.php.dist index 7e690160..7da85e39 100644 --- a/config/module_casserver.php.dist +++ b/config/module_casserver.php.dist @@ -115,8 +115,10 @@ $config = [ 'service_ticket_expire_time' => 5, // how many seconds proxy granting tickets are valid for at most, defaults to 3600 'proxy_granting_ticket_expire_time' => 600, - //how many seconds proxy tickets are valid for, defaults to 5 + // how many seconds proxy tickets are valid for, defaults to 5 'proxy_ticket_expire_time' => 5, + // OPTIONAL, enable CAS passive mode, defaults to false + //'enable_passive_mode' => true, // If query param debugMode=true is sent to the login endpoint then print cas ticket xml. Default false 'debugMode' => true, diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 2b7c4f88..0f708573 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -164,34 +164,34 @@ public function login( // This will be used to come back from the AuthSource login or from the Processing Chain $returnToUrl = $this->getReturnUrl($request, $sessionTicket); - // Authenticate - if ( - $requestForceAuthenticate || !$this->authSource->isAuthenticated() - ) { - $params = [ - 'ForceAuthn' => $forceAuthn, - 'isPassive' => $gateway, - 'ReturnTo' => $returnToUrl, - ]; - - if (isset($entityId)) { - $params['saml:idp'] = $entityId; - } + // Use case 4: renew=true and gateway=true are incompatible → prefer interactive login (disable passive) + if ($gateway && $forceAuthn) { + $gateway = false; + } - if (isset($this->idpList)) { - if (count($this->idpList) > 1) { - $params['saml:IDPList'] = $this->idpList; - } else { - $params['saml:idp'] = $this->idpList[0]; - } + // Handle passive authentication + if ($gateway && !$this->authSource->isAuthenticated() && !$requestForceAuthenticate) { + $gwResult = $this->handleUnauthenticatedGateway( + $request, + $serviceUrl, + $entityId, + $returnToUrl, + ); + $gateway = $gwResult['gateway']; + if ($gwResult['response'] !== null) { + return $gwResult['response']; } + } - /* - * REDIRECT TO AUTHSOURCE LOGIN - * */ - return new RunnableResponse( - [$this->authSource, 'login'], - [$params], + // Handle interactive authentication + if ( + $requestForceAuthenticate || !$this->authSource->isAuthenticated() + ) { + return $this->handleInteractiveAuthenticate( + forceAuthn: $forceAuthn, + gateway: $gateway, + returnToUrl: $returnToUrl, + entityId: $entityId, ); } @@ -204,9 +204,8 @@ public function login( $this->ticketStore->addTicket($sessionTicket); } - /* - * We are done. REDIRECT TO LOGGEDIN - * */ + /* We are done. REDIRECT TO LOGGEDIN */ + if (!isset($serviceUrl) && $this->authProcId === null) { $loggedInUrl = Module::getModuleURL('casserver/loggedIn'); return new RunnableResponse( @@ -251,6 +250,7 @@ public function login( return $t; } + // Use case 1: user has SSO or non-interactive auth succeeded → redirect/POST to service WITH a ticket $ticketName = $this->calculateTicketName($service); $this->postAuthUrlParameters[$ticketName] = $serviceTicket['id']; @@ -464,4 +464,126 @@ private function instantiateClassDependencies(): void // Attribute Extractor $this->attributeExtractor = new AttributeExtractor($this->casConfig, $processingChainFactory); } + + /** + * Trigger interactive authentication via the AuthSource. + * + * @param bool $forceAuthn + * @param bool $gateway + * @param string $returnToUrl + * @param string|null $entityId + * + * @return RunnableResponse + */ + private function handleInteractiveAuthenticate( + bool $forceAuthn, + bool $gateway, + string $returnToUrl, + ?string $entityId, + ): RunnableResponse { + $params = [ + 'ForceAuthn' => $forceAuthn, + 'isPassive' => $gateway, + 'ReturnTo' => $returnToUrl, + ]; + + if (isset($entityId)) { + $params['saml:idp'] = $entityId; + } + + if (isset($this->idpList)) { + if (sizeof($this->idpList) > 1) { + $params['saml:IDPList'] = $this->idpList; + } else { + $params['saml:idp'] = $this->idpList[0]; + } + } + + return new RunnableResponse( + [$this->authSource, 'login'], + [$params], + ); + } + + + /** + * Handle the gateway flow when the user is NOT authenticated. + * Passive mode is only attempted if 'enable_passive_mode' is enabled in configuration. + * + * Returns: + * - ['response' => RunnableResponse|null, 'gateway' => bool] where 'gateway' may be toggled off for scenario 3. + */ + private function handleUnauthenticatedGateway( + Request $request, + ?string $serviceUrl, + ?string $entityId, + string $returnToUrl, + ): array { + $passiveAllowed = $this->casConfig->getOptionalBoolean('enable_passive_mode', false); + + // If passive is not enabled by configuration, follow scenario 2/3 directly. + if (!$passiveAllowed) { + if ($serviceUrl !== null) { + // Scenario 2: redirect to service WITHOUT any CAS parameters (always via GET redirect) + return [ + 'response' => new RunnableResponse( + [$this->httpUtils, 'redirectTrustedURL'], + [$serviceUrl, []], + ), + 'gateway' => true, + ]; + } + // Scenario 3: no service → disable gateway and proceed with interactive login + return ['response' => null, 'gateway' => false]; + } + + // Passive mode enabled: try a passive (non-interactive) authentication once + $gatewayTried = $this->getRequestParam($request, 'gatewayTried'); + if ($gatewayTried !== '1') { + $rt = str_contains($returnToUrl, 'gatewayTried=') + ? $returnToUrl + : $returnToUrl . (str_contains($returnToUrl, '?') ? '&' : '?') . 'gatewayTried=1'; + + $passiveParams = [ + 'ForceAuthn' => false, + 'isPassive' => true, + 'ReturnTo' => $rt, + ]; + + if (isset($entityId)) { + $passiveParams['saml:idp'] = $entityId; + } + + if (isset($this->idpList)) { + if (sizeof($this->idpList) > 1) { + $passiveParams['saml:IDPList'] = $this->idpList; + } else { + $passiveParams['saml:idp'] = $this->idpList[0]; + } + } + + return [ + 'response' => new RunnableResponse( + [$this->authSource, 'login'], + [$passiveParams], + ), + 'gateway' => true, + ]; + } + + // Passive attempt already performed and still not authenticated. + if ($serviceUrl !== null) { + // Scenario 2: redirect to service WITHOUT any CAS parameters (always via GET redirect) + return [ + 'response' => new RunnableResponse( + [$this->httpUtils, 'redirectTrustedURL'], + [$serviceUrl, []], + ), + 'gateway' => true, + ]; + } + + // Scenario 3: no service provided → disable gateway and proceed with interactive login + return ['response' => null, 'gateway' => false]; + } } diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php index ba6b7a61..1babaa0f 100644 --- a/tests/src/Controller/LoginControllerTest.php +++ b/tests/src/Controller/LoginControllerTest.php @@ -215,8 +215,15 @@ public function testAuthSourceLogin(array $requestParameters, array $loginParame $response = $controllerMock->login($loginRequest, ...$requestParameters); $this->assertInstanceOf(RunnableResponse::class, $response); + + // Assert we call into authSource->login $callable = (array)$response->getCallable(); $this->assertEquals('login', $callable[1] ?? ''); + + // Assert the interactive authenticate parameters (entityId/idpList logic) + $arguments = $response->getArguments(); + $actualLoginParams = $arguments[0] ?? []; + $this->assertEquals($loginParameters, $actualLoginParams); } /** @@ -319,4 +326,313 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b $callable = (array)$response->getCallable(); $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); } + + public function testGatewayPassiveDisabledRedirectsWithoutParams(): void + { + // enable_passive_mode disabled + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = false; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $params = [ + 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'gateway' => true, + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $params, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + // Unauthenticated so gateway path is exercised + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + // Session used to build ReturnTo + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + $response = $controllerMock->login($loginRequest, ...$params); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); + + // Service URL unchanged and NO CAS params appended + $arguments = $response->getArguments(); + $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]); + $this->assertSame([], $arguments[1] ?? []); + } + + public function testGatewayPassiveEnabledPerformsPassiveAttempt(): void + { + // enable_passive_mode enabled + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = true; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $params = [ + 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'gateway' => true, + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $params, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + // Unauthenticated so gateway path is exercised, first passive attempt + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + // Session used to build ReturnTo + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + $response = $controllerMock->login($loginRequest, ...$params); + + $this->assertInstanceOf(RunnableResponse::class, $response); + + // Should attempt passive auth via authSource->login + $callable = (array)$response->getCallable(); + $this->assertEquals('login', $callable[1] ?? ''); + + // Verify isPassive=true, ForceAuthn=false, ReturnTo contains gatewayTried=1 + $arguments = $response->getArguments(); + $actualLoginParams = $arguments[0] ?? []; + $this->assertArrayHasKey('ForceAuthn', $actualLoginParams); + $this->assertFalse($actualLoginParams['ForceAuthn']); + $this->assertArrayHasKey('isPassive', $actualLoginParams); + $this->assertTrue($actualLoginParams['isPassive']); + $this->assertArrayHasKey('ReturnTo', $actualLoginParams); + $this->assertIsString($actualLoginParams['ReturnTo']); + $this->assertStringContainsString('gatewayTried=1', $actualLoginParams['ReturnTo']); + } + + public function testGatewayPassiveEnabledSecondPassRedirectsWithoutParams(): void + { + // enable_passive_mode enabled + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = true; + $casconfig = Configuration::loadFromArray($moduleConfig); + + // Include gatewayTried in the Request only + $requestParams = [ + 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'gateway' => true, + 'gatewayTried' => '1', // simulate second pass after passive attempt + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $requestParams, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + // Do not pass 'gatewayTried' as named argument + $callParams = $requestParams; + unset($callParams['gatewayTried']); + + $response = $controllerMock->login($loginRequest, ...$callParams); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); + + $arguments = $response->getArguments(); + $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]); + $this->assertSame([], $arguments[1] ?? []); + } + + public function testGatewayNoServicePassiveDisabledFallsBackToInteractive(): void + { + // enable_passive_mode disabled + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = false; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $params = [ + 'gateway' => true, // no 'service' provided + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $params, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + $response = $controllerMock->login($loginRequest, ...$params); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('login', $callable[1] ?? ''); + + // isPassive should be false because gateway is disabled when no service in this scenario + $arguments = $response->getArguments(); + $loginArgs = $arguments[0] ?? []; + $this->assertArrayHasKey('isPassive', $loginArgs); + $this->assertFalse($loginArgs['isPassive']); + } + + public function testRenewAndGatewayConflictDisablesPassive(): void + { + // enable_passive_mode enabled (doesn't matter because renew must disable passive) + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = true; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $params = [ + 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'gateway' => true, + 'renew' => true, + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $params, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + $response = $controllerMock->login($loginRequest, ...$params); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('login', $callable[1] ?? ''); + + // isPassive must be false when renew=true (gateway disabled) + $arguments = $response->getArguments(); + $loginArgs = $arguments[0] ?? []; + $this->assertArrayHasKey('isPassive', $loginArgs); + $this->assertFalse($loginArgs['isPassive']); + $this->assertArrayHasKey('ForceAuthn', $loginArgs); + $this->assertTrue($loginArgs['ForceAuthn']); + } + + public function testAuthenticatedPostSubmitsViaPostWithTicket(): void + { + $moduleConfig = $this->moduleConfig; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $requestParams = [ + 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'method' => 'POST', + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $requestParams, + ); + + // Prepare controller with mocks + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + $sessionId = session_create_id(); + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->exactly(2))->method('getSessionId')->willReturn($sessionId); + + // Simulate authenticated state and required auth data + $this->authSimpleMock->expects($this->any())->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, + ], + ]); + + $response = $controllerMock->login($loginRequest, ...$requestParams); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('submitPOSTData', $callable[1] ?? ''); + + $arguments = $response->getArguments(); + $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]); + $params = $arguments[1] ?? []; + // default ticket name for CAS is 'ticket' + $this->assertArrayHasKey('ticket', $params); + $this->assertStringStartsWith('ST-', $params['ticket']); + } + + public function testGatewayRedirectPreservesServiceQueryWithoutTicket(): void + { + // Passive disabled to trigger direct redirect with no ticket + $moduleConfig = $this->moduleConfig; + $moduleConfig['enable_passive_mode'] = false; + + // Service URL with existing query parameters + $serviceWithQuery = 'https://example.com/ssp/module.php/cas/linkback.php?foo=1&bar=2'; + // Ensure the exact service URL (including its query string) is allowed + $moduleConfig['legal_service_urls'] = [$serviceWithQuery]; + + $casconfig = Configuration::loadFromArray($moduleConfig); + + $params = [ + 'service' => $serviceWithQuery, + 'gateway' => true, + ]; + + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $params, + ); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + // Unauthenticated so gateway path is exercised + $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); + + // Session used to build ReturnTo + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + + // Execute + $response = $controllerMock->login($loginRequest, ...$params); + + // Validate redirect with original query intact and no CAS parameters appended + $this->assertInstanceOf(RunnableResponse::class, $response); + $callable = (array)$response->getCallable(); + $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); + + $arguments = $response->getArguments(); + // URL should be exactly the original service URL including its query string + $this->assertEquals($serviceWithQuery, $arguments[0]); + // No additional parameters (e.g., no ticket) should be appended by CAS + $this->assertSame([], $arguments[1] ?? []); + } } From f71834b6d5869d39868dc749598a71b42665cae4 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 31 Oct 2025 19:12:34 +0200 Subject: [PATCH 2/4] Further simplify the code --- src/Controller/LoginController.php | 177 ++++++++++++++++------------- 1 file changed, 96 insertions(+), 81 deletions(-) diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 0f708573..70a18adf 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -164,32 +164,36 @@ public function login( // This will be used to come back from the AuthSource login or from the Processing Chain $returnToUrl = $this->getReturnUrl($request, $sessionTicket); - // Use case 4: renew=true and gateway=true are incompatible → prefer interactive login (disable passive) + // renew=true and gateway=true are incompatible → prefer interactive login (disable passive) + // Protocol: gateway and renew are incompatible; behavior is undefined if both are set. + // OPTIONAL (implementation policy): Prefer renew (interactive, non-passive) by disabling gateway. + // OPTIONAL alternative: Reject with 400 to signal incompatible parameters. if ($gateway && $forceAuthn) { $gateway = false; } // Handle passive authentication + // Protocol (gateway set): CAS MUST NOT prompt for credentials during this branch. if ($gateway && !$this->authSource->isAuthenticated() && !$requestForceAuthenticate) { - $gwResult = $this->handleUnauthenticatedGateway( + $response = $this->handleUnauthenticatedGateway( $request, $serviceUrl, $entityId, $returnToUrl, ); - $gateway = $gwResult['gateway']; - if ($gwResult['response'] !== null) { - return $gwResult['response']; + if ($response !== null) { + return $response; } } // Handle interactive authentication + // Protocol: Normal interactive authentication flow (applies when gateway is not in effect). + // Renew semantics: when renew=true, server MUST enforce re-authentication (no SSO reuse). if ( $requestForceAuthenticate || !$this->authSource->isAuthenticated() ) { return $this->handleInteractiveAuthenticate( forceAuthn: $forceAuthn, - gateway: $gateway, returnToUrl: $returnToUrl, entityId: $entityId, ); @@ -250,7 +254,10 @@ public function login( return $t; } - // Use case 1: user has SSO or non-interactive auth succeeded → redirect/POST to service WITH a ticket + // User has SSO or non-interactive auth succeeded → redirect/POST to service WITH a ticket + // Protocol: With gateway and a successful non-interactive auth (or existing SSO), CAS MAY redirect to the + // service and append a ticket. + // Protocol: CAS MAY interpose an advisory page indicating that authentication took place. $ticketName = $this->calculateTicketName($service); $this->postAuthUrlParameters[$ticketName] = $serviceTicket['id']; @@ -469,7 +476,6 @@ private function instantiateClassDependencies(): void * Trigger interactive authentication via the AuthSource. * * @param bool $forceAuthn - * @param bool $gateway * @param string $returnToUrl * @param string|null $entityId * @@ -477,113 +483,122 @@ private function instantiateClassDependencies(): void */ private function handleInteractiveAuthenticate( bool $forceAuthn, - bool $gateway, string $returnToUrl, ?string $entityId, ): RunnableResponse { - $params = [ - 'ForceAuthn' => $forceAuthn, - 'isPassive' => $gateway, - 'ReturnTo' => $returnToUrl, - ]; - - if (isset($entityId)) { - $params['saml:idp'] = $entityId; - } - - if (isset($this->idpList)) { - if (sizeof($this->idpList) > 1) { - $params['saml:IDPList'] = $this->idpList; - } else { - $params['saml:idp'] = $this->idpList[0]; - } - } - - return new RunnableResponse( - [$this->authSource, 'login'], - [$params], + return $this->handleAuthenticate( + forceAuthn: $forceAuthn, + gateway: false, + returnToUrl: $returnToUrl, + entityId: $entityId, ); } - /** * Handle the gateway flow when the user is NOT authenticated. * Passive mode is only attempted if 'enable_passive_mode' is enabled in configuration. * - * Returns: - * - ['response' => RunnableResponse|null, 'gateway' => bool] where 'gateway' may be toggled off for scenario 3. + * Returns: RunnableResponse|null + * - RunnableResponse for either a passive attempt or a redirect to service without ticket. + * - null to indicate: proceed with interactive login (non-passive). */ private function handleUnauthenticatedGateway( Request $request, ?string $serviceUrl, ?string $entityId, string $returnToUrl, - ): array { + ): ?RunnableResponse { $passiveAllowed = $this->casConfig->getOptionalBoolean('enable_passive_mode', false); - // If passive is not enabled by configuration, follow scenario 2/3 directly. + // Passive mode is not enabled by configuration. + // Protocol: If non-interactive auth cannot be established: + // - If service is present, CAS MUST redirect to the service URL WITHOUT a ticket parameter. + // - If service is absent, behavior is undefined; it is RECOMMENDED + // to request credentials as if neither parameter was specified. if (!$passiveAllowed) { - if ($serviceUrl !== null) { - // Scenario 2: redirect to service WITHOUT any CAS parameters (always via GET redirect) - return [ - 'response' => new RunnableResponse( - [$this->httpUtils, 'redirectTrustedURL'], - [$serviceUrl, []], - ), - 'gateway' => true, - ]; - } - // Scenario 3: no service → disable gateway and proceed with interactive login - return ['response' => null, 'gateway' => false]; + // Passive attempt already performed and still not authenticated. + return $this->gatewayFallback($serviceUrl); } // Passive mode enabled: try a passive (non-interactive) authentication once + // OPTIONAL (implementation): Attempt passive exactly once while avoiding loops. $gatewayTried = $this->getRequestParam($request, 'gatewayTried'); if ($gatewayTried !== '1') { $rt = str_contains($returnToUrl, 'gatewayTried=') ? $returnToUrl : $returnToUrl . (str_contains($returnToUrl, '?') ? '&' : '?') . 'gatewayTried=1'; - $passiveParams = [ - 'ForceAuthn' => false, - 'isPassive' => true, - 'ReturnTo' => $rt, - ]; + return $this->handleAuthenticate( + forceAuthn: false, + gateway: true, + returnToUrl: $rt, + entityId: $entityId, + ); + } - if (isset($entityId)) { - $passiveParams['saml:idp'] = $entityId; - } + // Passive attempt already performed and still not authenticated. + return $this->gatewayFallback($serviceUrl); + } - if (isset($this->idpList)) { - if (sizeof($this->idpList) > 1) { - $passiveParams['saml:IDPList'] = $this->idpList; - } else { - $passiveParams['saml:idp'] = $this->idpList[0]; - } - } + /** + * Handle authentication request by configuring parameters and triggering login via auth source. + * + * @param bool $forceAuthn Whether to force authentication regardless of existing session + * @param bool $gateway Whether authentication should be passive/non-interactive + * @param string $returnToUrl URL to return to after authentication + * @param string|null $entityId Optional specific IdP entity ID to use + * + * @return RunnableResponse Response containing the login redirect + */ + private function handleAuthenticate( + bool $forceAuthn, + bool $gateway, + string $returnToUrl, + ?string $entityId, + ): RunnableResponse { + $params = [ + 'ForceAuthn' => $forceAuthn, + 'isPassive' => $gateway, + 'ReturnTo' => $returnToUrl, + ]; - return [ - 'response' => new RunnableResponse( - [$this->authSource, 'login'], - [$passiveParams], - ), - 'gateway' => true, - ]; + if (isset($entityId)) { + $params['saml:idp'] = $entityId; } - // Passive attempt already performed and still not authenticated. - if ($serviceUrl !== null) { - // Scenario 2: redirect to service WITHOUT any CAS parameters (always via GET redirect) - return [ - 'response' => new RunnableResponse( - [$this->httpUtils, 'redirectTrustedURL'], - [$serviceUrl, []], - ), - 'gateway' => true, - ]; + if (isset($this->idpList)) { + if (sizeof($this->idpList) > 1) { + $params['saml:IDPList'] = $this->idpList; + } else { + $params['saml:idp'] = $this->idpList[0]; + } } - // Scenario 3: no service provided → disable gateway and proceed with interactive login - return ['response' => null, 'gateway' => false]; + return new RunnableResponse( + [$this->authSource, 'login'], + [$params], + ); + } + + /** + * Gateway fallback per CAS gateway semantics: + * - Protocol (MUST): If a service is provided and non-interactive auth cannot be established, + * redirect to the service WITHOUT any CAS parameters (no "ticket"). + * - Protocol (Undefined, RECOMMENDED): If no service is provided, proceed with interactive login + * (request credentials). + * @param string|null $serviceUrl + * @return RunnableResponse|null + */ + private function gatewayFallback(?string $serviceUrl): ?RunnableResponse + { + if ($serviceUrl !== null) { + // MUST: Redirect to service WITHOUT a "ticket" parameter (and without other CAS params). + return new RunnableResponse( + [$this->httpUtils, 'redirectTrustedURL'], + [$serviceUrl, []], + ); + } + // RECOMMENDED: No service specified; proceed with interactive login as if neither parameter was specified. + return null; } } From 526159d8189d4fcef95aaf6dfde78aec1be86ffa Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Wed, 5 Nov 2025 19:26:11 +0200 Subject: [PATCH 3/4] Remove loop handler query parameter. Loop prevention in passive mode is not a realistic scenario. --- src/Controller/LoginController.php | 27 +--- tests/src/Controller/LoginControllerTest.php | 127 ++++--------------- 2 files changed, 35 insertions(+), 119 deletions(-) diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 70a18adf..fbb69d08 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -176,7 +176,6 @@ public function login( // Protocol (gateway set): CAS MUST NOT prompt for credentials during this branch. if ($gateway && !$this->authSource->isAuthenticated() && !$requestForceAuthenticate) { $response = $this->handleUnauthenticatedGateway( - $request, $serviceUrl, $entityId, $returnToUrl, @@ -503,7 +502,6 @@ private function handleInteractiveAuthenticate( * - null to indicate: proceed with interactive login (non-passive). */ private function handleUnauthenticatedGateway( - Request $request, ?string $serviceUrl, ?string $entityId, string $returnToUrl, @@ -520,24 +518,13 @@ private function handleUnauthenticatedGateway( return $this->gatewayFallback($serviceUrl); } - // Passive mode enabled: try a passive (non-interactive) authentication once - // OPTIONAL (implementation): Attempt passive exactly once while avoiding loops. - $gatewayTried = $this->getRequestParam($request, 'gatewayTried'); - if ($gatewayTried !== '1') { - $rt = str_contains($returnToUrl, 'gatewayTried=') - ? $returnToUrl - : $returnToUrl . (str_contains($returnToUrl, '?') ? '&' : '?') . 'gatewayTried=1'; - - return $this->handleAuthenticate( - forceAuthn: false, - gateway: true, - returnToUrl: $rt, - entityId: $entityId, - ); - } - - // Passive attempt already performed and still not authenticated. - return $this->gatewayFallback($serviceUrl); + // Passive mode enabled: attempt a passive (non-interactive) authentication. + return $this->handleAuthenticate( + forceAuthn: false, + gateway: true, + returnToUrl: $returnToUrl, + entityId: $entityId, + ); } /** diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php index 1babaa0f..4089ba19 100644 --- a/tests/src/Controller/LoginControllerTest.php +++ b/tests/src/Controller/LoginControllerTest.php @@ -318,6 +318,7 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b parameters: $queryParameters, ); + /** @psalm-suppress InvalidArgument */ $response = $controllerMock->login($loginRequest, ...$queryParameters); $this->assertInstanceOf(RunnableResponse::class, $response); $arguments = $response->getArguments(); @@ -327,15 +328,35 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); } - public function testGatewayPassiveDisabledRedirectsWithoutParams(): void + /** + * @return array + */ + public function serviceUrlsProvider(): array + { + return [ + ['https://example.com/ssp/module.php/cas/linkback.php'], + ['https://example.com/ssp/module.php/cas/linkback.php?foo=1&bar=2'], + ]; + } + + /** + * When passive is disabled and a service is provided, CAS must redirect to the service without appending CAS params + * + * @dataProvider serviceUrlsProvider + */ + public function testGatewayPassiveDisabledRedirectsWithoutParams(string $serviceUrl): void { // enable_passive_mode disabled $moduleConfig = $this->moduleConfig; $moduleConfig['enable_passive_mode'] = false; + + // Ensure the exact service URL (including its query string, if any) is allowed + $moduleConfig['legal_service_urls'] = [$serviceUrl]; + $casconfig = Configuration::loadFromArray($moduleConfig); $params = [ - 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', + 'service' => $serviceUrl, 'gateway' => true, ]; $loginRequest = Request::create( @@ -355,15 +376,16 @@ public function testGatewayPassiveDisabledRedirectsWithoutParams(): void $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); + // Execute $response = $controllerMock->login($loginRequest, ...$params); + // Validate redirect with original service URL and no CAS parameters appended $this->assertInstanceOf(RunnableResponse::class, $response); $callable = (array)$response->getCallable(); $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); - // Service URL unchanged and NO CAS params appended $arguments = $response->getArguments(); - $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]); + $this->assertEquals($serviceUrl, $arguments[0]); $this->assertSame([], $arguments[1] ?? []); } @@ -403,7 +425,7 @@ public function testGatewayPassiveEnabledPerformsPassiveAttempt(): void $callable = (array)$response->getCallable(); $this->assertEquals('login', $callable[1] ?? ''); - // Verify isPassive=true, ForceAuthn=false, ReturnTo contains gatewayTried=1 + // Verify isPassive=true, ForceAuthn=false $arguments = $response->getArguments(); $actualLoginParams = $arguments[0] ?? []; $this->assertArrayHasKey('ForceAuthn', $actualLoginParams); @@ -412,50 +434,6 @@ public function testGatewayPassiveEnabledPerformsPassiveAttempt(): void $this->assertTrue($actualLoginParams['isPassive']); $this->assertArrayHasKey('ReturnTo', $actualLoginParams); $this->assertIsString($actualLoginParams['ReturnTo']); - $this->assertStringContainsString('gatewayTried=1', $actualLoginParams['ReturnTo']); - } - - public function testGatewayPassiveEnabledSecondPassRedirectsWithoutParams(): void - { - // enable_passive_mode enabled - $moduleConfig = $this->moduleConfig; - $moduleConfig['enable_passive_mode'] = true; - $casconfig = Configuration::loadFromArray($moduleConfig); - - // Include gatewayTried in the Request only - $requestParams = [ - 'service' => 'https://example.com/ssp/module.php/cas/linkback.php', - 'gateway' => true, - 'gatewayTried' => '1', // simulate second pass after passive attempt - ]; - $loginRequest = Request::create( - uri: Module::getModuleURL('casserver/login'), - parameters: $requestParams, - ); - - $controllerMock = $this->getMockBuilder(LoginController::class) - ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) - ->onlyMethods(['getSession']) - ->getMock(); - - $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); - - $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); - $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); - - // Do not pass 'gatewayTried' as named argument - $callParams = $requestParams; - unset($callParams['gatewayTried']); - - $response = $controllerMock->login($loginRequest, ...$callParams); - - $this->assertInstanceOf(RunnableResponse::class, $response); - $callable = (array)$response->getCallable(); - $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); - - $arguments = $response->getArguments(); - $this->assertEquals('https://example.com/ssp/module.php/cas/linkback.php', $arguments[0]); - $this->assertSame([], $arguments[1] ?? []); } public function testGatewayNoServicePassiveDisabledFallsBackToInteractive(): void @@ -572,6 +550,7 @@ public function testAuthenticatedPostSubmitsViaPostWithTicket(): void ], ]); + /** @psalm-suppress InvalidArgument */ $response = $controllerMock->login($loginRequest, ...$requestParams); $this->assertInstanceOf(RunnableResponse::class, $response); @@ -585,54 +564,4 @@ public function testAuthenticatedPostSubmitsViaPostWithTicket(): void $this->assertArrayHasKey('ticket', $params); $this->assertStringStartsWith('ST-', $params['ticket']); } - - public function testGatewayRedirectPreservesServiceQueryWithoutTicket(): void - { - // Passive disabled to trigger direct redirect with no ticket - $moduleConfig = $this->moduleConfig; - $moduleConfig['enable_passive_mode'] = false; - - // Service URL with existing query parameters - $serviceWithQuery = 'https://example.com/ssp/module.php/cas/linkback.php?foo=1&bar=2'; - // Ensure the exact service URL (including its query string) is allowed - $moduleConfig['legal_service_urls'] = [$serviceWithQuery]; - - $casconfig = Configuration::loadFromArray($moduleConfig); - - $params = [ - 'service' => $serviceWithQuery, - 'gateway' => true, - ]; - - $loginRequest = Request::create( - uri: Module::getModuleURL('casserver/login'), - parameters: $params, - ); - - $controllerMock = $this->getMockBuilder(LoginController::class) - ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) - ->onlyMethods(['getSession']) - ->getMock(); - - // Unauthenticated so gateway path is exercised - $this->authSimpleMock->expects($this->atLeastOnce())->method('isAuthenticated')->willReturn(false); - - // Session used to build ReturnTo - $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); - $this->sessionMock->expects($this->once())->method('getSessionId')->willReturn(session_create_id()); - - // Execute - $response = $controllerMock->login($loginRequest, ...$params); - - // Validate redirect with original query intact and no CAS parameters appended - $this->assertInstanceOf(RunnableResponse::class, $response); - $callable = (array)$response->getCallable(); - $this->assertEquals('redirectTrustedURL', $callable[1] ?? ''); - - $arguments = $response->getArguments(); - // URL should be exactly the original service URL including its query string - $this->assertEquals($serviceWithQuery, $arguments[0]); - // No additional parameters (e.g., no ticket) should be appended by CAS - $this->assertSame([], $arguments[1] ?? []); - } } From 3d953b1cc8a6b2a1df64a11a7c15af7c72bbb55d Mon Sep 17 00:00:00 2001 From: Patrick Radtke Date: Thu, 20 Nov 2025 20:39:31 -0800 Subject: [PATCH 4/4] Updates for docker testing; phpstan; consolidate gateway methods --- config/module_casserver.php.dist | 3 +- docker/ssp/authsources.php | 1 + docker/ssp/module_casserver.php | 2 +- src/Controller/LoginController.php | 54 ++++---------------- tests/src/Controller/LoginControllerTest.php | 2 +- 5 files changed, 16 insertions(+), 46 deletions(-) diff --git a/config/module_casserver.php.dist b/config/module_casserver.php.dist index 7da85e39..25e50d47 100644 --- a/config/module_casserver.php.dist +++ b/config/module_casserver.php.dist @@ -117,7 +117,8 @@ $config = [ 'proxy_granting_ticket_expire_time' => 600, // how many seconds proxy tickets are valid for, defaults to 5 'proxy_ticket_expire_time' => 5, - // OPTIONAL, enable CAS passive mode, defaults to false + // OPTIONAL, if `gateway=true` is requested and user has no session, invoke the authsource with `isPassive=true`. + // defaults to false //'enable_passive_mode' => true, // If query param debugMode=true is sent to the login endpoint then print cas ticket xml. Default false diff --git a/docker/ssp/authsources.php b/docker/ssp/authsources.php index 134815f8..64dea51a 100644 --- a/docker/ssp/authsources.php +++ b/docker/ssp/authsources.php @@ -14,6 +14,7 @@ 'users' => [ 'student:studentpass' => [ 'uid' => ['student'], + 'cn' => ['Firsty Lasty'], 'eduPersonAffiliation' => ['member', 'student'], 'eduPersonNickname' => 'Sir_Nickname', 'displayName' => 'Some User', diff --git a/docker/ssp/module_casserver.php b/docker/ssp/module_casserver.php index 4f5867d5..290cb4b0 100644 --- a/docker/ssp/module_casserver.php +++ b/docker/ssp/module_casserver.php @@ -115,7 +115,7 @@ url query parameter to CAS logout mandatory for obvious reasons.*/ // how many seconds service tickets are valid for, defaults to 5 - 'service_ticket_expire_time' => 5, + 'service_ticket_expire_time' => 60, // how many seconds proxy granting tickets are valid for at most, defaults to 3600 'proxy_granting_ticket_expire_time' => 600, //how many seconds proxy tickets are valid for, defaults to 5 diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 0f1ee651..a16562ec 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -165,24 +165,18 @@ public function login( $returnToUrl = $this->getReturnUrl($request, $sessionTicket); // renew=true and gateway=true are incompatible → prefer interactive login (disable passive) - // Protocol: gateway and renew are incompatible; behavior is undefined if both are set. - // OPTIONAL (implementation policy): Prefer renew (interactive, non-passive) by disabling gateway. - // OPTIONAL alternative: Reject with 400 to signal incompatible parameters. if ($gateway && $forceAuthn) { $gateway = false; } - // Handle passive authentication + // Handle passive authentication if service url defined // Protocol (gateway set): CAS MUST NOT prompt for credentials during this branch. - if ($gateway && !$this->authSource->isAuthenticated() && !$requestForceAuthenticate) { - $response = $this->handleUnauthenticatedGateway( + if ($serviceUrl && $gateway && !$this->authSource->isAuthenticated() && !$requestForceAuthenticate) { + return $this->handleUnauthenticatedGateway( $serviceUrl, $entityId, $returnToUrl, ); - if ($response !== null) { - return $response; - } } // Handle interactive authentication @@ -254,9 +248,6 @@ public function login( } // User has SSO or non-interactive auth succeeded → redirect/POST to service WITH a ticket - // Protocol: With gateway and a successful non-interactive auth (or existing SSO), CAS MAY redirect to the - // service and append a ticket. - // Protocol: CAS MAY interpose an advisory page indicating that authentication took place. $ticketName = $this->calculateTicketName($service); $this->postAuthUrlParameters[$ticketName] = $serviceTicket['id']; @@ -502,20 +493,19 @@ private function handleInteractiveAuthenticate( * - null to indicate: proceed with interactive login (non-passive). */ private function handleUnauthenticatedGateway( - ?string $serviceUrl, + string $serviceUrl, ?string $entityId, string $returnToUrl, - ): ?RunnableResponse { + ): RunnableResponse { $passiveAllowed = $this->casConfig->getOptionalBoolean('enable_passive_mode', false); - // Passive mode is not enabled by configuration. - // Protocol: If non-interactive auth cannot be established: - // - If service is present, CAS MUST redirect to the service URL WITHOUT a ticket parameter. - // - If service is absent, behavior is undefined; it is RECOMMENDED - // to request credentials as if neither parameter was specified. + // Passive mode is not enabled by configuration + // CAS MUST redirect to the service URL WITHOUT a ticket parameter. if (!$passiveAllowed) { - // Passive attempt already performed and still not authenticated. - return $this->gatewayFallback($serviceUrl); + return new RunnableResponse( + [$this->httpUtils, 'redirectTrustedURL'], + [$serviceUrl, []], + ); } // Passive mode enabled: attempt a passive (non-interactive) authentication. @@ -566,26 +556,4 @@ private function handleAuthenticate( [$params], ); } - - /** - * Gateway fallback per CAS gateway semantics: - * - Protocol (MUST): If a service is provided and non-interactive auth cannot be established, - * redirect to the service WITHOUT any CAS parameters (no "ticket"). - * - Protocol (Undefined, RECOMMENDED): If no service is provided, proceed with interactive login - * (request credentials). - * @param string|null $serviceUrl - * @return RunnableResponse|null - */ - private function gatewayFallback(?string $serviceUrl): ?RunnableResponse - { - if ($serviceUrl !== null) { - // MUST: Redirect to service WITHOUT a "ticket" parameter (and without other CAS params). - return new RunnableResponse( - [$this->httpUtils, 'redirectTrustedURL'], - [$serviceUrl, []], - ); - } - // RECOMMENDED: No service specified; proceed with interactive login as if neither parameter was specified. - return null; - } } diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php index 4089ba19..a9f6a60e 100644 --- a/tests/src/Controller/LoginControllerTest.php +++ b/tests/src/Controller/LoginControllerTest.php @@ -331,7 +331,7 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b /** * @return array */ - public function serviceUrlsProvider(): array + public static function serviceUrlsProvider(): array { return [ ['https://example.com/ssp/module.php/cas/linkback.php'],