Skip to content

Commit 3b5f5ba

Browse files
tvdijentvdijen
committed
Fix incorrect handling of samlp:Status
1 parent 6566d6b commit 3b5f5ba

3 files changed

Lines changed: 50 additions & 20 deletions

File tree

‎modules/saml/src/Controller/WebBrowserSingleSignOn.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,9 @@ public function artifactResolutionService(Request $request): Response
8484
throw new Exception("Message received on ArtifactResolutionService wasn't a ArtifactResolve request.");
8585
}
8686

87-
$issuer = $request->getIssuer();
87+
$issuer = $request->getIssuer?->getValue();
8888
/** @psalm-assert \SimpleSAML\SAML2\XML\saml\Issuer $issuer */
8989
Assert::notNull($issuer);
90-
$issuer = $issuer->getValue();
9190
$spMetadata = $metadata->getMetaDataConfig($issuer, 'saml20-sp-remote');
9291
$artifact = $request->getArtifact();
9392
$responseData = $store->get('artifact', $artifact);

‎modules/saml/src/IdP/SAML2.php‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use SimpleSAML\SAML2\Exception\ArrayValidationException;
1919
use SimpleSAML\SAML2\XML\md\ContactPerson;
2020
use SimpleSAML\SAML2\XML\saml\{AttributeValue, Issuer, NameID, SubjectConfirmation, SubjectConfirmationData};
21+
use SimpleSAML\SAML2\XML\samlp\{Status, StatusCode, StatusMessage}; // Status
2122
use SimpleSAML\XML\DOMDocumentFactory;
2223
use SimpleSAML\XMLSecurity\XML\ds\{X509Certificate, X509Data, KeyInfo};
2324
use Symfony\Bridge\PsrHttpMessage\Factory\{HttpFoundationFactory, PsrHttpFactory};
@@ -169,11 +170,20 @@ public static function handleAuthError(Error\Exception $exception, array $state)
169170
$ar->setInResponseTo($requestId);
170171
$ar->setRelayState($relayState);
171172

172-
$status = [
173-
'Code' => $error->getStatus(),
174-
'SubCode' => $error->getSubStatus(),
175-
'Message' => $error->getStatusMessage(),
176-
];
173+
$subStatus = $error->getSubStatus();
174+
if ($subStatus !== null) {
175+
$subStatus = new StatusCode($subStatus);
176+
}
177+
178+
$statusMessage = $error->getStatusMessage();
179+
if ($statusMessage !== null) {
180+
$statusMessage = new StatusMessage($statusMessage);
181+
}
182+
183+
$status = new Status(
184+
new StatusCode($error->getStatus(), $subStatus ? [$subStatus] : []),
185+
$statusMessage,
186+
);
177187
$ar->setStatus($status);
178188

179189
$statsData = [
@@ -422,21 +432,23 @@ public static function receiveAuthnRequest(Request $request, IdP $idp): Response
422432
$requestId = $request->getId();
423433
$scoping = $request->getScoping();
424434

425-
$ProxyCount = $scoping->getProxyCount();
435+
$ProxyCount = $scoping?->getProxyCount();
426436
if ($ProxyCount !== null) {
427437
$ProxyCount--;
428438
}
429439

430-
if ($scoping->getIDPList() !== null) {
431-
$IDPList = ($scoping->getIDPList()->toArray())['IDPEntry'];
440+
$IDPList = $scoping?->getIDPList();
441+
if ($IDPList !== null) {
442+
$IDPList = ($IDPList->toArray())['IDPEntry'];
432443
} else {
433444
$IDPList = [];
434445
}
435446

436-
$RequesterID = $scoping->getRequesterID();
447+
$RequesterID = $scoping?->getRequesterID();
437448
if ($RequesterID !== null) {
438-
foreach ($scoping->getRequesterID() as $k => $rid) {
439-
$RequesterID[$k] = array_pop($rid->toArray());
449+
foreach ($requesterID as $k => $rid) {
450+
$rid = $rid->toArray();
451+
$RequesterID[$k] = array_pop($rid);
440452
}
441453
}
442454

@@ -449,8 +461,8 @@ public static function receiveAuthnRequest(Request $request, IdP $idp): Response
449461
$authnContext = $request->getRequestedAuthnContext();
450462

451463
$nameIdPolicy = $request->getNameIdPolicy();
452-
$nameIDFormat = $nameIdPolicy->getFormat();
453-
$allowCreate = $nameIdPolicy->getAllowCreate() ?? false;
464+
$nameIDFormat = $nameIdPolicy?->getFormat();
465+
$allowCreate = $nameIdPolicy?->getAllowCreate() ?? false;
454466

455467
$idpInit = false;
456468

@@ -599,10 +611,12 @@ public static function sendLogoutResponse(Request $request, IdP $idp, array $sta
599611

600612
if (isset($state['core:Failed']) && $state['core:Failed']) {
601613
$partial = true;
602-
$lr->setStatus([
603-
'Code' => C::STATUS_SUCCESS,
604-
'SubCode' => C::STATUS_PARTIAL_LOGOUT,
605-
]);
614+
$lr->setStatus(new Status(new StatusCode(
615+
C::STATUS_SUCCESS,
616+
[
617+
new StatusCode(C::STATUS_PARTIAL_LOGOUT),
618+
],
619+
)));
606620
Logger::info('Sending logout response for partial logout to SP ' . var_export($spEntityId, true));
607621
} else {
608622
$partial = false;

‎modules/saml/src/Message.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
use SimpleSAML\Assert\Assert;
1111
use SimpleSAML\SAML2\{Assertion, EncryptedAssertion}; // Assertions
1212
use SimpleSAML\SAML2\{AuthnRequest, LogoutRequest, LogoutResponse, Response, StatusResponse}; // Messages
13+
use SimpleSAML\SAML2\XML\samlp\{StatusCode, StatusMessage}; // Status
1314
use SimpleSAML\SAML2\{Constants as C, SignedElement};
1415
use SimpleSAML\SAML2\XML\saml\Issuer;
1516
use SimpleSAML\XMLSecurity\XML\ds\{KeyInfo, X509Certificate, X509Data};
1617

1718
use function array_key_exists;
1819
use function array_filter;
20+
use function array_map;
1921
use function array_pop;
2022
use function array_values;
2123
use function count;
@@ -459,7 +461,22 @@ private static function decryptAttributes(
459461
public static function getResponseError(StatusResponse $response): \SimpleSAML\Module\saml\Error
460462
{
461463
$status = $response->getStatus();
462-
return new \SimpleSAML\Module\saml\Error($status['Code'], $status['SubCode'], $status['Message']);
464+
$subcode = null;
465+
if (!empty($status->getStatusCode()->getSubCodes())) {
466+
$subcodes = array_map(
467+
function (StatusCode $code) {
468+
return $code->getValue();
469+
},
470+
$status->getStatusCode()->getSubCodes(),
471+
);
472+
$subcode = implode(' / ', $subcodes);
473+
}
474+
475+
return new \SimpleSAML\Module\saml\Error(
476+
$status->getStatusCode()->getValue(),
477+
$subcode,
478+
$status->getStatusMessage()?->getContent(),
479+
);
463480
}
464481

465482

0 commit comments

Comments
 (0)