From 959aa29de0830d3f792f6b1f785158f08e67e0a9 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Sat, 15 Jan 2022 01:48:15 +0100
Subject: [PATCH] Use lib exceptions
---
docs/simplesamlphp-errorhandling.md | 6 ++---
lib/SimpleSAML/Auth/ProcessingChain.php | 6 ++---
lib/SimpleSAML/IdP.php | 4 ++--
.../multiauth/lib/Auth/Source/MultiAuth.php | 5 ++---
modules/saml/lib/Auth/Source/SP.php | 22 +++++++++----------
tests/modules/saml/lib/Auth/Source/SPTest.php | 8 +++----
6 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/docs/simplesamlphp-errorhandling.md b/docs/simplesamlphp-errorhandling.md
index 5dc6fa8ea..00199aaa1 100644
--- a/docs/simplesamlphp-errorhandling.md
+++ b/docs/simplesamlphp-errorhandling.md
@@ -14,7 +14,7 @@ This document describes the way errors and exceptions are handled in authenticat
The basic goal is to be able to throw an exception during authentication, and then have that exception transported back to the SP in a way that the SP understands.
This means that internal SimpleSAMLphp exceptions must be mapped to transport specific error codes for the various transports that are supported by SimpleSAMLphp.
-E.g.: When a `\SimpleSAML\Error\NoPassive` error is thrown by an authentication processing filter in a SAML 2.0 IdP, we want to map that exception to the `urn:oasis:names:tc:SAML:2.0:status:NoPassive` status code.
+E.g.: When a `\SAML2\Exception\Protocol\NoPassiveException` error is thrown by an authentication processing filter in a SAML 2.0 IdP, we want to map that exception to the `urn:oasis:names:tc:SAML:2.0:status:NoPassive` status code.
That status code should then be returned to the SP.
@@ -61,7 +61,7 @@ Returning specific SAML 2 errors
By default, all thrown exceptions will be converted to a generic SAML 2 error.
In some cases, you may want to convert the exception to a specific SAML 2 status code.
-For example, the `\SimpleSAML\Error\NoPassive` exception should be converted to a SAML 2 status code with the following properties:
+For example, the `\SAML2\Exception\Protocol\NoPassiveException` exception should be converted to a SAML 2 status code with the following properties:
* The top-level status code should be `urn:oasis:names:tc:SAML:2.0:status:Responder`.
* The second-level status code should be `urn:oasis:names:tc:SAML:2.0:status:NoPassive`.
@@ -97,7 +97,7 @@ This is handled by the `toException()` method in `\SimpleSAML\Module\saml\Error`
The assertion consumer script of the SAML 2 authentication source (`modules/saml2/sp/acs.php`) uses this method.
The result is that generic exceptions are thrown from that authentication source.
-For example, `NoPassive` errors will be converted back to instances of `\SimpleSAML\Error\NoPassive`.
+For example, `NoPassive` errors will be converted back to instances of `\SAML2\Exception\Protocol\NoPassiveException`.
Other protocols
diff --git a/lib/SimpleSAML/Auth/ProcessingChain.php b/lib/SimpleSAML/Auth/ProcessingChain.php
index 9ffbcccee..24f1df99a 100644
--- a/lib/SimpleSAML/Auth/ProcessingChain.php
+++ b/lib/SimpleSAML/Auth/ProcessingChain.php
@@ -268,7 +268,7 @@ class ProcessingChain
/**
* Process the given state passivly.
*
- * Modules with user interaction are expected to throw an \SimpleSAML\Module\saml\Error\NoPassive exception
+ * Modules with user interaction are expected to throw an \SAML\Exception\Protocol\NoPassiveException exception
* which are silently ignored. Exceptions of other types are passed further up the call stack.
*
* This function will only return if processing completes.
@@ -289,8 +289,8 @@ class ProcessingChain
$filter = array_shift($state[self::FILTERS_INDEX]);
try {
$filter->process($state);
- } catch (Module\saml\Error\NoPassive $e) {
- // Ignore \SimpleSAML\Module\saml\Error\NoPassive exceptions
+ } catch (SAML2\Exception\Protocol\NoPassiveException $e) {
+ // Ignore \SAML2\Exception\Protocol\NoPassiveException exceptions
}
}
}
diff --git a/lib/SimpleSAML/IdP.php b/lib/SimpleSAML/IdP.php
index eab59467a..884d93430 100644
--- a/lib/SimpleSAML/IdP.php
+++ b/lib/SimpleSAML/IdP.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML;
use SAML2\Constants;
+use SAML2\Exception\Protocol\NoPassiveException;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
use SimpleSAML\Configuration;
@@ -13,7 +14,6 @@ use SimpleSAML\IdP\LogoutHandlerInterface;
use SimpleSAML\IdP\TraditionalLogoutHandler;
use SimpleSAML\Error;
use SimpleSAML\Metadata\MetaDataStorageHandler;
-use SimpleSAML\Module\saml\Error\NoPassive;
use SimpleSAML\Utils;
/**
@@ -339,7 +339,7 @@ class IdP
private function authenticate(array &$state): void
{
if (isset($state['isPassive']) && (bool) $state['isPassive']) {
- throw new NoPassive(Constants::STATUS_RESPONDER, 'Passive authentication not supported.');
+ throw new NoPassiveException(Constants::STATUS_RESPONDER . ': Passive authentication not supported.');
}
$this->authSource->login($state);
diff --git a/modules/multiauth/lib/Auth/Source/MultiAuth.php b/modules/multiauth/lib/Auth/Source/MultiAuth.php
index 906b41736..8af4fa037 100644
--- a/modules/multiauth/lib/Auth/Source/MultiAuth.php
+++ b/modules/multiauth/lib/Auth/Source/MultiAuth.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace SimpleSAML\Module\multiauth\Auth\Source;
use SAML2\Constants;
+use SAML2\Exception\Protocol\NoAuthnContextException;
use Exception;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Auth;
@@ -14,7 +15,6 @@ use SimpleSAML\HTTP\RunnableResponse;
use SimpleSAML\Module;
use SimpleSAML\Session;
use SimpleSAML\Utils;
-use SimpleSAML\Module\saml\Error\NoAuthnContext;
/**
* Authentication source which let the user chooses among a list of
@@ -173,8 +173,7 @@ class MultiAuth extends Auth\Source
$number_of_sources = count($new_sources);
if ($number_of_sources === 0) {
- throw new NoAuthnContext(
- Constants::STATUS_RESPONDER,
+ throw new NoAuthnContextException(
'No authentication sources exist for the requested AuthnContextClassRefs: ' . implode(', ', $refs)
);
} elseif ($number_of_sources === 1) {
diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php
index 5e53b1eac..2d079db99 100644
--- a/modules/saml/lib/Auth/Source/SP.php
+++ b/modules/saml/lib/Auth/Source/SP.php
@@ -7,6 +7,9 @@ namespace SimpleSAML\Module\saml\Auth\Source;
use SAML2\AuthnRequest;
use SAML2\Binding;
use SAML2\Constants;
+use SAML2\Exception\Protocol\NoAvailableIDPException;
+use SAML2\Exception\Protocol\NoPassiveException;
+use SAML2\Exception\Protocol\NoSupportedIDPException;
use SAML2\LogoutRequest;
use SAML2\XML\saml\NameID;
use SimpleSAML\Assert\Assert;
@@ -732,16 +735,14 @@ class SP extends \SimpleSAML\Auth\Source
if (empty($matchedEntities)) {
// all requested IdPs are unknown
- throw new Module\saml\Error\NoSupportedIDP(
- Constants::STATUS_REQUESTER,
+ throw new NoSupportedIDPException(
'None of the IdPs requested are supported by this proxy.'
);
}
if (!is_null($idp) && !array_key_exists($idp, $matchedEntities)) {
// the IdP is enforced but not in the IDPList
- throw new Module\saml\Error\NoAvailableIDP(
- Constants::STATUS_REQUESTER,
+ throw new NoAvailableIDPException(
'None of the IdPs requested are available to this proxy.'
);
}
@@ -802,8 +803,7 @@ class SP extends \SimpleSAML\Auth\Source
if (empty($intersection)) {
// all requested IdPs are unknown
- throw new Module\saml\Error\NoSupportedIDP(
- Constants::STATUS_REQUESTER,
+ throw new NoSupportedIDP(
'None of the IdPs requested are supported by this proxy.'
);
}
@@ -815,8 +815,7 @@ class SP extends \SimpleSAML\Auth\Source
*/
if (!is_null($this->idp) && !in_array($this->idp, $intersection, true)) {
// an IdP is enforced but not requested
- throw new Module\saml\Error\NoAvailableIDP(
- Constants::STATUS_REQUESTER,
+ throw new NoAvailableIDPException(
'None of the IdPs requested are available to this proxy.'
);
}
@@ -855,7 +854,7 @@ class SP extends \SimpleSAML\Auth\Source
* - 'core:IdP': the identifier of the local IdP.
* - 'SPMetadata': an array with the metadata of this local SP.
*
- * @throws \SimpleSAML\Module\saml\Error\NoPassive In case the authentication request was passive.
+ * @throws \SAML2\Exception\Protocol\NoPassiveException In case the authentication request was passive.
*/
public static function askForIdPChange(array &$state): void
{
@@ -866,9 +865,8 @@ class SP extends \SimpleSAML\Auth\Source
if (isset($state['isPassive']) && (bool) $state['isPassive']) {
// passive request, we cannot authenticate the user
- throw new Module\saml\Error\NoPassive(
- Constants::STATUS_REQUESTER,
- 'Reauthentication required'
+ throw new NoPassiveException(
+ Constants::STATUS_REQUESTER . ': Reauthentication required'
);
}
diff --git a/tests/modules/saml/lib/Auth/Source/SPTest.php b/tests/modules/saml/lib/Auth/Source/SPTest.php
index 1bb6ab700..0387ae082 100644
--- a/tests/modules/saml/lib/Auth/Source/SPTest.php
+++ b/tests/modules/saml/lib/Auth/Source/SPTest.php
@@ -8,13 +8,13 @@ use InvalidArgumentException;
use PHPUnit\Framework\TestCase;
use SAML2\AuthnRequest;
use SAML2\Constants;
+use SAML2\Exception\Protocol\NoAvailableIDPException;
+use SAML2\Exception\Protocol\NoSupportedIDPException;
use SAML2\LogoutRequest;
use SAML2\Utils;
use SAML2\XML\saml\NameID;
use SimpleSAML\Configuration;
use SimpleSAML\Error\Exception;
-use SimpleSAML\Module\saml\Error\NoAvailableIDP;
-use SimpleSAML\Module\saml\Error\NoSupportedIDP;
use SimpleSAML\Test\Metadata\MetaDataStorageSourceTest;
use SimpleSAML\TestUtils\ClearStateTestCase;
use SimpleSAML\Test\Utils\ExitTestException;
@@ -287,7 +287,7 @@ class SPTest extends ClearStateTestCase
*/
public function testIdpListWithNoMatchingMetadata(): void
{
- $this->expectException(NoSupportedIDP::class);
+ $this->expectException(NoSupportedIDPException::class);
$state = [
'saml:IDPList' => ['noSuchIdp']
];
@@ -304,7 +304,7 @@ class SPTest extends ClearStateTestCase
*/
public function testIdpListWithExplicitIdpNotMatch(): void
{
- $this->expectException(NoAvailableIDP::class);
+ $this->expectException(NoAvailableIDPException::class);
$entityId = "https://example.com";
$xml = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId);
$c = [
--
GitLab