OpenSAML user discussion

[Jaime Pérez Crespo <>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 12/06/2008, at 16:51, Scott Cantor wrote:
> > Now, the question: I'm having problems when trying to manage an
> > assertion manually at DOM level. I need to set //Assertion/Advice
> > elements on my own as the XPath used by OpenSAML1 to store advices
> > doesn't fit our technical specifications.
>
> I would say offhand that that isn't a good idea.

I'm not going to discuss that here. First of all because I'm not the  
one who's able to discuss such kind of things, and secondly because  
that's the specification of eduGAIN and I don't know the reasons to  
use that XPath instead the one that OpenSAML is using. And as far as I  
know, it is compliant with SAML standard. Anyway, if you want my  
opinion, it's much more easier and intuitive than the OpenSAML approach.

> > I have no problems to set
> > it, but it's completely painful to retrieve the manually-crafted
> > Advice from an existing, custom SAMLAssertion.
>
> You're supposed to call getAdvice. Why is that painful?

I can't use getAdvice(), because, as I said, we are setting Advices  
manually, and of course we need to retrieve them manually. If I use  
getAdvice(), then our advices would be have to be stored at // 
Assertion/Advice/AssertionIDReference/, which conflicts with our specs  
(see above).

> > To do that, I'm getting
> > the owner document of the assertion with:
> >
> >         Document document = assertion.toDOM().getOwnerDocument();
>
> That isn't a supported approach.

What's the supported approach, then?

> > Elements inside the assertion with
> > something like:
> >
> >         NodeList advices = document.getElementsByTag("Advice");
>
> First of all that's a DOM1 call, which is illegal here. Secondly, it  
> would
> return every nested Advice element, even one inside an assertion  
> that's
> itself nested inside the top one. I made that mistake numerous times.

The code ensures there are no nested assertions so that's not a real  
problem (although I recognize it's not a good idea).

> Lastly, if the element in question is nested below the assertion,  
> you can
> call getElementsByTagNameNS on the *Element* interface, you don't  
> have to
> use the Document.

Yes, but you have to still retrieve the owner document to get an  
element on which to call getElementByTagNameNS(). Otherwise, you'll  
have to assume that casting the Node returned by toDOM() to an Element  
object will work...

> > Reviewing the SAMLObject.java code, it seems like toDOM() just  
> > returns
> > the current Node stored in the root attribute of the object, which
> > sometimes has an incorrect owner document.
>
> That's false. It's not an "incorrect" document. The issue is that  
> OpenSAML
> doesn't guarantee you that the object itself will be *rooted* as the
> document element because it can't. It has no idea what you plan to  
> do with
> the document, or what the object's relationship with that document is.

I was using quite lax words, but that's what I was meaning. Anyway,  
the point here is that "does not guarantee the object will be rooted".  
I can understand that people may have different needs, so you have to  
be flexible enough to allow everyone to use the library and get what  
they want, but then the interface provided is really poor in my  
opinion. And it's very poor because when you call toDOM() on an  
object, from the user's point of view, you expect to get the DOM for  
that object, with the document rooted adequately and all the contents  
of the object inside, perfectly mapped and rendered as they are in the  
object itself. More in detail, that means that a user will expect that  
a call to toDOM() will call inside that "plantRoot()" method, just as  
toString() or signing methods do, just because the user expects the  
complete object *always*, not only when printing to a string.

> > More precisely, it seems
> > like toString(), as well as other methods like toStream(), call
> > internally a protected method of SAMLObject called 'plantRoot()'.  
> > That
> > method seems to fix the owner document by putting the correct root
> > Node (shouldn't the 'root' attribute already point to the real root?)
> > as its child.
>
> That's a side effect of using c14n to do the serialization, which  
> happens to
> require the object be rooted. As does signing.
>
> > My question is: is this a bug? Why the owner document gets broken
> > sometimes? Why toSAML() is not calling plantRoot(), just as toString
> > does?
>
> See above.
>
> > Or maybe am I missing some detail? If this is not a bug, then
> > the usage of the library is completely broken in my opinion, as
> > someone calling toDOM() on a SAML object would expect *always* to  
> > have
> > *exactly the same* contents in the SAMLObject itself and in the
> > Document object retrieved by means of toDOM().getOwnerDocument().
>
> No. The content of the object is fine. The problem is that the  
> content of
> the Document depends on the context of use.

As I've said, then, in my opinion, the interface offered by OpenSAML  
is very poor, as you don't have a clear idea of what you get when  
calling a method and what is worse, things does not work as you would  
expect them to do. It's exactly the same use case as signing and  
validating signatures on an OpenSAML object. You can't sign() and then  
inmediately validate an object, which is absolutely crazy. If you want  
to validate an object just signed, then you have to marshall and  
unmarshall it before doing any validation. I'm not discussing the  
reasons that made OpenSAML behave like that, but my complain is about  
the interface given to the user. You can't assume that a user will be  
calling toString() just after toDOM(). Furthermore, you can't ask a  
user to call toString() if he wants the previous call to toDOM() to  
work, because that's absolutely non-sense.

Regards,

- --
Jaime Pérez Crespo
Middleware Engineer
red.es / RedIRIS NREN

mail: 

xmpp: 

http://www.rediris.es





-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkhSUSgACgkQ5f+rJs0i1iElywCgvyb8i9TVaEGTs6VfZ8uclApE
IGcAn3sdhvB5de+aF/3OKqX98bGYFwHf
=hXp5
-----END PGP SIGNATURE-----