From 630a854700107b865c3484271606e72cf8dcc53d Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Fri, 14 Feb 2020 21:56:13 +0100
Subject: [PATCH] Upgrade saml2 library dep (#1280)
* Upgrade saml2-library to v4
* Stop considering Issuer-strings
* Stop considering NameID-arrays
---
.travis.yml | 8 -----
composer.json | 2 +-
lib/SimpleSAML/Metadata/SAMLParser.php | 10 ++-----
modules/admin/lib/Controller/Test.php | 41 ++++++++++----------------
modules/saml/lib/IdP/SAML2.php | 10 +------
modules/saml/lib/SP/LogoutStore.php | 10 -------
modules/saml/www/sp/saml2-acs.php | 10 +------
7 files changed, 21 insertions(+), 70 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index c2b31acd5..19c197da2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -47,14 +47,6 @@ jobs:
# Pre-conditions stage #
##########################
- - stage: pre-conditions
- php: 7.2
- env: Syntax check PHP
- before_script:
- - composer install
- script:
- - vendor/bin/check-syntax-php.sh
-
- stage: pre-conditions
php: 7.2
env: Syntax check PHP
diff --git a/composer.json b/composer.json
index a8acfa485..1bb7fc0a8 100644
--- a/composer.json
+++ b/composer.json
@@ -45,7 +45,7 @@
"gettext/gettext": "^4.6",
"phpmailer/phpmailer": "^6.0",
"robrichards/xmlseclibs": "^3.0.4",
- "simplesamlphp/saml2": "^3.4 || ^4.0",
+ "simplesamlphp/saml2": "^4.1",
"simplesamlphp/simplesamlphp-module-adfs": "^0.9",
"simplesamlphp/simplesamlphp-module-authcrypt": "^0.9",
"simplesamlphp/simplesamlphp-module-authfacebook": "^0.9",
diff --git a/lib/SimpleSAML/Metadata/SAMLParser.php b/lib/SimpleSAML/Metadata/SAMLParser.php
index 64c0f02bc..73d64c941 100644
--- a/lib/SimpleSAML/Metadata/SAMLParser.php
+++ b/lib/SimpleSAML/Metadata/SAMLParser.php
@@ -1118,19 +1118,13 @@ class SAMLParser
$keywords = $uiItem->getKeywords();
/** @psalm-var string|null $language */
$language = $uiItem->getLanguage();
- if (($keywords === []) || ($language === null)) {
+ if (($keywords === [])) {
continue;
}
$ret['UIInfo']['Keywords'][$language] = $keywords;
}
foreach ($e->getLogo() as $uiItem) {
- /** @psalm-suppress TypeDoesNotContainNull Remove in SSP 2.0 */
- if (
- !($uiItem instanceof Logo)
- || ($uiItem->getUrl() === null)
- || ($uiItem->getHeight() === null)
- || ($uiItem->getWidth() === null)
- ) {
+ if (!($uiItem instanceof Logo)) {
continue;
}
$logo = [
diff --git a/modules/admin/lib/Controller/Test.php b/modules/admin/lib/Controller/Test.php
index c03018d56..06b4baa0f 100644
--- a/modules/admin/lib/Controller/Test.php
+++ b/modules/admin/lib/Controller/Test.php
@@ -122,31 +122,22 @@ class Test
{
$translator = $t->getTranslator();
$result = '';
-
- /** @psalm-suppress TypeDoesNotContainNull Remove if-case in 2.0 */
- if ($nameId->getValue() === null) {
- $list = ["NameID" => [$translator->t('{status:subject_notset}')]];
- /** @var string $notset */
- $notset = $translator->t('{status:subject_notset}');
- $result .= "<p>NameID: <span class=\"notset\">" . $notset . "</span></p>";
- } else {
- $list = [
- "NameId" => [$nameId->getValue()],
- ];
- if ($nameId->getFormat() !== null) {
- /** @var string $format */
- $format = $translator->t('{status:subject_format}');
- $list[$format] = [$nameId->getFormat()];
- }
- if ($nameId->getNameQualifier() !== null) {
- $list['NameQualifier'] = [$nameId->getNameQualifier()];
- }
- if ($nameId->getSPNameQualifier() !== null) {
- $list['SPNameQualifier'] = [$nameId->getSPNameQualifier()];
- }
- if ($nameId->getSPProvidedID() !== null) {
- $list['SPProvidedID'] = [$nameId->getSPProvidedID()];
- }
+ $list = [
+ "NameId" => [$nameId->getValue()],
+ ];
+ if ($nameId->getFormat() !== null) {
+ /** @var string $format */
+ $format = $translator->t('{status:subject_format}');
+ $list[$format] = [$nameId->getFormat()];
+ }
+ if ($nameId->getNameQualifier() !== null) {
+ $list['NameQualifier'] = [$nameId->getNameQualifier()];
+ }
+ if ($nameId->getSPNameQualifier() !== null) {
+ $list['SPNameQualifier'] = [$nameId->getSPNameQualifier()];
+ }
+ if ($nameId->getSPProvidedID() !== null) {
+ $list['SPProvidedID'] = [$nameId->getSPProvidedID()];
}
return $result . $this->getAttributesHTML($t, $list, '');
}
diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php
index c07cfb4f0..0e654d133 100644
--- a/modules/saml/lib/IdP/SAML2.php
+++ b/modules/saml/lib/IdP/SAML2.php
@@ -368,16 +368,8 @@ class SAML2
throw new Error\BadRequest(
'Received message on authentication request endpoint without issuer.'
);
- } elseif ($issuer instanceof Issuer) {
- /** @psalm-var string|null $spEntityId */
- $spEntityId = $issuer->getValue();
- if ($spEntityId === null) {
- /* Without an issuer we have no way to respond to the message. */
- throw new Error\BadRequest('Received message on logout endpoint without issuer.');
- }
- } else { // we got a string, old case
- $spEntityId = $issuer;
}
+ $spEntityId = $issuer->getValue();
$spMetadata = $metadata->getMetaDataConfig($spEntityId, 'saml20-sp-remote');
\SimpleSAML\Module\saml\Message::validateMessage($spMetadata, $idpMetadata, $request);
diff --git a/modules/saml/lib/SP/LogoutStore.php b/modules/saml/lib/SP/LogoutStore.php
index c9c1ae138..fd6466694 100644
--- a/modules/saml/lib/SP/LogoutStore.php
+++ b/modules/saml/lib/SP/LogoutStore.php
@@ -357,11 +357,6 @@ class LogoutStore
}
// serialize and anonymize the NameID
- // TODO: remove this conditional statement
- if (is_array($nameId)) {
- /** @psalm-suppress UndefinedMethod */
- $nameId = NameID::fromArray($nameId);
- }
$strNameId = serialize($nameId);
$strNameId = sha1($strNameId);
@@ -400,11 +395,6 @@ class LogoutStore
}
// serialize and anonymize the NameID
- // TODO: remove this conditional statement
- if (is_array($nameId)) {
- /** @psalm-suppress UndefinedMethod */
- $nameId = NameID::fromArray($nameId);
- }
$strNameId = serialize($nameId);
$strNameId = sha1($strNameId);
diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php
index 7d5c39a79..3d1db4088 100644
--- a/modules/saml/www/sp/saml2-acs.php
+++ b/modules/saml/www/sp/saml2-acs.php
@@ -54,15 +54,7 @@ if ($issuer === null) {
throw new Exception('Missing <saml:Issuer> in message delivered to AssertionConsumerService.');
}
}
-
-if ($issuer instanceof \SAML2\XML\saml\Issuer) {
- /** @psalm-var string|null $issuer */
- $issuer = $issuer->getValue();
- if ($issuer === null) {
- // no issuer found in the assertions
- throw new Exception('Missing <saml:Issuer> in message delivered to AssertionConsumerService.');
- }
-}
+$issuer = $issuer->getValue();
$session = \SimpleSAML\Session::getSessionFromRequest();
$prevAuth = $session->getAuthData($sourceId, 'saml:sp:prevAuth');
--
GitLab