Skip to content

Commit 30ad63f

Browse files
pitbulkpitbulk
committed
Fix #263. Issue with LogoutRequest rejected by ADFS due NameID with unspecified format instead no format attribute
1 parent 52e2fde commit 30ad63f

4 files changed

Lines changed: 94 additions & 4 deletions

File tree

‎lib/Saml2/LogoutRequest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ public function __construct(OneLogin_Saml2_Settings $settings, $request = null,
7272
}
7373

7474
if (!empty($nameId)) {
75-
if (empty($nameIdFormat)) {
75+
if (empty($nameIdFormat) &&
76+
$spData['NameIDFormat'] != OneLogin_Saml2_Constants::NAMEID_UNSPECIFIED) {
7677
$nameIdFormat = $spData['NameIDFormat'];
7778
}
7879
$spNameQualifier = null;

‎lib/Saml2/Utils.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ public static function formatFingerPrint($fingerprint)
954954
*
955955
* @return string $nameIDElement DOMElement | XMLSec nameID
956956
*/
957-
public static function generateNameId($value, $spnq, $format, $cert = null, $nq = null)
957+
public static function generateNameId($value, $spnq, $format = null, $cert = null, $nq = null)
958958
{
959959

960960
$doc = new DOMDocument();
@@ -966,7 +966,9 @@ public static function generateNameId($value, $spnq, $format, $cert = null, $nq
966966
if (isset($nq)) {
967967
$nameId->setAttribute('NameQualifier', $nq);
968968
}
969-
$nameId->setAttribute('Format', $format);
969+
if (isset($format)) {
970+
$nameId->setAttribute('Format', $format);
971+
}
970972
$nameId->appendChild($doc->createTextNode($value));
971973

972974
$doc->appendChild($nameId);

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

Lines changed: 69 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,74 @@ 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+
150+
$nameId = 'test@example.com';
151+
$nameIdFormat = 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient';
152+
$settingsInfo['sp']['NameIDFormat'] = $nameIdFormat;
153+
$settings = new OneLogin_Saml2_Settings($settingsInfo);
154+
155+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($settings, null, $nameId, null, null);
156+
157+
$parameters = array('SAMLRequest' => $logoutRequest->getRequest());
158+
$logoutUrl = OneLogin_Saml2_Utils::redirect('http://idp.example.com/SingleLogoutService.php', $parameters, true);
159+
$this->assertRegExp('#^http://idp\.example\.com\/SingleLogoutService\.php\?SAMLRequest=#', $logoutUrl);
160+
parse_str(parse_url($logoutUrl, PHP_URL_QUERY), $exploded);
161+
// parse_url already urldecode de params so is not required.
162+
$payload = $exploded['SAMLRequest'];
163+
$decoded = base64_decode($payload);
164+
$inflated = gzinflate($decoded);
165+
$this->assertRegExp('#^<samlp:LogoutRequest#', $inflated);
166+
167+
$logoutNameId = OneLogin_Saml2_LogoutRequest::getNameId($inflated);
168+
$this->assertEquals($nameId, $logoutNameId);
169+
170+
$logoutNameIdData = OneLogin_Saml2_LogoutRequest::getNameIdData($inflated);
171+
$this->assertEquals($nameIdFormat, $logoutNameIdData['Format']);
172+
}
173+
174+
/**
175+
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
176+
*
177+
* @covers OneLogin_Saml2_LogoutRequest
178+
*/
179+
public function testConstructorWithoutNameIdFormat()
180+
{
181+
$settingsDir = TEST_ROOT .'/settings/';
182+
include $settingsDir.'settings1.php';
183+
184+
$nameId = 'test@example.com';
185+
$nameIdFormat = 'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified';
186+
$settingsInfo['sp']['NameIDFormat'] = $nameIdFormat;
187+
$settings = new OneLogin_Saml2_Settings($settingsInfo);
188+
189+
$logoutRequest = new OneLogin_Saml2_LogoutRequest($settings, null, $nameId, null, null);
190+
191+
$parameters = array('SAMLRequest' => $logoutRequest->getRequest());
192+
$logoutUrl = OneLogin_Saml2_Utils::redirect('http://idp.example.com/SingleLogoutService.php', $parameters, true);
193+
$this->assertRegExp('#^http://idp\.example\.com\/SingleLogoutService\.php\?SAMLRequest=#', $logoutUrl);
194+
parse_str(parse_url($logoutUrl, PHP_URL_QUERY), $exploded);
195+
// parse_url already urldecode de params so is not required.
196+
$payload = $exploded['SAMLRequest'];
197+
$decoded = base64_decode($payload);
198+
$inflated = gzinflate($decoded);
199+
$this->assertRegExp('#^<samlp:LogoutRequest#', $inflated);
200+
201+
$logoutNameId = OneLogin_Saml2_LogoutRequest::getNameId($inflated);
202+
$this->assertEquals($nameId, $logoutNameId);
203+
204+
$logoutNameIdData = OneLogin_Saml2_LogoutRequest::getNameIdData($inflated);
205+
$this->assertFalse(isset($logoutNameIdData['Format']));
206+
}
207+
140208
/**
141209
* Tests the OneLogin_Saml2_LogoutRequest Constructor.
142210
*

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,25 @@ public function testGenerateNameIdWithSPNameQualifier()
812812
$this->assertContains($nameidExpectedEnc, $nameIdEnc);
813813
}
814814

815+
/**
816+
* Tests the generateNameId method of the OneLogin_Saml2_Utils
817+
*
818+
* @covers OneLogin_Saml2_Utils::generateNameId
819+
*/
820+
public function testGenerateNameIdWithoutFormat()
821+
{
822+
$nameIdValue = 'ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde';
823+
824+
$nameId = OneLogin_Saml2_Utils::generateNameId(
825+
$nameIdValue,
826+
null,
827+
null
828+
);
829+
830+
$expectedNameId = '<saml:NameID>ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde</saml:NameID>';
831+
$this->assertEquals($nameId, $expectedNameId);
832+
}
833+
815834
/**
816835
* Tests the generateNameId method of the OneLogin_Saml2_Utils
817836
*

0 commit comments

Comments
 (0)