Skip to content
Snippets Groups Projects
Commit e5aa6821 authored by Jaime Perez Crespo's avatar Jaime Perez Crespo
Browse files

Bugfix

No data about previous authentication is stored if authentication was not started at the SP (IdP-first flow). That makes the replay protection measures fail, leading to an ugly exception show to the user. Fix that.

Additionally, give precedence to the RelayState configured in the local metadata, as the one received together with the SAML response may not even be an URL.

This resolves #230.
parent 3fd76a6b
No related branches found
No related tags found
No related merge requests found
...@@ -60,7 +60,12 @@ if ($prevAuth !== NULL && $prevAuth['id'] === $response->getId() && $prevAuth['i ...@@ -60,7 +60,12 @@ if ($prevAuth !== NULL && $prevAuth['id'] === $response->getId() && $prevAuth['i
* instead of displaying a confusing error message. * instead of displaying a confusing error message.
*/ */
SimpleSAML_Logger::info('Duplicate SAML 2 response detected - ignoring the response and redirecting the user to the correct page.'); SimpleSAML_Logger::info('Duplicate SAML 2 response detected - ignoring the response and redirecting the user to the correct page.');
\SimpleSAML\Utils\HTTP::redirectTrustedURL($prevAuth['redirect']); if (isset($prevAuth['redirect'])) {
\SimpleSAML\Utils\HTTP::redirectTrustedURL($prevAuth['redirect']);
}
SimpleSAML_Logger::info('No RelayState or ReturnURL available, cannot redirect.');
throw new SimpleSAML_Error_Exception('Duplicate assertion received.');
} }
$idpMetadata = array(); $idpMetadata = array();
...@@ -90,7 +95,11 @@ if (!empty($stateId)) { ...@@ -90,7 +95,11 @@ if (!empty($stateId)) {
$state = array( $state = array(
'saml:sp:isUnsolicited' => TRUE, 'saml:sp:isUnsolicited' => TRUE,
'saml:sp:AuthId' => $sourceId, 'saml:sp:AuthId' => $sourceId,
'saml:sp:RelayState' => \SimpleSAML\Utils\HTTP::checkURLAllowed($response->getRelayState()), 'saml:sp:RelayState' => \SimpleSAML\Utils\HTTP::checkURLAllowed($spMetadata->getString(
'RelayState',
$response->getRelayState()
)
),
); );
} }
...@@ -196,17 +205,17 @@ if ($expire !== NULL) { ...@@ -196,17 +205,17 @@ if ($expire !== NULL) {
$state['Expire'] = $expire; $state['Expire'] = $expire;
} }
// note some information about the authentication, in case we receive the same response again
$state['saml:sp:prevAuth'] = array(
'id' => $response->getId(),
'issuer' => $idp,
);
if (isset($state['SimpleSAML_Auth_Default.ReturnURL'])) { if (isset($state['SimpleSAML_Auth_Default.ReturnURL'])) {
/* Just note some information about the authentication, in case we receive the $state['saml:sp:prevAuth']['redirect'] = $state['SimpleSAML_Auth_Default.ReturnURL'];
* same response again. } elseif (isset($state['saml:sp:RelayState'])) {
*/ $state['saml:sp:prevAuth']['redirect'] = $state['saml:sp:RelayState'];
$state['saml:sp:prevAuth'] = array(
'id' => $response->getId(),
'issuer' => $idp,
'redirect' => $state['SimpleSAML_Auth_Default.ReturnURL'],
);
$state['PersistentAuthData'][] = 'saml:sp:prevAuth';
} }
$state['PersistentAuthData'][] = 'saml:sp:prevAuth';
$source->handleResponse($state, $idp, $attributes); $source->handleResponse($state, $idp, $attributes);
assert('FALSE'); assert('FALSE');
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment