OpenConext / OpenConext-engineblock

OpenConext SAML 2.0 IdP/SP Gateway
14 stars 22 forks source link

On error, show \'back to sp\' button #1720

Open phavekes opened 3 days ago

phavekes commented 3 days ago

This issue is imported from pivotal - Originaly created at Feb 16, 2021 by Thijs Kinkhorst

In the existing error screens related to the SFO endpoint, add a button that says "return to %name of sp%". Pressing this button will send a SAML Status response to the SP. The SAML status code, subcode and message are copied from those received from SA GW and known to the error page.

phavekes commented 3 days ago

We do not know whether SPs support the error response. But atm users get stuck with us, rather than giving the SP a chance to do something different. The above might mean that we need to make this behavior configurable. (Koen Cornelis - Mar 24, 2021)

phavekes commented 3 days ago

We should not invest copious amounts of effort to get this to work. This is a proof of concept that might be dismanteled at a later point if SPs do not supportAuthnFailed saml response (Michiel Kodde - Mar 24, 2021)

phavekes commented 3 days ago

What we already know in the FeedbackController on account of data being registered on the FeedbackInfo session:

StatusCode: example: Responder/NoAuthnContext
StatusMessage: example "(No message provided)"
RequestId: example "6077e6f25ef13"
ServiceProvider EnittyId: example "https://engine.vm.openconext.org/functional-testing/SSO-SP/metadata"
IdentityProvider: A IdP value object, Note this is the Engine StepUp IdP metadata

What we need for an error response:

        $response = new SAMLResponse();
        $response->setDestination($spAcsLocation);
        $response->setIssuer($idpEntityId);
        $response->setIssueInstant((new DateTime(\'now\'))->getTimestamp());
        $response->setInResponseTo($originalRequestId);
        $response->setStatus([
            \'Code\' => Constants::STATUS_RESPONDER,
            \'SubCode\' => Constants::STATUS_AUTHN_FAILED,
            \'Message\' => $message
        ]);

Conclusion, all data is available for a successful SAMLResponseFailed post back. (Michiel Kodde - Apr 15, 2021)

phavekes commented 3 days ago

Only one small challenge is required: load the SP ACS location based on the SP EntityID. This will introduce a new dependency and increase coupling on the FeedbackController. As we will need to use the SP Repository to fetch the ACS location. We may be able to do this before throwing the exception and task the EB StepUp ACS with this challenge. (Michiel Kodde - Apr 15, 2021)

phavekes commented 3 days ago

Regarding the Back to SP button. What about this?

<>

Alternatively we can add a button underneath the error description. (Michiel Kodde - Apr 15, 2021)

phavekes commented 3 days ago

Seems like a great initial place, we can always move it based on the experiences (Thijs Kinkhorst - Apr 15, 2021)

phavekes commented 3 days ago
Pff, takes some effort getting this thing finished.. 

@thijskh Before wrapping things up. I\'d like to show you a back button with a longer SP name. It will ultimately increase the footer size. I rather like it. I\'m going to ask @Koenekijn to ensure the other buttons also grow/shrink nicely. (Michiel Kodde - Apr 20, 2021)

phavekes commented 3 days ago

Yes, looks good to me (Thijs Kinkhorst - Apr 20, 2021)

phavekes commented 3 days ago

This is sent back from EB to the SP:

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                ID="_8250f088375471b6e1abe57624bc2d2d9e200eca67"
                Version="2.0"
                IssueInstant="2021-05-11T10:00:19Z"
                Destination="https://thki-sid.pt-48.utr.surfcloud.nl/ssp/module.php/saml/sp/saml2-acs.php/default-sp"
                InResponseTo="609a55b337845"
                >
    <saml:Issuer>https://sa-gw.test2.surfconext.nl/second-factor-only/metadata</saml:Issuer>
    <samlp:Status>
        <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Responder">
            <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:AuthnFailed" />
        </samlp:StatusCode>
        <samlp:StatusMessage>(No message provided)</samlp:StatusMessage>
    </samlp:Status>
</samlp:Response>

The Issuer is incorrect, EB is (from the view of this SP) the only entity it knows so it now rejects the message with unknown issuer. The message is issued by EB.

The issuer needs to be EB\'s IdP entityID just like for any other outgoing assertion to the SP. (Thijs Kinkhorst - May 11, 2021)

phavekes commented 3 days ago
It\'s actually a bit more complex. What needs to be changed?
phavekes commented 3 days ago

For future reference, we decided to temporarily disable the back to sp link untill the change described by Thijs is implemented. The disabling is handled in: https://github.com/OpenConext/OpenConext-engineblock/pull/1143 (Michiel Kodde - Jun 3, 2021)

phavekes commented 3 days ago

The three final points of feedback specified above have been addressed in: https://github.com/OpenConext/OpenConext-engineblock/pull/1240 (Michiel Kodde - May 3, 2023)