Open PardeepOne opened 4 years ago
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
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 https://github.com/italia/spid-regole-tecniche/issues/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.
@pitbulk
What I had in mind to fix the first problem is to let Auth
class support some optional java.util.BiFunction
s that allow the user to customize the instantiation of AuthRequest
, SamlResponse
, LogoutRequest
and LogoutResponse
in login()
, processResponse()
, logout()
and processSLO()
methods respectively. These BiFunction
s would have a AuthRequest
, SamlResponse
, LogoutRequest
and LogoutResponse
return types respectively, while the input depends on each case:
SamlResponse
and LogoutResponse
instantiation, these BiFunction
s need to take as arguments SamlSettings
and HttpRequest
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 requiredHowever, 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 BiFunction
s 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.
@pitbulk
Here is a commit which implements the above proposal, using dedicated factory classes though instead of plain BiFunction
s: 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.
https://github.com/mauromol/java-saml/commit/2a624d3b6af120d1c44310f3c18cd4b9caa8c050
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:
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)Auth
as such:Auth auth = new Auth(request, response);
auth.setAuthnRequestFactory(SpidAuthnRequest::new);
auth.login();
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.
@pitbulk this is great news, congratulations for your baby! :-) Take all your time, thank you!
@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 on
Auththe library will be able to use such an extension in place of the default
AuthnRequest`.
See current README file for details.
Hi, is it possible with the current implementation of this library to somehow add more attributes to the
<saml:Issuer/>
part of theAuthnRequest
because at the moment this part is generated in this way:but I would like to have something like this:
Thanks in advance.