Adding attributes to the "saml:Issuer" part in the AuthnRequest

[PardeepOne]

Hi, is it possible with the current implementation of this library to somehow add more attributes to the <saml:Issuer/> part of the AuthnRequest because at the moment this part is generated in this way:

<saml:Issuer>http://localhost:8091/DM_WEB/metadata.xhtml</saml:Issuer>

but I would like to have something like this:

<saml:Issuer
        NameQualifier="http://www.mylink.it"
        Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">
        http://www.mylink.it
</saml:Issuer>

Thanks in advance.

[pitbulk]

Not supported, but you can extend the AuthNRequest class.

Accept an issuer_NQ and issuer_Format at the constructor and use them later on the render

[mauromol]

First of all, the context: the SAML 2.0 specification says that, if the Format is missing, it must be intended as urn:oasis:names:tc:SAML:2.0:nameid-format:entity for an issuer (see section 2.2.5 of the SAML 2.0 core specification). Also, section 8.3.6 of the SAML 2.0 core specification says that, when format is urn:oasis:names:tc:SAML:2.0:nameid-format:entity, then the NameQualifier MUST be omitted.

So what is the rationale behind this request?
The problem is that the Italian SPID specifications go against the SAML specification and require that the Issuer of the AuthRequest has an explicit Format indication and a NameQualifier (whose nature is not so clear and for which usually the same SP entity id is used). I know, it's a sad story and it's tracked at italia/spid-regole-tecniche#15...

The problem of extending the AuthRequest class is that the login method of Auth class does not allow to change the type of AuthnRequest being instantiated: for this problem I'm thinking of a solution for which I will provide a PR.

The second problem is that the current implementation of AuthRequest is not very much extension-friendly, since it uses AuthnRequest.getAuthnRequestTemplate() which is static and private. So any extension will need to somewhat hack the superclass work to customize the generated XML. The best I can think of, without rethinking the whole XML generation mechanism, is to provide a sort of XML "post processing" facility.

[mauromol]

@pitbulk
What I had in mind to fix the first problem is to let Auth class support some optional java.util.BiFunctions that allow the user to customize the instantiation of AuthRequest, SamlResponse, LogoutRequest and LogoutResponse in login(), processResponse(), logout() and processSLO() methods respectively. These BiFunctions would have a AuthRequest, SamlResponse, LogoutRequest and LogoutResponse return types respectively, while the input depends on each case:

• to customize the SamlResponse and LogoutResponse instantiation, these BiFunctions need to take as arguments SamlSettings and HttpRequest
• for AuthRequest and LogoutRequest it's more complicated, because they take a lot of input parameters, so a BiFunction would not be enough, and two dedicated functional interfaces would be required

However, within the scope of #307 I introduced a refactoring for AuthRequest that allows to create one such instance given just a Saml2Settings and a AuthnRequestParams parameters: this has two benefits, the first one being the ability to simplify the so-many Auth.login(...) overloadings by moving the input parameter complexity into a AuthnRequestParams parameter, the second one being the ability to let the actual Auth.login(...) implementation construct an AuthnRequest object with simply two parameters, making the BiFunction route again viable for the AuthnRequest case as well.

If you agree with such a refactoring, I may follow a very similar approach for the LogoutRequest case.

To complete the picture: if the user does not specify any BiFunction to customize the above instantiations, Auth will use some very straightforward default implementations of those BiFunctions that simply return the result of the now hard-coded constructor invocations of the standard AuthRequest, SamlResponse, LogoutRequest and LogoutResponse classes, so the change would be 100% backward compatible, at both an API level and a functional level.

[mauromol]

@pitbulk
Here is a commit which implements the above proposal, using dedicated factory classes though instead of plain BiFunctions: in fact, the SamlResponse creation throws checked exceptions, which cannot be handled with BiFunction. The additional benefit is that this uses focused and dedicated functional interfaces, rather than generic functions.

mauromol@2a624d3

The commit is not ready for merge right now because it builds on top of other enhancements, like #321 and the anticipated refactoring of the LogoutRequest constructors, however it can give you an idea of the approach.

With all of this, if one wants to customise the creation of the <saml:Issuer> element of the <AuthnRequest> (as SPID requests), the library consumer can:

• create an extension of AuthnRequest (say it's called SpidAuthnRequest) which implements AuthnRequest.postProcess(String) to replace the default <saml:Issuer> produced by java-saml with a custom one (including the format and qualifier attributes)
• then use Auth as such:

Auth auth = new Auth(request, response);
auth.setAuthnRequestFactory(SpidAuthnRequest::new);
auth.login();

[pitbulk]

Hi @mauromol, I followed this thread but had not time yet to review all the work you are doing.
I had recently a baby so Im having a short rest, but will review all this work in 10 days or so.

[mauromol]

@pitbulk this is great news, congratulations for your baby! :-)
Take all your time, thank you!

[mauromol]

@pitbulk I believe this can be closed now that #321 and #352 have been merged.

The requested feature can be implemented by providing an extension to AuthnRequest' whose postProcessXmlmethod performs the necessary XML processing to add the needed attributes. Then, by setting a proper factory onAuththe library will be able to use such an extension in place of the defaultAuthnRequest`.

See current README file for details.