Marshalling a SAML assertion into a new Document loses its ID

[detelinyordanov at gmail.com]

Thanks for your response, I created
https://issues.shibboleth.net/jira/browse/OSJ-154

Regards,
   Detelin

2016-02-04 19:46 GMT+02:00 Brent Putman <putmanb at georgetown.edu>:

>
>
> On 2/4/16 4:04 AM, detelinyordanov at gmail.com wrote:
>
>    I have encountered an issue in Opensaml when marshalling an existing
> SAML assertion into a newly created empty Document. Apparently
> AbstractXMLObjectMarshaller.marshall(assertion, Document) uses
> Document.adoptNode() followed by Document.appendNode(..) but neither of
> these preserve the IDness of the assertion and if
> Document.getElementById(assertionId) is used on the resulting document, it
> returns null.
>
>
> This sounded familiar.  I went and looked and someone encountered a
> similar issue back in April 2012, discussed on this list.
>
>
> but then I noticed it is reproducible with Xerces as well and I saw that
> Xerces folks do not consider this a bug in Xerces DOM implementation - they
> argue that according to spec, the adoptNode() is not required to preserve
> the ID attributes, see XERCESJ-1022
> <https://issues.apache.org/jira/browse/XERCESJ-1022> and respective
> discussion: importNode()/adoptNode() and getElementById()
> <https://mail-archives.apache.org/mod_mbox/xerces-j-users/200701.mbox/%3C20070125062002.01E3310FB003@herse.apache.org%3E>
>
>
> Yep, I came across XERCESJ-1022 back in 2012.  Sadly that seems to be the
> official answer.
>
>
>
> Does this mean that this is a bug in Opensaml/Xmltooling
>
>
> Yes, I think it is a bug.  I don't know why an issue was never opened in
> 2012, but it wasn't and so we never really looked at fixing this.  Please
> open a bug against the v3 project (OSJ).
>
> As luck (or lack thereof) would have it, we literally just yesterday
> released the last non-security patch release of the v2 project (JOST in
> Jira), prior to its full EOL in July.  So there wouldn't be any plans to
> release it for v2.  Unless there were some security vulnerability aspect to
> this, but off-hand I don't see any.
>
> and Xmltooling node adoption code is required to ensure the ID attributes
> are preserved when marshalling into a new Document?
>
>
> Aside from the fix in OpenSAML itself:  Yes, after marshaling an existing
> XMObject, you would reset the IDness the same way the marshallers do it.
> For example, the AssertionMarshaller does this:
>
> domElement.setIdAttributeNS(null, Assertion.ID_ATTRIB_NAME, true);
>
>
> The fix in OpenSAML is likely to involve moving these
> setIdAttributeNS(...) calls out of the marshallAttributes(...) methods
> (which only get called when there is no existing DOM), and into a new one
> like marshallAttributeIDNess(...).  Then the latter gets called in both the
> new DOM and existing DOM cases.  Or something like that.  Doesn't look too
> bad to fix.
>
> --
> To unsubscribe from this list send an email to
> dev-unsubscribe at shibboleth.net
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20160205/7b8d82a8/attachment.html>