Reloading Saml2RelyingPartyRegistrations with help of Saml2RelyingPartyRegistrationConfiguration

[dawi]

The spring-security-saml extension provides a HTTPMetadataProvider which is able to automatically refresh SAML metadata in configurable intervals. It would be nice if this feature would also be supported by spring security, but I'm afraid that this feature is out of scope of this library (spring-projects/spring-security#9134).

This is ok, because this feature can be realized fairly simple by implementing a custom RelyingPartyRegistrationRepository.

However the user has to write a lot of code that is already part of Spring Boot and Spring Security internally (e.g. reading certificates, thinking about configuration properties, ...).

It would be nice if a user could just use the existing Spring Boot SAML configuration like this (example on github):

public ReloadingRelyingPartyRegistrationRepository(Saml2RelyingPartyProperties properties) {
    this.properties = properties;
    refreshRelyingPartyRegistrations();
}

@Scheduled(fixedDelayString = "${metadata-refresh-interval}", initialDelay = 10_000)
private void refreshRelyingPartyRegistrations() {

    LOGGER.debug("refreshRelyingPartyRegistrations");

    properties.getRegistration().forEach((registrationId, registrationProperties) -> {

        try {
            registrations.put(registrationId, asRegistration(registrationId, registrationProperties));
        }
        catch (Exception e) {
            LOGGER.warn("Could not refresh RelyingPartyRegistration configuration.", e);
        }
    });
}

My question is, would you consider making the private asRegistration(...) method from Saml2RelyingPartyRegistrationConfiguration in some form accessible to users, so that they don't have to repeat that code? Maybe not in Saml2RelyingPartyRegistrationConfiguration but in a separate utility class?

[dawi]

@philwebb Just curious, are there any news regarding this issue? :)

[philwebb]

Not much I'm afraid. We targeted it to 2.x which means it's a valid issue and something we'd like to fix, we just haven't had the time yet.

[dawi]

@philwebb How likely is it that this will be included in a 2.x release?
With guidance I can maybe help with a pull request.

[philwebb]

A pull-request would be most welcome. There a few options for fixing this, and I'm not sure which would be best.

We could create a RelyingPartyRegistrationProperyMapper class that contains the existing asRegistration method. This would allow us to do something like:

properties.getRegistration().entrySet().stream().map(RelyingPartyRegistrationProperyMapper::asRegistration)

Another option might be to introduce a RelyingPartyRegistrationRepositoryFactory interface that is given a List<RelyingPartyRegistration> and returns a RelyingPartyRegistrationRepository instance. That would allow us to keep the auto-configuration logic hidden but still allow users to plugin a different RelyingPartyRegistrationRepository implementation.

Flagging for team attention to see if there's a preference.

[mbhave]

Adding a RelyingPartyRegistrationRepositoryFactory could get confusing because then there would be two ways to supply a custom RelyingPartyRegistrationRepository . The auto-configuration would need to use the RelyingPartyRegistrationRepositoryFactory bean to create a RelyingPartyRegistrationRepository but back off in the presence of a user-provided RelyingPartyRegistrationRepository .

Given that it's cumbersome to get a list of RelyingPartyRegistrations from properties, moving asRegistration to a public util class would make sense.