Skip to content
Snippets Groups Projects
Commit 0e301eb3 authored by Patrick's avatar Patrick Committed by Thijs Kinkhorst
Browse files

AttributeLimit: allow options to use regex or ignore case. (#604) 

Enhancements to AttributeLimit to give additional control over allowed values
parent 73131575
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
modules/core/docs/authproc_attributelimit.md
+ 35
− 0
View file @ 0e301eb3
...@@ -32,6 +32,41 @@ Allow `eduPersonTargetedID` and `eduPersonAffiliation` by default, but allow the ...@@ -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. Don't allow any attributes by default, but allow the metadata to override it.
'authproc' => array( 'authproc' => array(
......
modules/core/lib/Auth/Process/AttributeLimit.php
+ 43
− 2
View file @ 0e301eb3
...@@ -19,7 +19,7 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing ...@@ -19,7 +19,7 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing
* *
* @var bool * @var bool
*/ */
private $isDefault = FALSE; private $isDefault = false;
/** /**
...@@ -113,7 +113,7 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing ...@@ -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) . throw new SimpleSAML_Error_Exception('AttributeLimit: Values for ' . var_export($name, TRUE) .
' must be specified in an array.'); ' 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])) { if (!empty($attributes[$name])) {
continue; continue;
} }
...@@ -124,4 +124,45 @@ class sspmod_core_Auth_Process_AttributeLimit extends SimpleSAML_Auth_Processing ...@@ -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);
}
} }
tests/modules/core/lib/Auth/Process/AttributeLimitTest.php
+ 169
− 0
View file @ 0e301eb3
...@@ -267,6 +267,175 @@ class Test_Core_Auth_Process_AttributeLimitTest extends TestCase ...@@ -267,6 +267,175 @@ class Test_Core_Auth_Process_AttributeLimitTest extends TestCase
$this->assertCount(0, $attributes); $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. * Test for allowed attributes not an array.
* *
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment