plp050452 / simplesamlphp

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

ArtifactResolutionService signature #429

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Purpose of code changes on this branch:

Sign artifact response on ArtifactResolutionService endpoint.

When reviewing my code changes, please focus on:

Should I use some option for this?

After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by comel...@gmail.com on 18 Aug 2011 at 7:03

Attachments:

GoogleCodeExporter commented 8 years ago
I don't think an option should be necessary.

But: I see that you don't add an certificate to the signature. Shouldn't one be 
present? Otherwise, the SP will have to guess which of the public keys of the 
IdP should be used to verify the message.

I think you can also use sspmod_saml_Message::addSign() to get both the 
signature and certificate added to the message. addSign() expects the metadata 
of the recipient as its second argument, but does not actually use it, so it 
should be safe to pass NULL instead.

Original comment by olavmrk@gmail.com on 5 Sep 2011 at 11:09

GoogleCodeExporter commented 8 years ago
addSign() expects instance of SimpleSAML_Configuration as a second argument. 
Could I add something like:

sspmod_saml_Message::addSign($idpMetadata, new 
SimpleSAML_Configuration(array(), '[EMPTY]'), $artifactResponse);

Original comment by comel...@gmail.com on 6 Sep 2011 at 8:07

GoogleCodeExporter commented 8 years ago
I had assumed that PHP would accept NULL instead of an instance of 
SimpleSAML_Configuration, however, it turns out not to be the case.

There is fortunately a simple fix: change the declaration of addSign to:

    public static function addSign(SimpleSAML_Configuration $srcMetadata, SimpleSAML_Configuration $dstMetadata = NULL, SAML2_SignedElement $element) {

This makes PHP accept NULL instead. (We could also remove the parameter, but I 
think we may have a need for it in the future, if/when we implement support for 
different signature algorithms.)

Original comment by olavmrk@gmail.com on 7 Sep 2011 at 8:05

GoogleCodeExporter commented 8 years ago
OK, committed as r2895.

Original comment by comel...@gmail.com on 7 Sep 2011 at 8:26