Skip to content

Commit da02c3c

Browse files
kwesselmonkeyiq
kwessel
and
monkeyiq
authored
New 'identifyingAttribute option for SAML Attribute NameID filter (#2260)
* New 'identifyingAttribute option for SAML Attribute NameID filter Allow a new option in place of identifyingAttribute called identifyingAttributes. This option expects an array of attribute names. The first one in the list that's available in the attributes being released to the SP will be used. This is particularly useful in the hosted IdP configuration. One can specify multiple attribute names in order of precedence, and the IdP will release the first one from that list to the SP that's in the SP's 'attributes' array. This avoids having to define a new authproc filter for each SP that needs a different value than the single one that was previously specified using 'identifyingAttribute'. The change still supports identifyingAttribute option which will be used instead of the new option if both are set. * Throw error when AttributeNameID filter has both identifyingAttribute and identifyingAttributes set * identifyingAttributes clean-up per PR comments * Cleaned up logic for finding a value for the nameID * Corrected bug when $value was empty at end of loop * add a test for the AttributeNameID using an array of attributes * lint * Update AttributeNameIDTest.php These entity ids were copied from the test that I then updated which lead to much better values for test idp and sp entities. I am updating these entities here inline with the comment at #2349 (review) * Update nameid.md Mention that these are mutex options. * Update nameid.md lint --------- Co-authored-by: Ben Martin <monkeyiq@users.sourceforge.net> Co-authored-by: monkeyiq <monkeyiq@gmail.com>
1 parent e6748d2 commit da02c3c

File tree

3 files changed

+193
-33
lines changed

3 files changed

+193
-33
lines changed

‎modules/saml/docs/nameid.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ Uses the value of an attribute to generate a NameID.
2929
`identifyingAttribute`
3030
: The name of the attribute we should use as the unique user ID.
3131

32+
`identifyingAttributes`
33+
: An array of attribute names to consider for the unique user ID.
34+
: The first attribute found in this array that's being released to the SP
35+
: will be used. Note that using this option means you must not also use
36+
: identifyingAttribute.
37+
3238
`Format`
3339
: The `Format` attribute of the generated NameID.
3440

@@ -113,7 +119,7 @@ This example makes three NameIDs available:
113119
],
114120
3 => [
115121
'class' => 'saml:AttributeNameID',
116-
'identifyingAttribute' => 'mail',
122+
'identifyingAttributes' => ['mail','eduPersonPrincipalName'],
117123
'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
118124
],
119125
],

‎modules/saml/src/Auth/Process/AttributeNameID.php‎

Lines changed: 51 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121
class AttributeNameID extends BaseNameIDGenerator
2222
{
2323
/**
24-
* The attribute we should use as the NameID.
24+
* A list of possible attributes we can use as the NameID.
25+
* The first one found in the attributes being released to the SP
26+
* will be used.
2527
*
26-
* @var string
28+
* @var array
2729
*/
28-
private string $identifyingAttribute;
30+
private array $identifyingAttributes;
2931

3032

3133
/**
@@ -34,7 +36,8 @@ class AttributeNameID extends BaseNameIDGenerator
3436
* @param array $config Configuration information about this filter.
3537
* @param mixed $reserved For future use.
3638
*
37-
* @throws \SimpleSAML\Error\Exception If the required options 'Format' or 'identifyingAttribute' are missing.
39+
* @throws \SimpleSAML\Error\Exception If the required options 'Format' or 'identifyingAttribute'
40+
* and 'identifyingAttributes' are either both missing or both set.
3841
*/
3942
public function __construct(array $config, $reserved)
4043
{
@@ -45,10 +48,20 @@ public function __construct(array $config, $reserved)
4548
}
4649
$this->format = (string) $config['Format'];
4750

48-
if (!isset($config['identifyingAttribute'])) {
49-
throw new Error\Exception("AttributeNameID: Missing required option 'identifyingAttribute'.");
51+
if (!isset($config['identifyingAttribute']) && !isset($config['identifyingAttributes'])) {
52+
throw new Error\Exception("AttributeNameID: Missing required " .
53+
"option one of 'identifyingAttribute' or 'identifyingAttributes'.");
54+
} elseif (isset($config['identifyingAttribute']) && isset($config['identifyingAttributes'])) {
55+
throw new Error\Exception("AttributeNameID: Options " .
56+
"'identifyingAttribute' and 'identifyingAttributes' are mutually " .
57+
"exclusive but both were provided.");
58+
}
59+
60+
if (isset($config['identifyingAttribute'])) {
61+
$this->identifyingAttributes[0] = (string) $config['identifyingAttribute'];
62+
} else {
63+
$this->identifyingAttributes = (array) $config['identifyingAttributes'];
5064
}
51-
$this->identifyingAttribute = (string) $config['identifyingAttribute'];
5265
}
5366

5467

@@ -60,35 +73,41 @@ public function __construct(array $config, $reserved)
6073
*/
6174
protected function getValue(array &$state): ?string
6275
{
63-
if (
64-
!isset($state['Attributes'][$this->identifyingAttribute])
65-
|| count($state['Attributes'][$this->identifyingAttribute]) === 0
66-
) {
67-
Logger::warning(
68-
'Missing attribute ' . var_export($this->identifyingAttribute, true) .
69-
' on user - not generating attribute NameID.',
70-
);
71-
return null;
72-
}
73-
if (count($state['Attributes'][$this->identifyingAttribute]) > 1) {
74-
Logger::warning(
75-
'More than one value in attribute ' . var_export($this->identifyingAttribute, true) .
76-
' on user - not generating attribute NameID.',
77-
);
78-
return null;
76+
foreach ($this->identifyingAttributes as $attr) {
77+
if (isset($state['Attributes'][$attr])) {
78+
if (count($state['Attributes'][$attr]) === 1) {
79+
// just in case the first index is no longer 0
80+
$value = array_values($state['Attributes'][$attr]);
81+
$value = strval($value[0]);
82+
83+
if (!empty($value)) {
84+
// Found the attribute
85+
break;
86+
} else { // empty value
87+
unset($value);
88+
Logger::warning(
89+
'Empty value in attribute ' . var_export($attr, true) .
90+
' on user - not using for attribute NameID.',
91+
);
92+
}
93+
} else { // multi-valued attribute
94+
Logger::warning(
95+
'More than one value in attribute ' . var_export($attr, true) .
96+
' on user - not using for attribute NameID.',
97+
);
98+
}
99+
} else { // attribute not returned
100+
Logger::warning(
101+
'Missing attribute ' . var_export($attr, true) .
102+
' on user - not using for attribute NameID.',
103+
);
104+
}
79105
}
80-
// just in case the first index is no longer 0
81-
$value = array_values($state['Attributes'][$this->identifyingAttribute]);
82-
$value = strval($value[0]);
106+
unset($attr);
83107

84-
if (empty($value)) {
85-
Logger::warning(
86-
'Empty value in attribute ' . var_export($this->identifyingAttribute, true) .
87-
' on user - not generating attribute NameID.',
88-
);
108+
if (!isset($value)) {
89109
return null;
90110
}
91-
92111
return $value;
93112
}
94113
}

‎tests/modules/saml/src/Auth/Process/AttributeNameIDTest.php‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\saml\Auth\Process;
6+
7+
use PHPUnit\Framework\Attributes\CoversClass;
8+
use PHPUnit\Framework\TestCase;
9+
use SimpleSAML\Module\saml\Auth\Process\AttributeNameID;
10+
use SimpleSAML\SAML2\Constants as C;
11+
12+
/**
13+
* Test for the AttributeNameID filter.
14+
*
15+
* @package SimpleSAMLphp
16+
*/
17+
#[CoversClass(AttributeNameID::class)]
18+
class AttributeNameIDTest extends TestCase
19+
{
20+
/**
21+
* Helper function to run the filter with a given configuration.
22+
*
23+
* @param array $config The filter configuration.
24+
* @param array $request The request state.
25+
* @return array The state array after processing.
26+
*/
27+
private function processFilter(array $config, array $request): array
28+
{
29+
$filter = new AttributeNameID($config, null);
30+
$filter->process($request);
31+
return $request;
32+
}
33+
34+
35+
/**
36+
* Test minimal configuration.
37+
*/
38+
public function testMinimalConfig(): void
39+
{
40+
$config = [];
41+
$spId = 'urn:x-simplesamlphp:sp';
42+
$idpId = 'urn:x-simplesamlphp:idp';
43+
$expectedEmail = 'foo@there';
44+
45+
$config = [
46+
'class' => 'saml:AttributeNameID',
47+
'identifyingAttributes' => ['mail','eduPersonPrincipalName'],
48+
'Format' => C::NAMEID_PERSISTENT,
49+
];
50+
51+
$request = [
52+
'Source' => [
53+
'entityid' => $spId,
54+
],
55+
'Destination' => [
56+
'entityid' => $idpId,
57+
],
58+
'Attributes' => [
59+
'eduPersonPrincipalName' => ['foo@there'],
60+
],
61+
];
62+
$result = $this->processFilter($config, $request);
63+
$this->assertNotNull($result['saml:NameID']);
64+
$resultNameId = $result['saml:NameID'][C::NAMEID_PERSISTENT];
65+
$this->assertNotNull($resultNameId);
66+
$this->assertEquals($expectedEmail, $resultNameId->toArray()['value']);
67+
}
68+
69+
70+
/**
71+
* Test third element in chain
72+
*/
73+
public function testSuccessInThirdElement(): void
74+
{
75+
$config = [];
76+
$spId = 'urn:x-simplesamlphp:sp';
77+
$idpId = 'urn:x-simplesamlphp:idp';
78+
$expectedEmail = 'foo@there';
79+
80+
$config = [
81+
'class' => 'saml:AttributeNameID',
82+
'identifyingAttributes' => ['mail','somethingelse','eduPersonPrincipalName'],
83+
'Format' => C::NAMEID_PERSISTENT,
84+
];
85+
86+
$request = [
87+
'Source' => [
88+
'entityid' => $spId,
89+
],
90+
'Destination' => [
91+
'entityid' => $idpId,
92+
],
93+
'Attributes' => [
94+
'eduPersonPrincipalName' => ['foo@there'],
95+
],
96+
];
97+
$result = $this->processFilter($config, $request);
98+
$this->assertNotNull($result['saml:NameID']);
99+
$resultNameId = $result['saml:NameID'][C::NAMEID_PERSISTENT];
100+
$this->assertNotNull($resultNameId);
101+
$this->assertEquals($expectedEmail, $resultNameId->toArray()['value']);
102+
}
103+
104+
105+
/**
106+
* Test attributes in list not found.
107+
*/
108+
public function testNotFound(): void
109+
{
110+
$config = [];
111+
$spId = 'urn:x-simplesamlphp:sp';
112+
$idpId = 'urn:x-simplesamlphp:idp';
113+
$expectedEmail = 'foo@there';
114+
115+
$config = [
116+
'class' => 'saml:AttributeNameID',
117+
'identifyingAttributes' => ['mail','eduPersonPrincipalName'],
118+
'Format' => C::NAMEID_PERSISTENT,
119+
];
120+
121+
$request = [
122+
'Source' => [
123+
'entityid' => $spId,
124+
],
125+
'Destination' => [
126+
'entityid' => $idpId,
127+
],
128+
'Attributes' => [
129+
'magic' => ['foo@there'],
130+
],
131+
];
132+
$result = $this->processFilter($config, $request);
133+
$this->assertNull($result['saml:NameID']);
134+
}
135+
}

0 commit comments

Comments
 (0)