plp050452 / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

It is not possible to configure an IdP with WantAuthnRequestsSigned set to True #528

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Configure saml20-idp-hosted.php and put 'redirect.sign' => TRUE
2. Get the metadata for your IdP

What is the expected output? What do you see instead?

Expected output:
<md:IDPSSODescriptor ... WantAuthnRequestsSigned="true">

Actual output:
<md:IDPSSODescriptor ... >

What version of the product are you using? On what operating system?

This can be reproduced with SSP 1.10.0 and trunk

Please provide any additional information below.

The bug is due to a typo and also in the metadata.php view, which is not 
passing this option to the Metadata/SAMLBuilder.php

Attached is a very simple patch that solves this issue.

Original issue reported on code.google.com by lorenzo....@gmail.com on 20 Dec 2012 at 1:09

Attachments:

GoogleCodeExporter commented 8 years ago
Hi,

the typo fix should definitively be applied, but the change in metadata.php is 
not correct. The SAMLBuilder class works on saml20-idp-remote metadata, while 
metadata.php works on saml20-idp-hosted metadata. In remote metadata, the flag 
is redirect.sign, while in hosted it is redirect.validate.

Also, since SAMLBuilder was written, we have also added some other flags to 
control signing and validation -- sign.authnrequest (saml20-idp-remote) and 
validate.authnrequest (saml20-idp-hosted). This option has precedence over the 
redirect options, so that the code first needs to look at that option.

Will you submit a new patch?

Original comment by olavmrk@gmail.com on 3 Jan 2013 at 7:59

GoogleCodeExporter commented 8 years ago
Yes.

Original comment by lorenzo....@gmail.com on 3 Jan 2013 at 2:29

GoogleCodeExporter commented 8 years ago
Here is a new version of the patch. Now in metadata.php it checks for the 
'redirect.validate' flag since this file works for saml20-idp-hosted metadata.

In the SAMLBuilder class I check for all flags (obsolete and new) that can 
force the WantAuthnRequestsSigned="true" attribute to be present in the 
IDPSSODescriptor element.

Acording to Olav's comment SAMLBuilder is usesd for saml20-idp-remote metadata 
but it is also called from metadata.php, which is used for saml20-idp-hosted 
metadata. That's why I think I should check for all the possible flags here.

Original comment by lorenzo....@gmail.com on 18 Feb 2013 at 7:56

Attachments:

GoogleCodeExporter commented 8 years ago
Hi,

sorry for the late response to this issue.

The patch has a couple of errors:

- 'sign.authnrequest' has precedence over 'redirect.sign', and 
'validate.authnrequest' has precedence over 'redirect.validate'. This means 
redirect.* should only be used if the other option is unset, and not if it is 
FALSE.

- While it is true that SAMLBuilder is used from metadata.php, the metadata 
arrays are transformed into saml20-idp-remote metadata before calling that 
class. This means that the class should only have to deal with 
saml20-idp-remote metadata.

Original comment by olavmrk@gmail.com on 11 Mar 2013 at 11:34

GoogleCodeExporter commented 8 years ago

Original comment by jaim...@gmail.com on 11 Jul 2013 at 9:30

GoogleCodeExporter commented 8 years ago
Hi guys, i would like to know if there are any plans on resolve this?

Original comment by richard....@gmail.com on 16 Sep 2013 at 5:07

GoogleCodeExporter commented 8 years ago
Closing the issue here, moved to:

https://github.com/simplesamlphp/simplesamlphp/issues/43

Original comment by jaim...@gmail.com on 27 Feb 2014 at 7:38

GoogleCodeExporter commented 8 years ago
This has been solved in commit 3ce8642.

Original comment by jaim...@gmail.com on 24 Jun 2014 at 2:08