Make name resolution configurable in OpenSamlLogoutRequestValidator

[palakova]

Describe the bug
Spring SAML considers NameID to hold username, populates Saml2AuthenticatedPrincipal#name with NameID value and later in Single Logout flow again populates/validates NameID value using Principal Name. This behaviour breaks our current integrations.

Username can be released by IdP in one of Assertion’s Attribute element instead. NameID, if present (as it is even optional, as also discussed in #11463), can be of different Format, holding different kind of values.

One example for all: According to SAML spec, NameID Format urn:oasis:names:tc:SAML:2.0:nameid-format:transient indicates that the content of the element is an identifier with transient semantics and SHOULD be treated as an opaque and temporary value by the relying party. This NameID element is also sent by IdP in LogoutRequest (causing validation against Principal Name in OpenSamlLogoutRequestValidator#validateNameId to fail) and is expected to be present in LogoutRequest sent from relying party (where it is populated with Principal Name in OpenSamlLogoutRequestResolver, making the IdP refuse the request).

Our workarounds:

1. Login: Custom responseAuthenticationConverter to retrieve username from Attribute + storing NameID element for later use.
2. Outbound LogoutRequest: Overriding NameID element with correct one stored during login.
3. Inbound LogoutRequest: There is no easy or clean way to work around this, because OpenSamlLogoutRequestValidator is not much configurable.

To Reproduce
Reproduced when integrating with Shibboleth IdP, which uses transient NameID and an Attribute to release username.

Expected behavior
Whole NameID element could be stored in DefaultSaml2AuthenticatedPrincipal (similar as session indexes are stored already) during login. It would be used to construct NameID element in outbound LogoutRequest, and it’s value would be used for validation when handling inbound LogoutRequest.

Ideally could you consider also to configure where to retrieve the username from in the first place (NameID element versus providing a Name of an Assertion Attribute)?

[jzheaux]

Thanks for the report, @palakova.

It sounds like there are two things happening here, and I want to see if we can separate them out.

First, you are saying that it would be nice to have the entire NameID available, at least so that the construction and validation of LogoutRequests can be customized, which I agree with worth looking at. Since transient is meant to be treated as opaque, I'm not sure that the framework can do better than that.

Second, you are asking for a way to provide a name-resolution strategy that is simpler than an entire converter.

If so, would you mind placing the second one on a separate ticket? Or if I haven't understood, would you help clarify your request?

[palakova]

Thank you for looking into this @jzheaux . I created a #12136 to separate enhancement for name-resolution strategy.

I agree, I don't think there is more to do with a NameID that does not hold username, other than using it in construction/validation of LogoutRequests.

[junytse]

Hi @jzheaux I also meet similar issues as @palakova when I was previously integrating SAML provider. It's great that I found this discussion.

Both former Spring SAML extension and Shibboleth SAML SP implementation will attempt to save NameID in the current session and later use it for logout. I workaround by subclassing DefaultSaml2AuthenticatedPrincipal to add a nameId field, store it on a ResponseAuthenticationConverter, and set it on a LogoutRequestResolverParametersConsumer:

public class MySaml2AuthenticatedPrincipal extends DefaultSaml2AuthenticatedPrincipal {
    public NameID getNameID() {
        return nameID;
    }

    private final NameID nameID; // the app support only NameID

    // in the early version, sessionIndexes are also saved here

    public MySaml2AuthenticatedPrincipal(NameID nameID, Map<String, List<Object>> attributes, List<String> sessionIndexes) {
        super(nameID.getValue(), attributes, sessionIndexes);
        this.nameID = nameID;
    }

}
public class MySaml2Util {
    public static Converter<OpenSaml4AuthenticationProvider.ResponseToken, Saml2Authentication> createMyResponseAuthenticationConverter() {
        return (responseToken) -> {
            Response response = responseToken.getResponse();
            Saml2AuthenticationToken token = responseToken.getToken();
            Assertion assertion = CollectionUtils.firstElement(response.getAssertions());
            RelyingPartyRegistration relyingPartyRegistration = token.getRelyingPartyRegistration();

            // add here
            NameID nameId = assertion.getSubject().getNameID(); // be aware of NPE
            Map<String, List<Object>> attributes = getAssertionAttributes(assertion); // copied from OpenSaml4AuthenticationProvider
            List<String> sessionIndexes = getSessionIndexes(assertion); // copied from OpenSaml4AuthenticationProvider

            DefaultSaml2AuthenticatedPrincipal principal = new MySaml2AuthenticatedPrincipal(nameId, attributes, sessionIndexes);
            principal.setRelyingPartyRegistrationId(relyingPartyRegistration.getRegistrationId());

            // customization codes
            // ...

            return new Saml2Authentication(principal, token.getSaml2Response(),
                    AuthorityUtils.createAuthorityList("ROLE_USER"));
        };
    }

    public static Consumer<LogoutRequestParameters> createMyLogoutRequestResolverParametersConsumer() {
        return (parameters) -> {
            //...
            LogoutRequest logoutRequest = parameters.getLogoutRequest();
            MySaml2AuthenticatedPrincipal principal = (MySaml2AuthenticatedPrincipal)parameters.getAuthentication().getPrincipal(); // be aware of exception
            NameID nameID = logoutRequest.getNameID();
            NameID principalNameID = principal.getNameID();
            //...
            // be aware of NPE...
            nameID.setFormat(principalNameID.getFormat());
            nameID.setFormat(principalNameID.getFormat());
            nameID.setNameQualifier(principalNameID.getNameQualifier());
            nameID.setSPNameQualifier(principalNameID.getSPNameQualifier());
            nameID.setSPProvidedID(principalNameID.getSPProvidedID());
            nameID.setValue(principalNameID.getValue());

            // in the early version, sessionIndexes are also handled here
        };
    }
}

My thoughts:
I think improvements could be made about standard SAML implementations:

• It'd be good if Saml2AuthenticatedPrincipal holds NameID;
• Since I overwritten responseAuthenticationConverter and customize the code about Saml2AuthenticatedPrincipal generation, I have to copy functions about attributes/sessionIndexes extration from OpenSaml4AuthenticationProvider; it'd be good if the population of Saml2AuthenticatedPrincipal, which holds SAML only contents, could be extracted to a dedicated method;