OpenSAML user discussion

[Jaime Pérez Crespo <>]

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

Hi Scott,

On 13/06/2008, at 17:19, Scott Cantor wrote:

> > 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.
>
> You misunderstand my comment. The schema is what it is. I don't know  
> what
> XPath has to do with this, but my point was that OpenSAML simply  
> tries to
> let you put stuff into Advice in accordance with the schema. It  
> shouldn't be
> in conflict with whatever you're doing, but if it is, I wanted to  
> understand
> why.
>
> But adding to the DOM directly is definitely not supported in  
> general. I'm
> sorry if that's a problem for you, but that is the reason you  
> encountered an
> issue.

I just need to access the DOM to put contents into the Advice tag,  
without having any other child tags. That's what I was telling, our  
specs force to put Advices in this way:

<Advice>
    textContent
</Advice>

But maybe we should review our specs...

> > 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).
>
> That's the schema. I don't understand what you're doing that  
> "conflicts"
> with that but somehow is legal. I'm sure it's possible, but I'm simply
> trying to understand your problem so I can suggest the best work- 
> around.
>
> I'm simply asking if this assumption about having to do it all  
> manually is a
> misunderstanding.

No, see above. Unfortunately we can't use setAdvice() to build the  
advices the way we want...

> > What's the supported approach, then?
>
> The only support is the set/getAdvice methods. If that's not good  
> enough,
> there isn't any supported mechanism, and that means you'll have  
> problems
> that have to be worked around.
>
> The reason the Advice support is very limited is that Advice is not
> something I had a requirement for.

Well, I know OpenSAML1 does not support any other way to set advices  
manually, but I understand this is where toDOM() makes sense, no? I  
mean, at least you provide the "do it your way" method, which is  
toDOM() here...

> > 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...
>
> And you can assume that, because that's guaranteed by the method.  
> Sorry if
> that's not clear. The reason it returns a Node is mostly historical  
> I think.
> I don't know that I even recall why it wasn't typed as an Element.
>
> In any case, you do not need to get the owner document except to  
> create DOM
> nodes yourself, which is not supported. But if you do it, using
> toDOM().getOwnerDocument() is ok for that because the SAML object  
> doesn't
> need to be rooted for that to work.

Ok, that's pretty good, thanks for the clarification. I was assuming  
the object returned was not guaranteed to be an Element (I couldn't  
find any doc telling that). That simplifies a lot managing the object  
at DOM level, and removes the requisite of retrieving the Document  
itself. That way I can avoid my original problem, altough I finally  
solved by iterating through the childs of the Node searching manually  
for advices (which is not the optimal solution, I think).

> > 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.
>
> And I don't agree. I think it means you get a DOM. Rooting it in a  
> document
> was not a requirement I had, and is not something I expect. Mainly  
> because
> what you're asking would break any attempt to add that object in DOM  
> form to
> some other DOM tree, because appendChild breaks if the object  
> already has a
> parent node. I had that requirement and that's why the code works  
> that way.
> It also breaks the recursive use of the method to build up the DOM  
> in the
> first place.
> > 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.
>
> You get a complete object. You don't get a complete *document*. That  
> is what
> *this* user expected and needed. Since the code was written for me,  
> that
> kind of drives the process.
>
> > 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.
>
> Not having a clear idea is a failure of documentation, not code.  
> Since the
> code is virtually undocumented, that isn't a big surprise, but I  
> have always
> been up front with people about that.

I think the problem here is that OpenSAML was designed and developed  
to fit your needs, and we all are using it with our own needs. For me,  
when I get the Document of an OpenSAML object, I expect to get it  
rooted to be able to operate with it without having to worry whether  
if its included in any other object or not. Basically, because I want  
to find inside the Document object exactly the same as I have inside  
the OpenSAML one. So, given that both of us have different uses for  
the library, and expect different behaviours, I think the interface  
should provide more methods to operate with it in a more controlled  
way. I understand it fits your requirements, but I think it should  
also be quite more open to any other uses. Anyway, I agree with you  
here, probably the root problem is the lack of a proper documentation  
giving complete details of how OpenSAML works and what you should  
expect from any method.

> > 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.
>
> You can't do that in most libraries. Try it with xmlsec alone. It  
> fails
> miserably.

I've never used xmlsec alone. Does it really behave like that? That's  
what I was talking about. Such kind of behaviours are absolutely crazy  
for a user who does not know anything about the internals of the  
library (and since we are talking about libraries, users should never  
know about the internals...)

And now we are talking about signatures, I'll ask you another  
question: what's your recommended way of testing signatures? I mean,  
if you need to validate an object recently signed, what would you do  
to ensure validation will work?

> > 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().
>
> And I don't. Look, try to think about this for two seconds. What  
> happens if
> you call toDOM on a nested object like a statement or a condition?  
> Would you
> expect that to suddenly become the document element? Why? What  
> happens to
> the assertion it was wrapped in?

Yes, I understand that every SAML object can have a context, and that  
context will be "destroyed" if you root the Document to the current  
object. But think also about the opposite, what if you just got a new  
assertion, and you don't mind if it is part of a bigger document? I  
guess you can be sure the document will be rooted if you created the  
object yourself (not from DOM), because that way you're sure it's not  
part of a bigger document, but, what if you don't know and you don't  
even mind?

> I don't think you're grasping the larger context of this problem.  
> But since
> I doubt we're going to agree on this, I'll point out a more relevant  
> issue:
> the code is NOT changing. It's been replaced already.
>
> What exactly are you asking me to do?

Probably I don't, as I'm not SAML or XML expert. But that's why I'm  
saying that OpenSAML makes assumptions about the knowledge the user  
has about the internals of SAML or XML.

Anyway, don't worry, I'm not asking you to do nothing. I know  
OpenSAML1 is now closed. I just wanted to know if my issue with  
toDOM(), or furthermore, with signatures, was a bug or a "feature".  
Now I know it's a "feature", I understand the reasons to make it  
behave like that, although I think it should provide a better  
interface and, of course, documentation. I really think a good  
documentation would save both of us lots of time. Mine, because I  
won't be crazy anymore trying to find why something does not work as I  
would expect it to do, and yours because you won't have to spend time  
replying to the same questions again and again, nor explaining the way  
OpenSAML works or why does it work like that, which is the exact case  
of this discussion. Although I must say this kind of discussions are  
pretty interesting, in my opinion. You can take this as a suggestion,  
if you want, to consider giving more intuitive interfaces or a better  
documentation for further developments of OpenSAML.

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)

iEYEARECAAYFAkhWIpkACgkQ5f+rJs0i1iH6NQCg21Kojqr4IVUQ4qEDqpP0rGr0
czAAnjY8snEIxSY5cHwjlg6MyWO+R40m
=qqag
-----END PGP SIGNATURE-----