Configured IDPList is not enforced after Discovery response

Inizia da modules/saml/src/Controller/ServiceProvider.php, in particolare da ServiceProvider::discoResponse(), dove lo state ripristinato e l’idpentityid inviato vengono passati a startSSO. Traccia come viene popolato state['saml:IDPList'] e confrontalo con l’enforcement esistente di Discovery. Il lavoro è completato quando un IdP al di fuori dell’elenco configurato viene rifiutato prima che l’autenticazione prosegua, mentre un IdP consentito continua a funzionare.

When SimpleSAMLphp 2.4.8 is used as a SAML proxy and an IDPList is configured for a Service Provider, the Discovery Service correctly limits the displayed Identity Providers.

However, after the Discovery response is processed, the selected Identity Provider (idpentityid) is accepted without being validated against the configured IDPList.

As a result, a client can modify the Discovery request and authenticate against an Identity Provider that is not contained in the configured IDPList.

Affected versions

Reproduced on:

• 2.4.2
• 2.4.8

Steps to reproduce

1. Configure an SP with the following IDPList:

• https://idp-a.example.org
• https://idp-b.example.org

2. Ensure another IdP exists in the proxy metadata, for example: https://idp-c.example.org
   where idp-c is not part of the configured IDPList.
3. Start authentication.
4. The Discovery Service correctly displays only:
   idp-a
   idp-b
5. Inspect or intercept the Discovery request and modify one of the generated IdP selection parameters.
   Example:
   Original: /module.php/saml/disco?...&idp_https://idp-a.example.org=
   Modified: /module.php/saml/disco?...&idp_https://idp-c.example.org=
6. Authentication succeeds against idp-c, although it is not contained in the configured IDPList.

Actual behaviour

The Discovery UI correctly filters the displayed Identity Providers.

However, after the Discovery response is processed, the selected idpentityid is accepted without being validated against the configured IDPList.

As a result, the configured IDPList is effectively enforced only by the Discovery UI and not by the server. Since client-side filtering cannot be considered a security control, the selected IdP should be validated server-side before continuing the authentication flow.

The Discovery response processing eventually invokes:
return new RunnableResponse([$source, 'startSSO'], [$idpEntityId, $state]);
without validating $idpEntityId against $state['saml:IDPList'].

Expected behaviour

The selected Identity Provider should be validated against the configured IDPList.

If the selected IdP is not part of the configured list, authentication should fail.

Additional observation

The Discovery Service already correctly enforces the configured IDPList in another scenario:

If a user already has an active session with an Identity Provider that is not part of the configured IDPList, the Discovery Service correctly detects this and does not automatically reuse that session. Instead, the user is prompted to authenticate using one of the Identity Providers contained in the configured IDPList.

This indicates that the configured IDPList is already considered during Discovery. However, the same restriction is not enforced when processing the user-selected idpentityid in the Discovery response, resulting in inconsistent behavior.

Impact

A Service Provider configured to trust only a specific subset of Identity Providers can still be authenticated through another Identity Provider configured on the proxy but not permitted by the Service Provider's configured IDPList.

This effectively bypasses the configured IDPList restriction. This allows authentication through an Identity Provider that is known to the proxy but explicitly excluded by the Service Provider's configured IDPList.

Observed implementation

During debugging, the following code path appears to be involved:
File: modules/saml/src/Controller/ServiceProvider.php
Method: ServiceProvider::discoResponse()
The controller restores the authentication state, reads the user supplied idpentityid, and immediately calls:
return new RunnableResponse([$source, 'startSSO'], [$idpEntityId, $state]);
without validating the selected IdP against:
$state['saml:IDPList']
Adding a server-side validation at this point prevents the observed bypass.

Documentation inconsistency

According to the SAML 2.0 Core specification, IDPList specifies the identity providers trusted by the requester. The specification describes IDPList as scoping information for the authentication request. The current implementation correctly uses this information to filter the Discovery UI, but does not enforce the same restriction when processing the Discovery response. As a result, an IdP that is not contained in the configured IDPList can still be selected by modifying the request. This behavior also appears inconsistent with the existing implementation, which already stores and restores state['saml:IDPList'] across the Discovery flow, indicating that the information is intentionally preserved but not enforced before continuing the authentication process.

The SimpleSAMLphp documentation describes IDPList as:
"Only relevant if you are a proxy/bridge and want to limit the IdPs on your proxy that this SP can use."

Based on both descriptions, it is reasonable to expect IDPList to be enforced server-side and not only used to filter the Discovery UI.

The current implementation appears inconsistent:

• Discovery correctly limits the displayed IdPs.
• The selected IdP is not validated against the configured IDPList after the Discovery response.

Additional debugging information

During debugging I added the following validation as a proof of concept in ServiceProvider::discoResponse() before invoking startSSO():

if (
    isset($idpEntityId) && isset($state['saml:IDPList']) && !empty($state['saml:IDPList']) && !in_array($idpEntityId, $state['saml:IDPList'], true)
) {
    throw new NoAvailableIDPException(
        sprintf(
            'Identity Provider "%s" is not allowed for this Service Provider.',
            $idpEntityId
        )
    );
}

This validation was only added for debugging purposes and was not intended as a proposed upstream implementation. With this additional validation in place, the bypass is no longer possible.

I am not suggesting that this is necessarily the correct upstream fix. The proof-of-concept validation was only used during debugging to verify whether server-side enforcement of state['saml:IDPList'] prevents the bypass. It does, which demonstrates that the selected IdP is currently not validated against the configured IDPList after the Discovery response.

The maintainers may prefer implementing the validation elsewhere in the authentication flow, but the above test demonstrates the missing enforcement.