Skip to content

Commit

Permalink
Merge pull request #3 from cirrusidentity/feature/attribute-suffix
Browse files Browse the repository at this point in the history 
Add check for attributes suffixed scope
  • Loading branch information
@sitya
sitya committed Apr 6, 2016
2 parents 9add9b8 + 721c52b commit 1da3811
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 7 deletions.
6 changes: 6 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Additionally, it is also capable to handle 'scope attributes' such as _schacHome
* Regular expressions in `shibmd:Scope` are not supported.
* It is recommended to run this filter after _oid2name_. Please note that attribute names in the module configuration are case sensitive and must match the names in attributemaps.
* 'scope Attributes' must be singled valued, otherwise they are removed.
* Specifying an attribute in multiple configuration options is likely a user configuration issue. A value will only
pass if it conforms to the validation rule for each configured option.

## Installing the module
You can install the module with composer:
Expand Down Expand Up @@ -43,5 +45,9 @@ _config/config.php_
## Configurations Options

* `attributesWithScope` an array of attributes that should be scoped and should match the scope from the metadata
* `attributesWithScopeSuffix` an array of attributes that have the scope as a suffix. For example, `user@department.example.com`
and `department.example.com` are both suffixed with `example.com`. Useful when an SP is reliant on `mail` attribute to identify users and
the IdP users various subdomains for mail.
* `scopeAttributes` an array of attributes that should exactly match the scope from the metadata
* `ignoreCheckForEntities` an array of IdP entity IDs to skip scope checking for. Useful when an IdP is a SAML proxy and is trusted to assert any scope.

59 changes: 55 additions & 4 deletions lib/Auth/Process/FilterAttributes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class sspmod_attributescope_Auth_Process_FilterAttributes extends SimpleSAML_Aut

private $ignoreCheckForEntities = array();

private $attributesWithScopeSuffix = array();

public function __construct($config, $reserved)
{
parent::__construct($config, $reserved);
Expand All @@ -39,6 +41,9 @@ public function __construct($config, $reserved)
if (array_key_exists('ignoreCheckForEntities', $config)) {
$this->ignoreCheckForEntities = $config['ignoreCheckForEntities'];
}
if (array_key_exists('attributesWithScopeSuffix', $config)) {
$this->attributesWithScopeSuffix = $config['attributesWithScopeSuffix'];
}
}

/**
Expand Down Expand Up @@ -101,23 +106,69 @@ public function process(&$request)
}
}
}

foreach ($this->attributesWithScopeSuffix as $attributeWithSuffix) {
if (!isset($request['Attributes'][$attributeWithSuffix])) {
continue;
}
if ($noscope) {
SimpleSAML_Logger::info('Attribute '.$attributeWithSuffix.' is filtered out due to missing scope information in IdP metadata.');
unset($request['Attributes'][$attributeWithSuffix]);
continue;
}
$values = $request['Attributes'][$attributeWithSuffix];
$newValues = array();
foreach ($values as $value) {
if ($this->isProperlySuffixed($value, $scopes)) {
$newValues[] = $value;
} else {
SimpleSAML_Logger::warning('Attribute value ('.$value.') is removed by attributeWithScopeSuffix check.');
}
}

if (count($newValues)) {
$request['Attributes'][$attributeWithSuffix] = $newValues;
} else {
unset($request['Attributes'][$attributeWithSuffix]);
}
}
}

/**
* Determines whether an attribute value is properly scoped.
*
* @param string $value
* @param array $scopes
* @param string $value The attribute value to check
* @param array $scopes The array of scopes for the Idp
*
* @return bool
* @return bool true if properly scoped
*/
private function isProperlyScoped($value, $scopes)
{
foreach ($scopes as $scope) {
$preg = '/^[^@]*@'.preg_quote($scope).'$/';
$preg = '/^[^@]+@'.preg_quote($scope).'$/';
if (preg_match($preg, $value) == 1) {
return true;
}
}
}

/**
* Determines whether an attribute value is properly suffixed with the scope.
* @ and (literal) . are used for suffix boundries
*
* @param string $value The attribute value to check
* @param array $scopes The array of scopes for the IdP
*
* @return bool true if attribute is suffixed with a scope
*/
private function isProperlySuffixed($value, $scopes)
{
foreach ($scopes as $scope) {
$scopeRegex = '/^[^@]+@(.*\.)?'.preg_quote($scope).'$/';
$subdomainRegex = '/^([^@]*\.)?'.preg_quote($scope).'$/';
if (preg_match($subdomainRegex, $value) === 1 || preg_match($scopeRegex, $value) === 1) {
return true;
}
}
}
}
73 changes: 70 additions & 3 deletions tests/lib/Auth/Process/FilterAttributesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ private static function processFilter(array $config, array $request)
*/
public function testWrongScope($source)
{
$config = array();
$config = array(
'attributesWithScopeSuffix' => array('sampleSuffixedAttribute')
);
$request = array(
'Attributes' => array(
'eduPersonPrincipalName' => array('joe@example.com'),
'nonScopedAttribute' => array('not-removed'),
'eduPersonScopedAffiliation' => array('student@example.com', 'staff@example.com', 'missing-scope'),
'schacHomeOrganization' => array('example.com')
'schacHomeOrganization' => array('example.com'),
'sampleSuffixedAttribute' => array('joe@example.com'),
),
'Source' => $source,
);
Expand Down Expand Up @@ -108,7 +111,13 @@ public function testMixedMultivaluedAttributes()
$request = array(
'Attributes' => array(
'nonScopedAttribute' => array('not-removed'),
'eduPersonScopedAffiliation' => array('faculty@abc.com', 'student@example.com', 'staff@other.com'),
'eduPersonScopedAffiliation' => array(
'faculty@abc.com',
'student@example.com',
'staff@other.com',
'member@a@example.com',
'@example.com'
),
// schacHomeOrganization is required to be single valued and gets filtered out if multi-valued
'schacHomeOrganization' => array('abc.com', 'example.com', 'other.com')
),
Expand Down Expand Up @@ -163,4 +172,62 @@ public function testIgnoreSourceScope()
$attributes = $result['Attributes'];
$this->assertEquals($expectedData, $attributes, "Scope check ignored");
}

/**
* Test attributes values that need to end with the scope or some subdomain of the scope.
*/
public function testAttributeSuffix()
{

$request = array(
'Attributes' => array(
'department' => array(
// Valid values
'engineering.example.com', // Subdomain
'example.com', // scope
'.example.com',
// Invalid values
'invalid-example.com', // not subdomain
'cexample.com',
'examplecom',
),
'email' => array(
// Valid values
'user@example.com',
'user@gsb.example.com',
// Invalid values
'user@invalid-example.com',
'user@examplecom',
'user@cexample.com',
'abc@efg@example.com', // double '@'
// scoped values need data before the '@'
'@example.com',
'@other.example.com',
),
),
'Source' => array(
'scope' => array('example.com'),
'entityid' => 'https://example.com/idp'
)
);

$config = array(
'attributesWithScopeSuffix' => array('department', 'email')
);
$result = self::processFilter($config, $request);

$attributes = $result['Attributes'];
$expectedData = array(
'department' => array(
'engineering.example.com',
'example.com',
'.example.com',
),
'email' => array(
'user@example.com',
'user@gsb.example.com',
),
);
$this->assertEquals($expectedData, $attributes, "Incorrectly suffixed variables should be removed");
}
}

0 comments on commit 1da3811

Please sign in to comment.