diff --git a/config/module_casserver.php.dist b/config/module_casserver.php.dist index 25e50d47..b4dd55aa 100644 --- a/config/module_casserver.php.dist +++ b/config/module_casserver.php.dist @@ -97,6 +97,19 @@ $config = [ // Additional authproc filter ], + /** + * Optional entity ID identifying this CAS server to the authproc filters above, exposed to them + * as $state['Source']['entityid']. Filters that key off the authenticating entity (federation + * logging such as F-ticks, per-entity authorization, audit trails) need it. The requesting CAS + * service URL is made available to the same filters as $state['Destination']['entityid']. + * + * NOTE: this is unrelated to the ?entityId= query parameter of the login endpoint, which selects + * which upstream SAML IdP the authsource should authenticate against. + * + * Defaults to the SimpleSAMLphp base URL. + */ + //'idp_entity_id' => 'https://login.example.org/idp/metadata.php', + 'base64attributes' => true, // base64 encode transferred attributes, defaults to false /** diff --git a/docs/ChangeLog.md b/docs/ChangeLog.md index ea31eae0..793134e2 100644 --- a/docs/ChangeLog.md +++ b/docs/ChangeLog.md @@ -8,6 +8,12 @@ Unreleased * Minimum supported simplesamlphp version bumped to 1.17 * debugMode option to display cas ticket xml * Allow per service overriding of configuration options for www/login +* Pass the requesting cas service url to the authproc filters as the sp + entity id, which was previously always an empty string +* Added an 'idp_entity_id' option, passed to the authproc filters as the + idp entity id and defaulting to the simplesamlphp base url +* Reject a resumed authproc state whose service url differs from the one + the filters ran for 2018-07-20 Bjorn Rohde Jensen diff --git a/src/Cas/AttributeExtractor.php b/src/Cas/AttributeExtractor.php index c8e3c1d9..76266617 100644 --- a/src/Cas/AttributeExtractor.php +++ b/src/Cas/AttributeExtractor.php @@ -10,6 +10,7 @@ use SimpleSAML\Error\NoState; use SimpleSAML\Module; use SimpleSAML\Module\casserver\Cas\Factories\ProcessingChainFactory; +use SimpleSAML\Utils; /** * Extract the user and any mapped attributes from the AuthSource attributes @@ -19,6 +20,9 @@ class AttributeExtractor /** @var \SimpleSAML\Auth\State */ private State $authState; + /** @var \SimpleSAML\Utils\HTTP */ + protected Utils\HTTP $httpUtils; + /** * ID of the Authentication Source used during authn. */ @@ -28,8 +32,11 @@ class AttributeExtractor public function __construct( protected Configuration $casconfig, protected ProcessingChainFactory $processingChainFactory, + // Facilitate testing + ?Utils\HTTP $httpUtils = null, ) { $this->authState = new State(); + $this->httpUtils = $httpUtils ?? new Utils\HTTP(); } @@ -95,8 +102,9 @@ public function extractUserAndAttributes(?array $state): array /** * Run authproc filters with the processing chain * Creating the ProcessingChain require metadata. - * - For the idp metadata use the OIDC issuer as the entityId (and the authprocs from the main config file) - * - For the sp metadata use the client id as the entityId (and don’t set authprocs). + * - For the idp metadata use the configured casserver entity ID as the entityId (and the authprocs + * from the casserver config file) + * - For the sp metadata use the CAS service URL as the entityId (and don’t set authprocs). * * @param array $state * @@ -107,14 +115,20 @@ public function extractUserAndAttributes(?array $state): array protected function runAuthProcs(array &$state): void { $filters = $this->casconfig->getOptionalArray('authproc', []); - $idpMetadata = [ - 'entityid' => $state['Source']['entityid'] ?? '', - // ProcessChain needs to know the list of authproc filters we defined in module_oidc configuration - 'authproc' => $filters, - ]; - $spMetadata = [ - 'entityid' => $state['Destination']['entityid'] ?? '', - ]; + + // Preserve any other metadata the caller may have seeded, and only take over the keys we own. + $idpMetadata = \is_array($state['Source'] ?? null) ? $state['Source'] : []; + $spMetadata = \is_array($state['Destination'] ?? null) ? $state['Destination'] : []; + + $idpMetadata['entityid'] = $this->resolveIdpEntityId($idpMetadata['entityid'] ?? null); + // ProcessChain needs to know the list of authproc filters we defined in the casserver configuration + $idpMetadata['authproc'] = $filters; + // The CAS service URL is the closest analogue to an SP entity ID. It is seeded into the state by + // the login controller, which has already validated it against the legal service URLs. + $spMetadata['entityid'] = (string)($spMetadata['entityid'] ?? ''); + // The authproc filters are owned by the casserver configuration alone. Anything the caller may + // have seeded here is a control key rather than metadata, and must not add filters to the chain. + unset($spMetadata['authproc']); // Get the ReturnTo from the state or fallback to the login page $state['ReturnURL'] = $state['ReturnTo'] ?? Module::getModuleURL('casserver/login.php'); @@ -125,6 +139,34 @@ protected function runAuthProcs(array &$state): void } + /** + * Resolve the entity ID that casserver presents to the authproc filters as the authenticating IdP. + * + * casserver has no entity ID of its own, so the value is resolved in the following order: + * whatever the state already carries, then the 'idp_entity_id' configuration option, and finally + * the SimpleSAMLphp base URL. Without this, filters that key off the authenticating entity (for + * example federation logging or per-entity authorization) would silently observe an empty string. + * + * @param mixed $stateEntityId The entity ID already present in the state, if any. + * + * @return string + * @throws \Exception + */ + private function resolveIdpEntityId(mixed $stateEntityId): string + { + if (\is_string($stateEntityId) && $stateEntityId !== '') { + return $stateEntityId; + } + + $configuredEntityId = $this->casconfig->getOptionalString('idp_entity_id', null); + if (!empty($configuredEntityId)) { + return $configuredEntityId; + } + + return $this->httpUtils->getBaseURL(); + } + + /** * This is a wrapper around Auth/State::loadState that facilitates testing by * hiding the static method diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 92f71e4c..0d13299c 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -218,6 +218,24 @@ public function login( // Get the state. $state = $this->getState(); $state['ReturnTo'] = $returnToUrl; + // The CAS service is the closest analogue to an SP entity ID, so expose it to the authproc + // filters the same way a SAML IdP would. It has already been validated against the legal + // service URLs by handleServiceConfiguration(). + $stateServiceUrl = $state['Destination']['entityid'] ?? null; + if (!empty($stateServiceUrl) && !empty($serviceUrl) && $stateServiceUrl !== $serviceUrl) { + // A state resumed from an authproc filter is bound to the service those filters ran for. + // They are not run again on this path, so honouring a different service here would let the + // decisions taken for one service be reused for another. + $message = 'Service parameter provided to CAS server does not match the service the ' + . 'authentication processing filters ran for: [service] = ' . var_export($serviceUrl, true); + Logger::debug('casserver:' . $message); + + throw new RuntimeException($message); + } + // Any value already in the state wins, so a resumed state keeps what the first pass established. + if (!empty($serviceUrl) && empty($stateServiceUrl)) { + $state['Destination']['entityid'] = $serviceUrl; + } if ($this->authProcId !== null) { $state[ProcessingChain::AUTHPARAM] = $this->authProcId; } diff --git a/tests/src/AttributeExtractorTest.php b/tests/src/AttributeExtractorTest.php index 315feff6..d7a23ae3 100644 --- a/tests/src/AttributeExtractorTest.php +++ b/tests/src/AttributeExtractorTest.php @@ -8,9 +8,21 @@ use SimpleSAML\Configuration; use SimpleSAML\Module\casserver\Cas\AttributeExtractor; use SimpleSAML\Module\casserver\Cas\Factories\ProcessingChainFactory; +use SimpleSAML\Utils; class AttributeExtractorTest extends TestCase { + /** + * An authproc filter that records the IdP and SP entity IDs the processing chain was built with, + * so that the test can assert on what a real filter would observe. + */ + private const array ENTITY_ID_OBSERVER = [ + 'class' => 'core:PHP', + 'code' => '$attributes["observedIdpEntityId"] = [$state["Source"]["entityid"] ?? "MISSING"];' + . '$attributes["observedSpEntityId"] = [$state["Destination"]["entityid"] ?? "MISSING"];', + ]; + + /** * Confirm behavior of a default configuration */ @@ -162,4 +174,180 @@ public function testAuthprocConfig(): void $this->assertEquals('testuser@example.com', $result['user']); $this->assertEquals($expectedAttributes, $result['attributes']); } + + + /** + * The CAS service URL seeded into the state is handed to the authproc filters as the SP entity ID, + * and the IdP entity ID falls back to the SimpleSAMLphp base URL when not configured. + */ + public function testAuthprocReceivesServiceUrlAsSpEntityId(): void + { + putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config/'); + $serviceUrl = 'https://myservice.example.com/cas/callback'; + + $casConfig = [ + 'authproc' => [self::ENTITY_ID_OBSERVER], + ]; + + $state = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + ], + // Seeded by LoginController from the validated ?service= / ?TARGET= parameter + 'Destination' => ['entityid' => $serviceUrl], + ]; + + $loadedConfig = Configuration::loadFromArray($casConfig); + $attributeExtractor = new AttributeExtractor( + $loadedConfig, + new ProcessingChainFactory($loadedConfig), + ); + + $result = $attributeExtractor->extractUserAndAttributes($state); + + $this->assertEquals([$serviceUrl], $result['attributes']['observedSpEntityId']); + + // Regression guard: the whole failure mode is that this silently becomes an empty string. + $observedIdpEntityId = $result['attributes']['observedIdpEntityId'][0]; + $this->assertIsString($observedIdpEntityId); + $this->assertNotEmpty($observedIdpEntityId); + $this->assertEquals((new Utils\HTTP())->getBaseURL(), $observedIdpEntityId); + } + + + /** + * The configured idp_entity_id is handed to the authproc filters as the IdP entity ID. + */ + public function testAuthprocReceivesConfiguredIdpEntityId(): void + { + putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config/'); + $idpEntityId = 'https://login.example.org/idp/metadata.php'; + + $casConfig = [ + 'idp_entity_id' => $idpEntityId, + 'authproc' => [self::ENTITY_ID_OBSERVER], + ]; + + $state = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + ], + ]; + + $loadedConfig = Configuration::loadFromArray($casConfig); + $attributeExtractor = new AttributeExtractor( + $loadedConfig, + new ProcessingChainFactory($loadedConfig), + ); + + $result = $attributeExtractor->extractUserAndAttributes($state); + + $this->assertEquals([$idpEntityId], $result['attributes']['observedIdpEntityId']); + } + + + /** + * Entity IDs already present in the state take precedence over the configuration, so that a state + * resumed from an authproc filter keeps what the first pass established. + */ + public function testAuthprocPreservesPreExistingEntityIds(): void + { + putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config/'); + $stateIdpEntityId = 'https://from-state.example.org/idp/metadata.php'; + $stateSpEntityId = 'https://from-state.example.com/cas/callback'; + + $casConfig = [ + 'idp_entity_id' => 'https://from-config.example.org/idp/metadata.php', + 'authproc' => [self::ENTITY_ID_OBSERVER], + ]; + + $state = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + ], + 'Source' => ['entityid' => $stateIdpEntityId], + 'Destination' => ['entityid' => $stateSpEntityId], + ]; + + $loadedConfig = Configuration::loadFromArray($casConfig); + $attributeExtractor = new AttributeExtractor( + $loadedConfig, + new ProcessingChainFactory($loadedConfig), + ); + + $result = $attributeExtractor->extractUserAndAttributes($state); + + $this->assertEquals([$stateIdpEntityId], $result['attributes']['observedIdpEntityId']); + $this->assertEquals([$stateSpEntityId], $result['attributes']['observedSpEntityId']); + } + + + /** + * Only the casserver configuration may contribute authproc filters. Filters seeded into the SP + * metadata by a caller must not be executed by the processing chain. + */ + public function testAuthprocIgnoresFiltersSeededIntoSpMetadata(): void + { + putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config/'); + $casConfig = [ + 'authproc' => [self::ENTITY_ID_OBSERVER], + ]; + + $state = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + ], + 'Destination' => [ + 'entityid' => 'https://myservice.example.com/cas/callback', + // A control key, not metadata. It must be dropped rather than added to the chain. + 'authproc' => [ + [ + 'class' => 'core:PHP', + 'code' => '$attributes["filterFromSpMetadata"] = ["should not run"];', + ], + ], + ], + ]; + + $loadedConfig = Configuration::loadFromArray($casConfig); + $attributeExtractor = new AttributeExtractor( + $loadedConfig, + new ProcessingChainFactory($loadedConfig), + ); + + $result = $attributeExtractor->extractUserAndAttributes($state); + + $this->assertArrayNotHasKey('filterFromSpMetadata', $result['attributes']); + // The configured filters still ran. + $this->assertArrayHasKey('observedSpEntityId', $result['attributes']); + } + + + /** + * Without a service URL the chain still runs, and the SP entity ID stays empty. + */ + public function testAuthprocWithoutServiceUrl(): void + { + putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config/'); + $casConfig = [ + 'idp_entity_id' => 'https://login.example.org/idp/metadata.php', + 'authproc' => [self::ENTITY_ID_OBSERVER], + ]; + + $state = [ + 'Attributes' => [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + ], + ]; + + $loadedConfig = Configuration::loadFromArray($casConfig); + $attributeExtractor = new AttributeExtractor( + $loadedConfig, + new ProcessingChainFactory($loadedConfig), + ); + + $result = $attributeExtractor->extractUserAndAttributes($state); + + $this->assertEquals([''], $result['attributes']['observedSpEntityId']); + } } diff --git a/tests/src/Controller/LoginControllerTest.php b/tests/src/Controller/LoginControllerTest.php index 6e647e8c..a0eafb2f 100644 --- a/tests/src/Controller/LoginControllerTest.php +++ b/tests/src/Controller/LoginControllerTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use SimpleSAML\Auth\ProcessingChain; use SimpleSAML\Auth\Simple; use SimpleSAML\Configuration; use SimpleSAML\HTTP\RunnableResponse; @@ -359,6 +360,164 @@ public function testValidServiceUrl(string $serviceParam, string $redirectURL, b } + /** + * The authproc filters must observe the requesting CAS service as the SP entity ID and the + * configured idp_entity_id as the IdP entity ID. Both were previously always empty strings. + * + * @throws \Exception + */ + public function testAuthprocReceivesServiceAndIdpEntityIds(): void + { + $serviceUrl = 'https://example.com/ssp/module.php/cas/linkback.php'; + $idpEntityId = 'https://login.example.org/idp/metadata.php'; + + $state['Attributes'] = [ + 'eduPersonPrincipalName' => ['testuser@example.com'], + 'Expire' => 9999999999, + ]; + + $moduleConfig = $this->moduleConfig; + $moduleConfig['idp_entity_id'] = $idpEntityId; + // Record what the processing chain was built with, so we can assert on it via the ticket. + $moduleConfig['authproc'] = [ + [ + 'class' => 'core:PHP', + 'code' => '$attributes["observedIdpEntityId"] = [$state["Source"]["entityid"] ?? "MISSING"];' + . '$attributes["observedSpEntityId"] = [$state["Destination"]["entityid"] ?? "MISSING"];', + ], + ]; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession']) + ->getMock(); + + $sessionId = session_create_id(); + $this->sessionMock->expects($this->exactly(2))->method('getSessionId')->willReturn($sessionId); + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $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($state); + + $queryParameters = ['service' => $serviceUrl]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $queryParameters, + ); + + $response = $this->callLogin($controllerMock, $loginRequest, $queryParameters); + $this->assertInstanceOf(RunnableResponse::class, $response); + + $arguments = $response->getArguments(); + $ticketId = array_values($arguments[1])[0] ?? null; + $this->assertIsString($ticketId); + + $ticket = $controllerMock->getTicketStore()->getTicket($ticketId); + $this->assertIsArray($ticket); + + $this->assertEquals([$serviceUrl], $ticket['attributes']['observedSpEntityId'] ?? null); + $this->assertEquals([$idpEntityId], $ticket['attributes']['observedIdpEntityId'] ?? null); + } + + + /** + * A state resumed from an authproc filter is bound to the service those filters ran for. The + * filters are not run again on this path, so a different service must be rejected rather than + * silently inheriting the previous service's authproc decisions. + * + * @throws \Exception + */ + public function testResumedAuthprocStateRejectsDifferentService(): void + { + $filteredServiceUrl = 'https://example.com/ssp/module.php/cas/linkback.php'; + $requestedServiceUrl = 'https://example.org/ssp/module.php/cas/linkback.php'; + + $moduleConfig = $this->moduleConfig; + $moduleConfig['legal_service_urls'] = [$filteredServiceUrl, $requestedServiceUrl]; + $casconfig = Configuration::loadFromArray($moduleConfig); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession', 'getState']) + ->getMock(); + + // The state the processing chain saved during the first pass, for $filteredServiceUrl. + $controllerMock->method('getState')->willReturn([ + 'Attributes' => ['eduPersonPrincipalName' => ['testuser@example.com']], + 'Destination' => ['entityid' => $filteredServiceUrl], + ]); + + $sessionId = session_create_id(); + $this->sessionMock->expects($this->exactly(2))->method('getSessionId')->willReturn($sessionId); + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->authSimpleMock->expects($this->any())->method('isAuthenticated')->willReturn(true); + $this->authSimpleMock->expects($this->once())->method('getAuthData')->with('Expire')->willReturn(9999999999); + + // The user comes back from the authproc filter having swapped in a different legal service. + $queryParameters = [ + 'service' => $requestedServiceUrl, + ProcessingChain::AUTHPARAM => 'someAuthProcId', + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $queryParameters, + ); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage( + 'Service parameter provided to CAS server does not match the service the authentication ' + . "processing filters ran for: [service] = '" . $requestedServiceUrl . "'", + ); + + $this->callLogin($controllerMock, $loginRequest, $queryParameters); + } + + + /** + * Resuming an authproc state with the service it was created for still issues a ticket. + * + * @throws \Exception + */ + public function testResumedAuthprocStateAcceptsMatchingService(): void + { + $serviceUrl = 'https://example.com/ssp/module.php/cas/linkback.php'; + $casconfig = Configuration::loadFromArray($this->moduleConfig); + + $controllerMock = $this->getMockBuilder(LoginController::class) + ->setConstructorArgs([$this->sspConfig, $casconfig, $this->authSimpleMock, $this->httpUtils]) + ->onlyMethods(['getSession', 'getState']) + ->getMock(); + + $controllerMock->method('getState')->willReturn([ + 'Attributes' => ['eduPersonPrincipalName' => ['testuser@example.com']], + 'Destination' => ['entityid' => $serviceUrl], + ]); + + $sessionId = session_create_id(); + $this->sessionMock->expects($this->exactly(2))->method('getSessionId')->willReturn($sessionId); + $controllerMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->authSimpleMock->expects($this->any())->method('isAuthenticated')->willReturn(true); + $this->authSimpleMock->expects($this->once())->method('getAuthData')->with('Expire')->willReturn(9999999999); + + $queryParameters = [ + 'service' => $serviceUrl, + ProcessingChain::AUTHPARAM => 'someAuthProcId', + ]; + $loginRequest = Request::create( + uri: Module::getModuleURL('casserver/login'), + parameters: $queryParameters, + ); + + $response = $this->callLogin($controllerMock, $loginRequest, $queryParameters); + + $this->assertInstanceOf(RunnableResponse::class, $response); + $arguments = $response->getArguments(); + $this->assertEquals($serviceUrl, $arguments[0]); + $this->assertStringStartsWith('ST-', array_values($arguments[1])[0] ?? ''); + } + + /** * @return array */