Logout fails because SPEntityID is not in state

[SrGesus]

When the logouttype is iframe (didn't test traditional), and I try to abort consent while i am also already logged onto a different service, instead of the standard core/logout-iframe i get the error

SimpleSAML\Error\Error: UNHANDLEDEXCEPTION

Backtrace:
2 src/SimpleSAML/Error/ExceptionHandler.php:47 (SimpleSAML\Error\ExceptionHandler::customExceptionHandler)
1 vendor/symfony/error-handler/ErrorHandler.php:538 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
0 [builtin] (N/A)
Caused by: TypeError: SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig(): Argument #1 ($entityId) must be of type string, null given, called in [simplesamlphpdir]/modules/core/src/Controller/Logout.php on line 343
Backtrace:
6 src/SimpleSAML/Metadata/MetaDataStorageHandler.php:372 (SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig)
5 modules/core/src/Controller/Logout.php:343 (SimpleSAML\Module\core\Controller\Logout::logoutIframe)
4 vendor/symfony/http-kernel/HttpKernel.php:183 (Symfony\Component\HttpKernel\HttpKernel::handleRaw)
3 vendor/symfony/http-kernel/HttpKernel.php:76 (Symfony\Component\HttpKernel\HttpKernel::handle)
2 vendor/symfony/http-kernel/Kernel.php:193 (Symfony\Component\HttpKernel\Kernel::handle)
1 src/SimpleSAML/Module.php:240 (SimpleSAML\Module::process)
0 public/module.php:17 (N/A)

This seems to be because in simplesaml's logout iframe if there's a 'core:TerminatedAssocId' in state it is assumed there's also a 'saml:SPEntityId'
https://github.com/simplesamlphp/simplesamlphp/blob/e5ce356c42789cdd4921e8248d7d2a11b7c159a3/modules/core/src/Controller/Logout.php#L347
this seems to be the case within simplesaml, where the only time core:TerminatedAssocId is set is in receiveLogoutMessage, which is the only time handleLogoutRequest is called with a non-null assocId when SPEntityId is also set
https://github.com/simplesamlphp/simplesamlphp/blob/2a1651b8179b590e064eaa085fdaa3d92a8fc298/modules/saml/src/IdP/SAML2.php#L689-L694
but in the consent module handleLogoutRequest is called with an assocId (stateId) even if saml:SPEntityId is not set in the state

simplesamlphp-module-consent/src/Controller/ConsentController.php

Lines 301 to 304 in cd6f3b5

	$state['Responder'] = ['\SimpleSAML\Module\consent\Logout', 'postLogout'];
	$idp = IdP::getByState($state);
	return new RunnableResponse([$idp, 'handleLogoutRequest'], [&$state, $stateId]);

[SrGesus]

This worked as a quick fix, I have no idea whether it's sensible.

diff --git a/src/Controller/ConsentController.php b/src/Controller/ConsentController.php
index 9d02e4a..db8e6dd 100644
--- a/src/Controller/ConsentController.php
+++ b/src/Controller/ConsentController.php
@@ -300,6 +300,13 @@ class ConsentController
         }
         $state['Responder'] = ['\SimpleSAML\Module\consent\Logout', 'postLogout'];

+        if (array_key_exists('core:SP', $state)) {
+            $spentityid = $state['core:SP'];
+        } elseif (array_key_exists('saml:sp:State', $state)) {
+            $spentityid = $state['saml:sp:State']['core:SP'];
+        }
+        $state['saml:SPEntityId'] = $spentityid;
+
         $idp = IdP::getByState($state);
         return new RunnableResponse([$idp, 'handleLogoutRequest'], [&$state, $stateId]);
     }

Essentially it's what is in getconsent() here,

simplesamlphp-module-consent/src/Controller/ConsentController.php

Lines 101 to 109 in cd6f3b5

	if (is_null($state)) {
	throw new Error\NoState();
	} elseif (array_key_exists('core:SP', $state)) {
	$spentityid = $state['core:SP'];
	} elseif (array_key_exists('saml:sp:State', $state)) {
	$spentityid = $state['saml:sp:State']['core:SP'];
	} else {
	$spentityid = 'UNKNOWN';
	}

I just took unknown out because it will still fail at the same place it would before (i.e https://github.com/simplesamlphp/simplesamlphp/blob/e5ce356c42789cdd4921e8248d7d2a11b7c159a3/modules/core/src/Controller/Logout.php#L347) with a MetadataNotFound

[monkeyiq]

In Controller/Logout.php since it already has

$associations = $idp->getAssociations();

If there is a core:TerminatedAssocId then that should be in $associations.

So perhaps:

$assocId = $state['core:TerminatedAssocId'];
$association = $associations[$assocId];

In logoutIframeDone it shorts out if if (!isset($associations[$assocId])) as compledted.

Then, looking at SAML2.php:sendResponse

$association = [
            'id'                => 'saml:' . $spEntityId,
            'Handler'           => '\SimpleSAML\Module\saml\IdP\SAML2',
            'Expires'           => $assertion->getSessionNotOnOrAfter(),
            'saml:entityID'     => $spEntityId,
            'saml:NameID'       => $state['saml:idp:NameID'],
            'saml:SessionIndex' => $assertion->getSessionIndex(),
        ];

For the offending line in Controller/Logout.php we might be able to directly reference the specific association
from the TerminatedAssocId $association['saml:entityID'] ?

https://github.com/simplesamlphp/simplesamlphp/blob/e5ce356c42789cdd4921e8248d7d2a11b7c159a3/modules/core/src/Controller/Logout.php#L347

- $terminated = $mdh->getMetaDataConfig($state['saml:SPEntityId'], $mdset)->toArray();
+ $assocId = $state['core:TerminatedAssocId'];
+ $association = $associations[$assocId];
+ $terminated = $mdh->getMetaDataConfig($association['saml:entityID'], $mdset)->toArray();