diff --git a/modules/saml/lib/Auth/Process/FilterScopes.php b/modules/saml/lib/Auth/Process/FilterScopes.php index 9748a9c614208d0912d364d42ea10ad4061f487f..f6c0a8f92ba44b20f4428fd53258f6409509fb65 100644 --- a/modules/saml/lib/Auth/Process/FilterScopes.php +++ b/modules/saml/lib/Auth/Process/FilterScopes.php @@ -25,7 +25,6 @@ class FilterScopes extends ProcessingFilter 'eduPersonPrincipalName' ]; - /** * Constructor for the processing filter. * @@ -41,7 +40,6 @@ class FilterScopes extends ProcessingFilter } } - /** * This method applies the filter, removing any values * @@ -50,53 +48,45 @@ class FilterScopes extends ProcessingFilter public function process(array &$request): void { $src = $request['Source']; - if (!count($this->scopedAttributes)) { - // paranoia, should never happen - Logger::warning('No scoped attributes configured.'); - return; - } + $validScopes = []; + $host = ''; if (array_key_exists('scope', $src) && is_array($src['scope']) && !empty($src['scope'])) { $validScopes = $src['scope']; + } else { + $ep = Utils\Config\Metadata::getDefaultEndpoint($request['Source']['SingleSignOnService']); + $host = parse_url($ep['Location'], PHP_URL_HOST) ?? ''; } - $ep = Utils\Config\Metadata::getDefaultEndpoint($request['Source']['SingleSignOnService']); - if ($ep !== null) { - foreach ($this->scopedAttributes as $attribute) { - if (!isset($request['Attributes'][$attribute])) { - continue; - } + foreach ($this->scopedAttributes as $attribute) { + if (!isset($request['Attributes'][$attribute])) { + continue; + } - $values = $request['Attributes'][$attribute]; - $newValues = []; - foreach ($values as $value) { - $loc = $ep['Location']; - $host = parse_url($loc, PHP_URL_HOST); - if ($host === null) { - $host = ''; - } - $value_a = explode('@', $value, 2); - if (count($value_a) < 2) { - $newValues[] = $value; - continue; // there's no scope - } - $scope = $value_a[1]; - if (in_array($scope, $validScopes, true)) { - $newValues[] = $value; - } elseif (strpos($host, $scope) === strlen($host) - strlen($scope)) { - $newValues[] = $value; - } else { - Logger::warning("Removing value '$value' for attribute '$attribute'. Undeclared scope."); - } + $values = $request['Attributes'][$attribute]; + $newValues = []; + foreach ($values as $value) { + $value_a = explode('@', $value, 2); + if (count($value_a) < 2) { + $newValues[] = $value; + continue; // there's no scope } - - if (empty($newValues)) { - Logger::warning("No suitable values for attribute '$attribute', removing it."); - unset($request['Attributes'][$attribute]); // remove empty attributes + $scope = $value_a[1]; + if (in_array($scope, $validScopes, true)) { + $newValues[] = $value; + } elseif (strpos($host, $scope) === strlen($host) - strlen($scope)) { + $newValues[] = $value; } else { - $request['Attributes'][$attribute] = $newValues; + Logger::warning("Removing value '$value' for attribute '$attribute'. Undeclared scope."); } } + + if (empty($newValues)) { + Logger::warning("No suitable values for attribute '$attribute', removing it."); + unset($request['Attributes'][$attribute]); // remove empty attributes + } else { + $request['Attributes'][$attribute] = $newValues; + } } } } diff --git a/tests/modules/saml/lib/Auth/Process/FilterScopesTest.php b/tests/modules/saml/lib/Auth/Process/FilterScopesTest.php index 724eebe57ac0aa6927a05b4581306d7e2bed4f7a..2730453c3bb554b9f87f97dbe12e3d086a7a8df7 100644 --- a/tests/modules/saml/lib/Auth/Process/FilterScopesTest.php +++ b/tests/modules/saml/lib/Auth/Process/FilterScopesTest.php @@ -68,19 +68,12 @@ class FilterScopesTest extends TestCase $result = $this->processFilter($config, $request); $this->assertEquals($request['Attributes'], $result['Attributes']); - // test implicit scope - $request['Attributes'] = [ - 'eduPersonPrincipalName' => ['jdoe@example.org'], - ]; - $result = $this->processFilter($config, $request); - $this->assertEquals($request['Attributes'], $result['Attributes']); - // test alternative attributes $config['attributes'] = [ 'mail', ]; $request['Attributes'] = [ - 'mail' => ['john.doe@example.org'], + 'mail' => ['john.doe@example.com'], ]; $result = $this->processFilter($config, $request); $this->assertEquals($request['Attributes'], $result['Attributes']); @@ -91,6 +84,35 @@ class FilterScopesTest extends TestCase $this->assertEquals($request['Attributes'], $result['Attributes']); } + /** + * Test implict scope matching on IdP hostname + */ + public function testImplicitScopes(): void + { + $config = []; + $request = [ + 'Source' => [ + 'SingleSignOnService' => [ + [ + 'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', + 'Location' => 'https://example.org/saml2/idp/SSOService.php', + ], + ], + ], + 'Attributes' => [ + 'eduPersonPrincipalName' => ['jdoe@example.org'], + ], + ]; + + $result = $this->processFilter($config, $request); + $this->assertEquals($request['Attributes'], $result['Attributes']); + + $request['Attributes'] = [ + 'eduPersonPrincipalName' => ['jdoe@example.com'], + ]; + $result = $this->processFilter($config, $request); + $this->assertEquals([], $result['Attributes']); + } /** * Test invalid scopes. @@ -137,5 +159,64 @@ class FilterScopesTest extends TestCase ]; $result = $this->processFilter($config, $request); $this->assertEquals($request['Attributes'], $result['Attributes']); + + } + + /** + * Test that implicit matching is not done when explicit scopes present + */ + public function testNoImplicitMatchingWhenExplicitScopes(): void + { + // test declared scopes + $config = []; + $request = [ + 'Source' => [ + 'SingleSignOnService' => [ + [ + 'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', + 'Location' => 'https://example.org/saml2/idp/SSOService.php', + ], + ], + 'scope' => [ + 'example.com', + 'example.net', + ], + ], + 'Attributes' => [ + 'eduPersonPrincipalName' => ['jdoe@example.org'], + ], + ]; + $result = $this->processFilter($config, $request); + $this->assertEquals([], $result['Attributes']); + } + + /** + * Test that the scope is considered to be the part after the first @ sign + */ + public function testAttributeValueMultipleAt(): void + { + $config = []; + $request = [ + 'Source' => [ + 'SingleSignOnService' => [ + [ + 'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', + 'Location' => 'https://example.org/saml2/idp/SSOService.php', + ], + ], + 'scope' => [ + 'example.com', + ], + ], + 'Attributes' => [ + 'eduPersonPrincipalName' => ['jdoe@gmail.com@example.com'], + ], + ]; + $result = $this->processFilter($config, $request); + $this->assertEquals([], $result['Attributes']); + + $request['Source']['scope'] = ['gmail.com@example.com']; + $result = $this->processFilter($config, $request); + $this->assertEquals($request['Attributes'], $result['Attributes']); } }