Skip to content

Commit 201b4b9

Browse files
pitbulkpitbulk
committed
Add possibility to handle nameId NameQualifier attribute in SLO Request. Don't set unspecified NameIDFormat on LogoutRequest that conflicts on ADFS
1 parent c0b29ab commit 201b4b9

7 files changed

Lines changed: 168 additions & 23 deletions

File tree

‎composer.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
"satooshi/php-coveralls": ">=1.0.2",
2525
"sebastian/phpcpd": "*",
2626
"phploc/phploc": "*",
27-
"pdepend/pdepend" : ">=2.5.0"
27+
"pdepend/pdepend": ">=2.5.0",
28+
"squizlabs/php_codesniffer": "~3.1.1"
2829
},
2930
"suggest": {
3031
"ext-openssl": "Install openssl lib in order to handle with x509 certs (require to support sign and encryption)",

‎lib/Saml2/Auth.php‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ class OneLogin_Saml2_Auth
4848
*/
4949
private $_nameidFormat;
5050

51+
/**
52+
* NameID NameQualifier
53+
*
54+
* @var string
55+
*/
56+
private $_nameidNameQualifier;
57+
5158
/**
5259
* If user is authenticated.
5360
*
@@ -191,6 +198,7 @@ public function processResponse($requestId = null)
191198
$this->_attributes = $response->getAttributes();
192199
$this->_nameid = $response->getNameId();
193200
$this->_nameidFormat = $response->getNameIdFormat();
201+
$this->_nameidNameQualifier = $response->getNameIdNameQualifier();
194202
$this->_authenticated = true;
195203
$this->_sessionIndex = $response->getSessionIndex();
196204
$this->_sessionExpiration = $response->getSessionNotOnOrAfter();
@@ -350,6 +358,16 @@ public function getNameIdFormat()
350358
return $this->_nameidFormat;
351359
}
352360

361+
/**
362+
* Returns the nameID NameQualifier
363+
*
364+
* @return string The nameID NameQualifier of the assertion
365+
*/
366+
public function getNameIdNameQualifier()
367+
{
368+
return $this->_nameidNameQualifier;
369+
}
370+
353371
/**
354372
* Returns the SessionIndex
355373
*
@@ -450,18 +468,19 @@ public function login($returnTo = null, $parameters = array(), $forceAuthn = fal
450468
/**
451469
* Initiates the SLO process.
452470
*
453-
* @param string|null $returnTo The target URL the user should be returned to after logout.
454-
* @param array $parameters Extra parameters to be added to the GET
455-
* @param string|null $nameId The NameID that will be set in the LogoutRequest.
456-
* @param string|null $sessionIndex The SessionIndex (taken from the SAML Response in the SSO process).
457-
* @param bool $stay True if we want to stay (returns the url string) False to redirect
458-
* @param string|null $nameIdFormat The NameID Format will be set in the LogoutRequest.
471+
* @param string|null $returnTo The target URL the user should be returned to after logout.
472+
* @param array $parameters Extra parameters to be added to the GET
473+
* @param string|null $nameId The NameID that will be set in the LogoutRequest.
474+
* @param string|null $sessionIndex The SessionIndex (taken from the SAML Response in the SSO process).
475+
* @param bool $stay True if we want to stay (returns the url string) False to redirect
476+
* @param string|null $nameIdFormat The NameID Format will be set in the LogoutRequest.
477+
* @param string|null $nameIdNameQualifier The NameID NameQualifier will be set in the LogoutRequest.
459478
*
460479
* @return If $stay is True, it return a string with the SLO URL + LogoutRequest + parameters
461480
*
462481
* @throws OneLogin_Saml2_Error
463482
*/
464-
public function logout($returnTo = null, $parameters = array(), $nameId = null, $sessionIndex = null, $stay = false, $nameIdFormat = null)
483+
public function logout($returnTo = null, $parameters = array(), $nameId = null, $sessionIndex = null, $stay = false, $nameIdFormat = null, $nameIdNameQualifier = null)
465484
{
466485
assert(is_array($parameters));
467486

@@ -480,7 +499,7 @@ public function logout($returnTo = null, $parameters = array(), $nameId = null,
480499
$nameIdFormat = $this->_nameidFormat;
481500
}
482501

483-
$logoutRequest = new OneLogin_Saml2_LogoutRequest($this->_settings, null, $nameId, $sessionIndex, $nameIdFormat);
502+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($this->_settings, null, $nameId, $sessionIndex, $nameIdFormat, $nameIdNameQualifier);
484503

485504
$this->_lastRequest = $logoutRequest->getXML();
486505
$this->_lastRequestID = $logoutRequest->id;

‎lib/Saml2/LogoutRequest.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,14 @@ class OneLogin_Saml2_LogoutRequest
5151
/**
5252
* Constructs the Logout Request object.
5353
*
54-
* @param OneLogin_Saml2_Settings $settings Settings
55-
* @param string|null $request A UUEncoded Logout Request.
56-
* @param string|null $nameId The NameID that will be set in the LogoutRequest.
57-
* @param string|null $sessionIndex The SessionIndex (taken from the SAML Response in the SSO process).
58-
* @param string|null $nameIdFormat The NameID Format will be set in the LogoutRequest.
54+
* @param OneLogin_Saml2_Settings $settings Settings
55+
* @param string|null $request A UUEncoded Logout Request.
56+
* @param string|null $nameId The NameID that will be set in the LogoutRequest.
57+
* @param string|null $sessionIndex The SessionIndex (taken from the SAML Response in the SSO process).
58+
* @param string|null $nameIdFormat The NameID Format will be set in the LogoutRequest.
59+
* @param string|null $nameIdNameQualifier The NameID NameQualifier will be set in the LogoutRequest.
5960
*/
60-
public function __construct(OneLogin_Saml2_Settings $settings, $request = null, $nameId = null, $sessionIndex = null, $nameIdFormat = null)
61+
public function __construct(OneLogin_Saml2_Settings $settings, $request = null, $nameId = null, $sessionIndex = null, $nameIdFormat = null, $nameIdNameQualifier = null)
6162
{
6263
$this->_settings = $settings;
6364

@@ -89,7 +90,8 @@ public function __construct(OneLogin_Saml2_Settings $settings, $request = null,
8990
}
9091

9192
if (!empty($nameId)) {
92-
if (empty($nameIdFormat)) {
93+
if (empty($nameIdFormat)
94+
&& $spData['NameIDFormat'] != OneLogin_Saml2_Constants::NAMEID_UNSPECIFIED) {
9395
$nameIdFormat = $spData['NameIDFormat'];
9496
}
9597
$spNameQualifier = null;
@@ -103,7 +105,8 @@ public function __construct(OneLogin_Saml2_Settings $settings, $request = null,
103105
$nameId,
104106
$spNameQualifier,
105107
$nameIdFormat,
106-
$cert
108+
$cert,
109+
$nameIdNameQualifier
107110
);
108111

109112
$sessionIndexStr = isset($sessionIndex) ? "<samlp:SessionIndex>{$sessionIndex}</samlp:SessionIndex>" : "";

‎lib/Saml2/Response.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,21 @@ public function getNameIdFormat()
668668
return $nameIdFormat;
669669
}
670670

671+
/**
672+
* Gets the NameID NameQualifier provided by the SAML response from the IdP.
673+
*
674+
* @return string Name ID NameQualifier
675+
*/
676+
public function getNameIdNameQualifier()
677+
{
678+
$nameIdNameQualifier = null;
679+
$nameIdData = $this->getNameIdData();
680+
if (!empty($nameIdData) && isset($nameIdData['NameQualifier'])) {
681+
$nameIdNameQualifier = $nameIdData['NameQualifier'];
682+
}
683+
return $nameIdNameQualifier;
684+
}
685+
671686
/**
672687
* Gets the SessionNotOnOrAfter from the AuthnStatement.
673688
* Could be used to set the local session expiration

‎lib/Saml2/Utils.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -973,10 +973,11 @@ public static function formatFingerPrint($fingerprint)
973973
* @param string $spnq SP Name Qualifier
974974
* @param string $format SP Format
975975
* @param string|null $cert IdP Public cert to encrypt the nameID
976+
* @param string|null $nq IdP Name Qualifier
976977
*
977978
* @return string $nameIDElement DOMElement | XMLSec nameID
978979
*/
979-
public static function generateNameId($value, $spnq, $format, $cert = null)
980+
public static function generateNameId($value, $spnq, $format = null, $cert = null, $nq = null)
980981
{
981982

982983
$doc = new DOMDocument();
@@ -985,7 +986,12 @@ public static function generateNameId($value, $spnq, $format, $cert = null)
985986
if (isset($spnq)) {
986987
$nameId->setAttribute('SPNameQualifier', $spnq);
987988
}
988-
$nameId->setAttribute('Format', $format);
989+
if (isset($nq)) {
990+
$nameId->setAttribute('NameQualifier', $nq);
991+
}
992+
if (isset($format)) {
993+
$nameId->setAttribute('Format', $format);
994+
}
989995
$nameId->appendChild($doc->createTextNode($value));
990996

991997
$doc->appendChild($nameId);

‎tests/src/OneLogin/Saml2/LogoutRequestTest.php‎

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public function testConstructorWithSessionIndex()
109109
*
110110
* @covers OneLogin_Saml2_LogoutRequest
111111
*/
112-
public function testConstructorWithNameIdFormat()
112+
public function testConstructorWithNameIdFormatOnParameter()
113113
{
114114
$settingsDir = TEST_ROOT .'/settings/';
115115
include $settingsDir.'settings1.php';
@@ -137,6 +137,93 @@ public function testConstructorWithNameIdFormat()
137137
$this->assertEquals($nameIdFormat, $logoutNameIdData['Format']);
138138
}
139139

140+
/**
141+
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
142+
*
143+
* @covers OneLogin_Saml2_LogoutRequest
144+
*/
145+
public function testConstructorWithNameIdFormatOnSettings()
146+
{
147+
$settingsDir = TEST_ROOT .'/settings/';
148+
include $settingsDir.'settings1.php';
149+
$nameId = 'test@example.com';
150+
$nameIdFormat = 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient';
151+
$settingsInfo['sp']['NameIDFormat'] = $nameIdFormat;
152+
$settings = new OneLogin_Saml2_Settings($settingsInfo);
153+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($settings, null, $nameId, null, null);
154+
$parameters = array('SAMLRequest' => $logoutRequest->getRequest());
155+
$logoutUrl = OneLogin_Saml2_Utils::redirect('http://idp.example.com/SingleLogoutService.php', $parameters, true);
156+
$this->assertRegExp('#^http://idp\.example\.com\/SingleLogoutService\.php\?SAMLRequest=#', $logoutUrl);
157+
parse_str(parse_url($logoutUrl, PHP_URL_QUERY), $exploded);
158+
// parse_url already urldecode de params so is not required.
159+
$payload = $exploded['SAMLRequest'];
160+
$decoded = base64_decode($payload);
161+
$inflated = gzinflate($decoded);
162+
$this->assertRegExp('#^<samlp:LogoutRequest#', $inflated);
163+
$logoutNameId = OneLogin_Saml2_LogoutRequest::getNameId($inflated);
164+
$this->assertEquals($nameId, $logoutNameId);
165+
$logoutNameIdData = OneLogin_Saml2_LogoutRequest::getNameIdData($inflated);
166+
$this->assertEquals($nameIdFormat, $logoutNameIdData['Format']);
167+
}
168+
169+
/**
170+
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
171+
*
172+
* @covers OneLogin_Saml2_LogoutRequest
173+
*/
174+
public function testConstructorWithoutNameIdFormat()
175+
{
176+
$settingsDir = TEST_ROOT .'/settings/';
177+
include $settingsDir.'settings1.php';
178+
$nameId = 'test@example.com';
179+
$nameIdFormat = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified';
180+
$settingsInfo['sp']['NameIDFormat'] = $nameIdFormat;
181+
$settings = new OneLogin_Saml2_Settings($settingsInfo);
182+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($settings, null, $nameId, null, null);
183+
$parameters = array('SAMLRequest' => $logoutRequest->getRequest());
184+
$logoutUrl = OneLogin_Saml2_Utils::redirect('http://idp.example.com/SingleLogoutService.php', $parameters, true);
185+
$this->assertRegExp('#^http://idp\.example\.com\/SingleLogoutService\.php\?SAMLRequest=#', $logoutUrl);
186+
parse_str(parse_url($logoutUrl, PHP_URL_QUERY), $exploded);
187+
// parse_url already urldecode de params so is not required.
188+
$payload = $exploded['SAMLRequest'];
189+
$decoded = base64_decode($payload);
190+
$inflated = gzinflate($decoded);
191+
$this->assertRegExp('#^<samlp:LogoutRequest#', $inflated);
192+
$logoutNameId = OneLogin_Saml2_LogoutRequest::getNameId($inflated);
193+
$this->assertEquals($nameId, $logoutNameId);
194+
$logoutNameIdData = OneLogin_Saml2_LogoutRequest::getNameIdData($inflated);
195+
$this->assertFalse(isset($logoutNameIdData['Format']));
196+
}
197+
/**
198+
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
199+
*
200+
* @covers OneLogin_Saml2_LogoutRequest
201+
*/
202+
public function testConstructorWithNameIdNameQualifier()
203+
{
204+
$settingsDir = TEST_ROOT .'/settings/';
205+
include $settingsDir.'settings1.php';
206+
$nameId = 'test@example.com';
207+
$nameIdFormat = 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient';
208+
$nameIdNameQualifier = 'https://test.example.com/saml/metadata';
209+
$settings = new OneLogin_Saml2_Settings($settingsInfo);
210+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($settings, null, $nameId, null, $nameIdFormat, $nameIdNameQualifier);
211+
$parameters = array('SAMLRequest' => $logoutRequest->getRequest());
212+
$logoutUrl = OneLogin_Saml2_Utils::redirect('http://idp.example.com/SingleLogoutService.php', $parameters, true);
213+
$this->assertRegExp('#^http://idp\.example\.com\/SingleLogoutService\.php\?SAMLRequest=#', $logoutUrl);
214+
parse_str(parse_url($logoutUrl, PHP_URL_QUERY), $exploded);
215+
// parse_url already urldecode de params so is not required.
216+
$payload = $exploded['SAMLRequest'];
217+
$decoded = base64_decode($payload);
218+
$inflated = gzinflate($decoded);
219+
$this->assertRegExp('#^<samlp:LogoutRequest#', $inflated);
220+
$logoutNameId = OneLogin_Saml2_LogoutRequest::getNameId($inflated);
221+
$this->assertEquals($nameId, $logoutNameId);
222+
$logoutNameIdData = OneLogin_Saml2_LogoutRequest::getNameIdData($inflated);
223+
$this->assertEquals($nameIdFormat, $logoutNameIdData['Format']);
224+
$this->assertEquals($nameIdNameQualifier, $logoutNameIdData['NameQualifier']);
225+
}
226+
140227
/**
141228
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
142229
* The creation of a deflated SAML Logout Request

‎tests/src/OneLogin/Saml2/UtilsTest.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,9 +795,6 @@ public function testQuery()
795795
*/
796796
public function testGenerateNameIdWithSPNameQualifier()
797797
{
798-
//$xml = '<root xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">'.$decrypted.'</root>';
799-
//$newDoc = new DOMDocument();
800-
801798
$nameIdValue = 'ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde';
802799
$entityId = 'http://stuff.com/endpoints/metadata.php';
803800
$nameIDFormat = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified';
@@ -829,6 +826,23 @@ public function testGenerateNameIdWithSPNameQualifier()
829826
$this->assertContains($nameidExpectedEnc, $nameIdEnc);
830827
}
831828

829+
/**
830+
* Tests the generateNameId method of the OneLogin_Saml2_Utils
831+
*
832+
* @covers OneLogin_Saml2_Utils::generateNameId
833+
*/
834+
public function testGenerateNameIdWithoutFormat()
835+
{
836+
$nameIdValue = 'ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde';
837+
$nameId = OneLogin_Saml2_Utils::generateNameId(
838+
$nameIdValue,
839+
null,
840+
null
841+
);
842+
$expectedNameId = '<saml:NameID>ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde</saml:NameID>';
843+
$this->assertEquals($nameId, $expectedNameId);
844+
}
845+
832846
/**
833847
* Tests the generateNameId method of the OneLogin_Saml2_Utils
834848
*

0 commit comments

Comments
 (0)