vivekrajenderan / simplesamlphp

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

r3067 not accepting SingleSignOnService as an array of endpoints #489

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Somewhere between 1.8.2 and r3067, the SAML 2.0 IdP metadata generation broke 
when the saml20-idp-hosted.php entry for "SingleSignOnService" is an array of 
endpoints.

ex.

        $metadata['https://www.example.com/saml/saml2/idp/metadata.php'] = array(

            // The hostname of the server (VHOST) that this SAML entity will use.
            'host'              => 'www.example.com',
            'certificate'       => 'server.crt',
            'privatekey'        => 'server.key',
            'auth'              => 'exampleAuth',
            'entityid'          => 'https://www.example.com/saml/saml2/idp/metadata.php',
            'SingleSignOnService' => array(
                0 => array (
                    'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
                    'Location' => 'https://www.example.com/saml/saml2/idp/SSOService.php',
                ),
                1 => array (
                    'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
                    'Location' => 'https://www.example.com/saml/saml2/idp/SSOService.php',
                ),
            ),
...

When loading up https://www.example.com/saml/saml2/idp/metadata.php, it results 
in an exception:

SimpleSAML_Error_Error: METADATA
Backtrace:
0 /usr/local/apache/htdocs/simplesamlphp/www/saml2/idp/metadata.php:163 (N/A)
Caused by: Exception: 
https://www.example.com/saml/saml2/idp/metadata.php['SingleSignOnService']:[0]: 
Location must be a string.
Backtrace:
2 /usr/local/apache/htdocs/simplesamlphp/lib/SimpleSAML/Configuration.php:958 
(SimpleSAML_Configuration::getEndpoints)
1 
/usr/local/apache/htdocs/simplesamlphp/lib/SimpleSAML/Metadata/SAMLBuilder.php:4
59 (SimpleSAML_Metadata_SAMLBuilder::addMetadataIdP20)
0 /usr/local/apache/htdocs/simplesamlphp/www/saml2/idp/metadata.php:123 (N/A)

This used to work. In fact admin/metadata-converter.php will convert the 
"SingleSignOnService" into an array endpoints, so saml20-idp-hosted should be 
able to accept that format.

The patch below (and attached) allows for an array of endpoints to function 
again, but may not be the correct way to fix this:

Index: www/saml2/idp/metadata.php
===================================================================
--- www/saml2/idp/metadata.php  (revision 3067)
+++ www/saml2/idp/metadata.php  (working copy)
@@ -57,12 +57,19 @@
        );
    }

+
+   $ssoService = $metadata->getGenerated('SingleSignOnService', 
'saml20-idp-hosted');
+
+   if (!is_array($ssoService)) {
+       $ssoService = array(0 => array(
+                   'Binding' => SAML2_Const::BINDING_HTTP_REDIRECT,
+                   'Location' => $ssoService));
+   }
+
    $metaArray = array(
        'metadata-set' => 'saml20-idp-remote',
        'entityid' => $idpentityid,
-       'SingleSignOnService' => array(0 => array(
-                   'Binding' => SAML2_Const::BINDING_HTTP_REDIRECT,
-                   'Location' => $metadata->getGenerated('SingleSignOnService', 
'saml20-idp-hosted'))),
+       'SingleSignOnService' => $ssoService,
        'SingleLogoutService' => $metadata->getGenerated('SingleLogoutService', 'saml20-idp-hosted'),
    );

Original issue reported on code.google.com by ecam...@gmail.com on 16 Apr 2012 at 6:22

Attachments:

GoogleCodeExporter commented 9 years ago
I don't think this is something we want to support. It was never the intended 
use of the SingleSignOnService idp-hosted metadata element.

(Note that elements supported by idp-remote metadata are not necessarily 
supported by idp-hosted metadata.)

The intended use of this parameter was to allow people to relocate the 
SingleSignOnService endpoint, which is why we assumed it to be a simple string.

A patch to check for a boolean config option and add the HTTP-POST endpoint if 
it is enabled would probably be welcome :)

Original comment by olavmrk@gmail.com on 17 Apr 2012 at 8:14