Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't rely on InclusiveNamespaces for namespace declarations #81

Merged
merged 4 commits into from Sep 12, 2016
Merged

Don't rely on InclusiveNamespaces for namespace declarations #81

merged 4 commits into from Sep 12, 2016

Conversation

mgduk
Copy link
Contributor

@mgduk mgduk commented Sep 6, 2016

The add_namespaces_to_child_assertions function assumes the presence of an
InclusiveNamespaces
element

from which it will read a list of namespaces.

If this is present, any namespace definitions declared by InclusiveNamespaces
are copied from the Response element to the Assertion. Then, when the Assertion
element is handled in isolation from the rest of the Response document, the
namespaces are all still defined.

However if the InclusiveNamespaces element is not found, no namespaces are
copied. If then the Assertion element is namespaced (e.g. <saml:Assertion>),
it will not be
found

and the document will be treated as not having an assertion at all.

If there is an InclusiveNamespaces element, we will continue to use that as
before to read the namespaces to apply to the Assertion element.

However if there is none (or it's empty), we'll just read all the namespaces from the Response
element onto the Assertion element.

This should ensure all the necessary namespaces are defined when the Assertion is used out of context, such as to validate the signature.

The `add_namespaces_to_child_assertions` function [assumes the presence of an
InclusiveNamespaces
element](https://github.com/Clever/saml2/blob/6cad30fb1e0e9a59ae850f7de6bed4310a403ba9/lib/saml2.coffee#L399)
from which it will read a list of namespaces.

If this is present, any namespace definitions declared by InclusiveNamespaces
are copied from the Response element to the Assertion. Then, when the Assertion
element is handled in isolation from the rest of the Response document, the
namespaces are all still defined.

However if the InclusiveNamespaces element is not found, no namespaces are
copied. If then the Assertion element is namespaced (e.g. `<saml:Assertion>`),
it [will not be
found](https://github.com/Clever/saml2/blob/6cad30fb1e0e9a59ae850f7de6bed4310a403ba9/lib/saml2.coffee#L440)
and the document will be treated as not having an assertion at all.

If there is an InclusiveNamespaces element, we will continue to use that as
before to read the namespaces to apply to the Assertion element.

However if there is none, we'll just read all the namespaces from the Response
element onto the Assertion element.

There seemed no value in asserting the presence of a Signature element within
the Assertion, as it's valid to just have the whole message signed, so I
removed that.
This was the previous behaviour (as far as I can ascertain from the code).
@brettkiefer
Copy link

👍

@jefff
Copy link
Contributor

jefff commented Sep 12, 2016

Thanks for the PR! lgtm.

@jefff jefff merged commit a3b3134 into Clever:master Sep 12, 2016
@jefff
Copy link
Contributor

jefff commented Sep 12, 2016

Merged and deployed as 1.9.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants