SAML-Toolkits / python3-saml

MIT License
682 stars 304 forks source link

Implementing returning failure in LogoutResponse #287

Closed mateuszmandera closed 2 years ago

mateuszmandera commented 2 years ago

Is there a recommended way of implementing LogoutResponse for communicating failure or any plans for bulding that functionality into the library? Currently the class appears to only support success responses:

    def build(self, in_response_to):
        """
        Creates a Logout Response object.
        :param in_response_to: InResponseTo value for the Logout Response.
        :type in_response_to: string
        """
        sp_data = self._settings.get_sp_data()

        self.id = self._generate_request_id()

        issue_instant = OneLogin_Saml2_Utils.parse_time_to_SAML(OneLogin_Saml2_Utils.now())

        logout_response = OneLogin_Saml2_Templates.LOGOUT_RESPONSE % \
            {
                'id': self.id,
                'issue_instant': issue_instant,
                'destination': self._settings.get_idp_slo_response_url(),
                'in_response_to': in_response_to,
                'entity_id': sp_data['entityId'],
                'status': "urn:oasis:names:tc:SAML:2.0:status:Success"
            }

        self._logout_response = logout_response

So it seems to me that the only way to make failure responses happen is to subclass it and override build to allow using different status codes and then generate, sign etc. the LogoutResponse in the application code by following the pattern of how OneLogin_Saml2_Auth.process_slo does it - though perhaps I'm missing a better way of achieving that.

This is needed due to the SAML spec requiring failure responses: https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf

The recipient of a message MUST respond with a message, of type StatusResponseType, with no additional content specified.

pitbulk commented 2 years ago

I think its ok to accept an status at the build method, and by default set the Success.

Do you have a PR for that? (please include unit tests)

mateuszmandera commented 2 years ago

@pitbulk I don't have a PR, but I'll try to find the time to work on one.

mateuszmandera commented 2 years ago

@pitbulk Opened #288 for this issue