From 44b99a6ada56fbaf8d7f75a879b8a081c4cf250b Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Mon, 12 Jun 2023 22:50:42 +0200
Subject: [PATCH] Migrate samlp:Scoping + sub-elements to new interface
---
modules/saml/src/Auth/Source/SP.php | 31 +++++++----
modules/saml/src/IdP/SAML2.php | 19 +++++--
tests/modules/saml/src/Auth/Source/SPTest.php | 51 ++++++++++---------
3 files changed, 63 insertions(+), 38 deletions(-)
diff --git a/modules/saml/src/Auth/Source/SP.php b/modules/saml/src/Auth/Source/SP.php
index 1693e5883..a0f8403eb 100644
--- a/modules/saml/src/Auth/Source/SP.php
+++ b/modules/saml/src/Auth/Source/SP.php
@@ -14,7 +14,7 @@ use SimpleSAML\SAML2\Exception\ArrayValidationException;
use SimpleSAML\SAML2\Exception\Protocol\{NoAvailableIDPException, NoPassiveException, NoSupportedIDPException};
use SimpleSAML\SAML2\XML\md\ContactPerson;
use SimpleSAML\SAML2\XML\saml\NameID;
-use SimpleSAML\SAML2\XML\samlp\Extensions;
+use SimpleSAML\SAML2\XML\samlp\{Extensions, IDPEntry, IDPList, RequesterID, Scoping};
use SimpleSAML\Store\StoreFactory;
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
use Symfony\Component\HttpFoundation\{RedirectResponse, Request, Response};
@@ -565,39 +565,50 @@ class SP extends Auth\Source
$ar->setNameIdPolicy($state['saml:NameIDPolicy']);
}
+ $proxyCount = $idpList = null;
$requesterID = [];
/* Only check for real info for Scoping element if we are going to send Scoping element */
if ($this->disable_scoping !== true && $idpMetadata->getOptionalBoolean('disable_scoping', false) !== true) {
+ $idpEntry = [];
if (isset($state['IDPList'])) {
- $ar->setIDPList($state['IDPList']);
+ $idpList = $state['IDPList'];
} elseif (!empty($this->metadata->getOptionalArray('IDPList', []))) {
- $ar->setIDPList($this->metadata->getArray('IDPList'));
+ foreach ($this->metadata->getArray('IDPList') as $entry) {
+ $idpEntry[] = new IDPEntry($entry);
+ }
+ $idpList = new IDPList($idpEntry);
} elseif (!empty($idpMetadata->getOptionalArray('IDPList', []))) {
- $ar->setIDPList($idpMetadata->getArray('IDPList'));
+ foreach ($idpMetadata->getArray('IDPList') as $entry) {
+ $idpEntry[] = new IDPEntry($entry);
+ }
+ $idpList = new IDPList($idpEntry);
}
if (isset($state['saml:ProxyCount']) && $state['saml:ProxyCount'] !== null) {
- $ar->setProxyCount($state['saml:ProxyCount']);
+ $proxyCount = $state['saml:ProxyCount'];
} elseif ($idpMetadata->hasValue('ProxyCount')) {
- $ar->setProxyCount($idpMetadata->getInteger('ProxyCount'));
+ $proxyCount = $idpMetadata->getInteger('ProxyCount');
} elseif ($this->metadata->hasValue('ProxyCount')) {
- $ar->setProxyCount($this->metadata->getInteger('ProxyCount'));
+ $proxyCount = $this->metadata->getInteger('ProxyCount');
}
$requesterID = [];
if (isset($state['saml:RequesterID'])) {
- $requesterID = $state['saml:RequesterID'];
+ foreach ($state['saml:RequesterID'] as $requesterId) {
+ $requesterID[] = new RequesterID($requesterId);
+ }
}
if (isset($state['core:SP'])) {
- $requesterID[] = $state['core:SP'];
+ $requesterID[] = new RequesterID($state['core:SP']);
}
} else {
Logger::debug('Disabling samlp:Scoping for ' . var_export($idpMetadata->getString('entityid'), true));
}
- $ar->setRequesterID($requesterID);
+ $scoping = new Scoping($proxyCount, $idpList, $requesterID);
+ $ar->setScoping($scoping);
// If the downstream SP has set extensions then use them.
// Otherwise use extensions that might be defined in the local SP (only makes sense in a proxy scenario)
diff --git a/modules/saml/src/IdP/SAML2.php b/modules/saml/src/IdP/SAML2.php
index 92de48445..b83b2626e 100644
--- a/modules/saml/src/IdP/SAML2.php
+++ b/modules/saml/src/IdP/SAML2.php
@@ -24,6 +24,7 @@ use Symfony\Bridge\PsrHttpMessage\Factory\{HttpFoundationFactory, PsrHttpFactory
use Symfony\Component\HttpFoundation\{Request, Response};
use function array_key_exists;
+use function array_map;
use function array_merge;
use function array_unique;
use function array_unshift;
@@ -419,12 +420,24 @@ class SAML2
$relayState = $request->getRelayState();
$requestId = $request->getId();
- $IDPList = $request->getIDPList();
- $ProxyCount = $request->getProxyCount();
+ $scoping = $request->getScoping();
+
+ $ProxyCount = $scoping->getProxyCount();
if ($ProxyCount !== null) {
$ProxyCount--;
}
- $RequesterID = $request->getRequesterID();
+
+ if ($scoping->getIDPList() !== null) {
+ $IDPList = ($scoping->getIDPList()->toArray())['IDPEntry'];
+ } else {
+ $IDPList = [];
+ }
+
+ $RequesterID = $scoping->getRequesterID();
+ if ($RequesterID !== null) {
+ $RequesterID = array_map('strval', $RequesterID);
+ }
+
$forceAuthn = $request->getForceAuthn();
$isPassive = $request->getIsPassive();
$consumerURL = $request->getAssertionConsumerServiceURL();
diff --git a/tests/modules/saml/src/Auth/Source/SPTest.php b/tests/modules/saml/src/Auth/Source/SPTest.php
index eecb8fb43..d32dc0213 100644
--- a/tests/modules/saml/src/Auth/Source/SPTest.php
+++ b/tests/modules/saml/src/Auth/Source/SPTest.php
@@ -14,6 +14,7 @@ use SimpleSAML\SAML2\Constants as C;
use SimpleSAML\SAML2\Exception\Protocol\{NoAvailableIDPException, NoSupportedIDPException};
use SimpleSAML\SAML2\Utils\XPath;
use SimpleSAML\SAML2\XML\saml\NameID;
+use SimpleSAML\SAML2\XML\samlp\{IDPEntry, IDPList};
use SimpleSAML\Test\Metadata\MetaDataStorageSourceTest;
use SimpleSAML\TestUtils\ClearStateTestCase;
use SimpleSAML\Test\Utils\{ExitTestException, SpTester};
@@ -497,12 +498,12 @@ class SPTest extends ClearStateTestCase
public function testSPIdpListScoping(): void
{
$ar = $this->createAuthnRequest([
- 'IDPList' => ['https://scope.example.com']
+ 'IDPList' => new IDPList([new IDPEntry('https://scope.example.com')]),
]);
$this->assertContains(
- 'https://scope.example.com',
- $ar->getIDPList()
+ (new IDPEntry('https://scope.example.com'))->toArray(),
+ ($ar->getScoping()->getIDPList()->toArray())['IDPEntry'],
);
}
@@ -516,8 +517,8 @@ class SPTest extends ClearStateTestCase
$ar = $this->createAuthnRequest([]);
$this->assertContains(
- 'https://scope.example.com',
- $ar->getIDPList()
+ (new IDPEntry('https://scope.example.com'))->toArray(),
+ ($ar->getScoping()->getIDPList()->toArray())['IDPEntry'],
);
}
@@ -541,8 +542,8 @@ class SPTest extends ClearStateTestCase
} catch (ExitTestException $e) {
['ar' => $ar] = $e->getTestResult();
$this->assertContains(
- 'https://scope.example.com',
- $ar->getIDPList()
+ (new IDPEntry('https://scope.example.com'))->toArray(),
+ ($ar->getScoping()->getIDPList()->toArray())['IDPEntry'],
);
}
}
@@ -555,7 +556,7 @@ class SPTest extends ClearStateTestCase
* @dataProvider getScopingOrders
*/
public function testSPIdpListScopingOrder(
- ?array $stateIdpList,
+ ?IDPList $stateIdpList,
?array $idpConfigArray,
?array $remoteMetadata,
string $expectedScope
@@ -583,8 +584,8 @@ class SPTest extends ClearStateTestCase
['ar' => $ar] = $e->getTestResult();
$this->assertContains(
- $expectedScope,
- $ar->getIDPList()
+ (new IDPEntry($expectedScope))->toArray(),
+ ($ar->getScoping()->getIDPList()->toArray())['IDPEntry'],
);
}
}
@@ -593,34 +594,34 @@ class SPTest extends ClearStateTestCase
{
return [
[
- 'stateIdpList' => ['https//scope1.example.com'],
- 'idpConfigArray' => ['https//scope2.example.com'],
- 'remoteMetadata' => ['https//scope3.example.com'],
- 'expectedScope' => 'https//scope1.example.com'
+ 'stateIdpList' => new IDPList([new IDPEntry('https://scope1.example.com')]),
+ 'idpConfigArray' => ['https://scope2.example.com'],
+ 'remoteMetadata' => ['https://scope3.example.com'],
+ 'expectedScope' => 'https://scope1.example.com'
],
[
'stateIdpList' => null,
- 'idpConfigArray' => ['https//scope2.example.com'],
- 'remoteMetadata' => ['https//scope3.example.com'],
- 'expectedScope' => 'https//scope3.example.com'
+ 'idpConfigArray' => ['https://scope2.example.com'],
+ 'remoteMetadata' => ['https://scope3.example.com'],
+ 'expectedScope' => 'https://scope3.example.com'
],
[
'stateIdpList' => null,
'idpConfigArray' => null,
- 'remoteMetadata' => ['https//scope3.example.com'],
- 'expectedScope' => 'https//scope3.example.com'
+ 'remoteMetadata' => ['https://scope3.example.com'],
+ 'expectedScope' => 'https://scope3.example.com'
],
[
- 'stateIdpList' => ['https//scope1.example.com'],
+ 'stateIdpList' => new IDPList([new IDPEntry('https://scope1.example.com')]),
'idpConfigArray' => null,
- 'remoteMetadata' => ['https//scope3.example.com'],
- 'expectedScope' => 'https//scope1.example.com'
+ 'remoteMetadata' => ['https://scope3.example.com'],
+ 'expectedScope' => 'https://scope1.example.com'
],
[
- 'stateIdpList' => ['https//scope1.example.com'],
- 'idpConfigArray' => ['https//scope2.example.com'],
+ 'stateIdpList' => new IDPList([new IDPEntry('https://scope1.example.com')]),
+ 'idpConfigArray' => ['https://scope2.example.com'],
'remoteMetadata' => null,
- 'expectedScope' => 'https//scope1.example.com'
+ 'expectedScope' => 'https://scope1.example.com'
]
];
}
--
GitLab