Skip to content

Missing SubjectConfirmationData must not break validation#825

Closed
m0ark wants to merge 1 commit intosimplesamlphp:masterfrom
m0ark:patch-1
Closed

Missing SubjectConfirmationData must not break validation#825
m0ark wants to merge 1 commit intosimplesamlphp:masterfrom
m0ark:patch-1

Conversation

@m0ark
Copy link
Contributor

@m0ark m0ark commented Apr 3, 2018

SubjectConfirmationData is optional , but must not break validation if absent!

SubjectConfirmationData is optional , but must not break validation if absent!
@trscavo
Copy link

trscavo commented Apr 3, 2018

Jumping in here...SubjectConfirmationData is NOT optional for SSO, at least according to the SAML2 Profiles specification. I must be missing something.

@m0ark
Copy link
Contributor Author

m0ark commented Apr 3, 2018

Well, according to the SAML2 SSO Profiles you are right, it is not optional.
But according to this documentation (https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf) it clearly is.
I will fix it asap.

@trscavo
Copy link

trscavo commented Apr 3, 2018

FWIW, I believe the Profiles specification takes precedence here.

@thijskh
Copy link
Member

thijskh commented Apr 4, 2018

This has been changed specifically to be this way. See the discussion in #530 and commit message 980b34c. As Tom says SimpleSAMLphp is about the SSO profile.

Is there something not working for you that you want to change this?

@m0ark
Copy link
Contributor Author

m0ark commented Apr 4, 2018

Actually I tried to get module-attributeaggregator working with encrypted assertions by using this function to decrypt and validate the response received after querying the attribute authority.
The response only contains one assertion with an empty <SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer" /> which should be fine according to the specs, but breaks due to this restriction.

@m0ark m0ark closed this Apr 25, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants