diff --git a/modules/core/docs/authproc_attributelimit.md b/modules/core/docs/authproc_attributelimit.md index 5b640fd82144a100b41688dc0c545e22ad0b40c6..73c4406efd4875c5c72ea8503ea32adcc3036c45 100644 --- a/modules/core/docs/authproc_attributelimit.md +++ b/modules/core/docs/authproc_attributelimit.md @@ -32,6 +32,41 @@ Allow `eduPersonTargetedID` and `eduPersonAffiliation` by default, but allow the ), ), +Only allow specific values for an attribute. + + 'authproc' => array( + 50 => array( + 'class' => 'core:AttributeLimit', + 'eduPersonEntitlement' => array('urn:x-surfnet:surf.nl:surfdrive:quota:100') + ), + ), + +Only allow specific values for an attribute ignoring case. + + 'authproc' => array( + 50 => array( + 'class' => 'core:AttributeLimit', + 'eduPersonEntitlement' => array( + 'ignoreCase' => true, + 'URN:x-surfnet:surf.nl:SURFDRIVE:quota:100' + ) + ), + ), + +Only allow specific values for an attribute that match a regex pattern + + 'authproc' => array( + 50 => array( + 'class' => 'core:AttributeLimit', + 'eduPersonEntitlement' => array( + 'regex' => true, + '/^urn:x-surfnet:surf/', + '/^urn:x-IGNORE_Case/i', + ) + ), + ), + + Don't allow any attributes by default, but allow the metadata to override it. 'authproc' => array( diff --git a/modules/core/lib/Auth/Process/AttributeLimit.php b/modules/core/lib/Auth/Process/AttributeLimit.php index d74d60355c1fd103caa9945cd7e75fbb3f3093db..0ae3a92770b4b8926731e0a12ccd7b9b79d81334 100644 --- a/modules/core/lib/Auth/Process/AttributeLimit.php +++ b/modules/core/lib/Auth/Process/AttributeLimit.php @@ -19,7 +19,7 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing * * @var bool */ - private $isDefault = FALSE; + private $isDefault = false; /** @@ -113,7 +113,7 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing throw new SimpleSAML_Error_Exception('AttributeLimit: Values for ' . var_export($name, TRUE) . ' must be specified in an array.'); } - $attributes[$name] = array_intersect($attributes[$name], $allowedAttributes[$name]); + $attributes[$name] = $this->filterAttributeValues($attributes[$name], $allowedAttributes[$name]); if (!empty($attributes[$name])) { continue; } @@ -124,4 +124,45 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing } + /** + * Perform the filtering of attributes + * @param array $values The current values for a given attribute + * @param array $allowedConfigValues The allowed values, and possibly configuration options. + * @return array The filtered values + */ + private function filterAttributeValues(array $values, array $allowedConfigValues) + { + if (array_key_exists('regex', $allowedConfigValues) && $allowedConfigValues['regex'] === true) { + $matchedValues = array(); + foreach ($allowedConfigValues as $option => $pattern) { + if (!is_int($option)) { + // Ignore any configuration options in $allowedConfig. e.g. regex=>true + continue; + } + foreach ($values as $index => $attributeValue) { + /* Suppress errors in preg_match since phpunit is set to fail on warnings, which + prevents us from testing with invalid regex. + */ + $regexResult = @preg_match($pattern, $attributeValue); + if ($regexResult === false) { + \SimpleSAML\Logger::warning("Error processing regex '$pattern' on value '$attributeValue'"); + break; + } elseif ($regexResult === 1) { + $matchedValues[] = $attributeValue; + // Remove matched value incase a subsequent regex also matches it. + unset($values[$index]); + } + } + } + return $matchedValues; + } elseif (array_key_exists('ignoreCase', $allowedConfigValues) && $allowedConfigValues['ignoreCase'] === true) { + unset($allowedConfigValues['ignoreCase']); + return array_uintersect($values, $allowedConfigValues, "strcasecmp"); + } + // The not true values for these options shouldn't leak through to array_intersect + unset($allowedConfigValues['ignoreCase']); + unset($allowedConfigValues['regex']); + + return array_intersect($values, $allowedConfigValues); + } } diff --git a/tests/modules/core/lib/Auth/Process/AttributeLimitTest.php b/tests/modules/core/lib/Auth/Process/AttributeLimitTest.php index 57e912e88cdf993f143e9240e79acd455b8738a3..4939ec8b1630b3bd5d697e9c87ac0bacb6d488a2 100644 --- a/tests/modules/core/lib/Auth/Process/AttributeLimitTest.php +++ b/tests/modules/core/lib/Auth/Process/AttributeLimitTest.php @@ -267,6 +267,175 @@ class Test_Core_Auth_Process_AttributeLimitTest extends TestCase $this->assertCount(0, $attributes); } + public function testBadOptionsNotTreatedAsValidValues() { + + // Ensure really misconfigured ignoreCase and regex options are not interpretted as valid valus + $config = array( + 'eduPersonAffiliation' => array('ignoreCase' => 'member', 'nomatch'), + 'mail' => array('regex' => 'user@example.org', 'nomatch') + ); + $result = self::processFilter($config, self::$request); + $attributes = $result['Attributes']; + $this->assertCount(0, $attributes); + } + + /** + * Verify that the true value for ignoreCase doesn't get converted into a string ('1') by + * php and matched against an attribute value of '1' + */ + public function testThatIgnoreCaseOptionNotMatchBooleanAsStringValue() { + $config = array( + 'someAttribute' => array('ignoreCase' => true, 'someValue') + ); + + $request = array( + 'Attributes' => array( + 'someAttribute' => array('1'), //boolean true as a string + + ), + ); + $result = self::processFilter($config, $request); + $attributes = $result['Attributes']; + $this->assertCount(0, $attributes); + } + + /** + * Test for attribute value matching ignore case + */ + public function testMatchAttributeValuesIgnoreCase() + { + $config = array( + 'eduPersonAffiliation' => array('ignoreCase' => true, 'meMber') + ); + + $result = self::processFilter($config, self::$request); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonAffiliation', $attributes); + $this->assertEquals($attributes['eduPersonAffiliation'], array('member')); + + $config = array( + 'eduPersonAffiliation' => array('ignoreCase' => true, 'membeR','sTaff') + ); + + $result = self::processFilter($config, self::$request); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonAffiliation', $attributes); + $this->assertEquals($attributes['eduPersonAffiliation'], array('member')); + + $config = array( + 'eduPersonAffiliation' => array('ignoreCase' => true, 'Student') + ); + $result = self::processFilter($config, self::$request); + $attributes = $result['Attributes']; + $this->assertCount(0, $attributes); + + $config = array( + 'eduPersonAffiliation' => array('ignoreCase' => true, 'studeNt','sTaff') + ); + $result = self::processFilter($config, self::$request); + $attributes = $result['Attributes']; + $this->assertCount(0, $attributes); + } + + /** + * Test for attribute value matching + */ + public function testMatchAttributeValuesRegex() + { + // SSP Logger requires a configuration to be set. + SimpleSAML_Configuration::loadFromArray(array(), '[ARRAY]', 'simplesaml'); + $state = self::$request; + $state['Attributes']['eduPersonEntitlement'] = array( + 'urn:mace:example.terena.org:tcs:personal-user', + 'urn:x-surfnet:surfdomeinen.nl:role:dnsadmin', + 'urn:x-surfnet:surf.nl:surfdrive:quota:100', + '1' //boolean true as a string + ); + + $config = array( + 'eduPersonEntitlement' => array( + 'regex' => true, + '/^urn:x-surfnet:surf/' + ) + ); + + $result = self::processFilter($config, $state); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonEntitlement', $attributes); + $this->assertEquals( + array('urn:x-surfnet:surfdomeinen.nl:role:dnsadmin', 'urn:x-surfnet:surf.nl:surfdrive:quota:100'), + $attributes['eduPersonEntitlement'] + ); + + // Matching multiple lines shouldn't duplicate the attribute + $config = array( + 'eduPersonEntitlement' => array( + 'regex' => true, + '/urn:x-surfnet:surf/', + '/urn:x-surfnet/' + + ) + ); + + $result = self::processFilter($config, $state); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonEntitlement', $attributes); + $this->assertEquals( + array('urn:x-surfnet:surfdomeinen.nl:role:dnsadmin', 'urn:x-surfnet:surf.nl:surfdrive:quota:100'), + $attributes['eduPersonEntitlement'] + ); + + // Invalid and no-match regex expressions should not stop a valid regex from matching + $config = array( + 'eduPersonEntitlement' => array( + 'regex' => true, + '/urn:mace:example.terena.org:tcs:no-match/', + '$invalidRegex[', + '/^URN:x-surf.*SURF.*n$/i' + ) + ); + + $result = self::processFilter($config, $state); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonEntitlement', $attributes); + $this->assertEquals( + array('urn:x-surfnet:surfdomeinen.nl:role:dnsadmin'), + $attributes['eduPersonEntitlement'] + ); + + // No matches should remove attribute + $config = array( + 'eduPersonEntitlement' => array( + 'regex' => true, + '/urn:x-no-match/' + ) + ); + $result = self::processFilter($config, $state); + $attributes = $result['Attributes']; + $this->assertCount(0, $attributes); + + // A regex that matches an input value multiple times should work. + $config = array( + 'eduPersonEntitlement' => array( + 'regex' => true, + '/surf/' + ) + ); + $result = self::processFilter($config, $state); + $attributes = $result['Attributes']; + $this->assertCount(1, $attributes); + $this->assertArrayHasKey('eduPersonEntitlement', $attributes); + $this->assertEquals( + array('urn:x-surfnet:surfdomeinen.nl:role:dnsadmin', 'urn:x-surfnet:surf.nl:surfdrive:quota:100'), + $attributes['eduPersonEntitlement'] + ); + } + /** * Test for allowed attributes not an array. *