search

plp050452 / simplesamlphp

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

AuthncontextClassRef validation on SP side #472

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
I attached the AuthnContext validator filter files and the patch of the 
saml2-acs.php.

Please test it, and if it's ok, use it.

Gyula Szabó
MTA SZTAKI

Original issue reported on code.google.com by gyufi@sztaki.hu on 31 Jan 2012 at 6:46

Attachments:

GoogleCodeExporter commented 8 years ago 
A few comments:

- I think this filter belongs in the saml-module rather than the core-module.

- In unauthorized(), you have:
    $id = $request["saml:sp:State"]["SimpleSAML_Auth_State.id"];
  This is not safe, since there is no guarantee that the state array has been saved at that point. Instead, use the SimpleSAML_Auth_State::saveState()-function.

- I don't think the retryURL is used anywhere? Maybe that code can be removed?

- You add an link to documentation for the filter in 
simplesamlphp-authproc.txt.diff, but the document is not included in 
authncontextclassref.tar.gz. Could you add it?

Original comment by olavmrk@gmail.com on 31 Jan 2012 at 11:48

GoogleCodeExporter commented 8 years ago 
Thank you Olav, I'm on it.

Original comment by gyufi@sztaki.hu on 31 Jan 2012 at 12:02

GoogleCodeExporter commented 8 years ago 
I think it's done. Take a look at it, please.

Thanks,
gyufi

Original comment by gyufi@sztaki.hu on 10 Feb 2012 at 1:16

Attachments:

GoogleCodeExporter commented 8 years ago 
There is one thing that must be fixed, so that the code does not break for 
those that have assertions enabled: In 
modules/saml/templates/sp/wrong_authncontextclassref.tpl.php you have:

    assert('array_key_exists("retryURL", $this->data)');

But that is never added to the data array, so this assert will always fail. 
(Also, $retryURL and $retry is never used in that code, so it can probably be 
deleted?

There is also a minor change I'd like to see: In 
modules/saml/www/sp/saml2-acs.php, you add:

    $state['saml:sp:AuthnContext'] = $assertion->getAuthnContext();

But you do not add 'saml:sp:AuthnContext' to PersistentAuthData. I think this 
information can also be useful to have available from the "client code", so it 
should be added to PersistentAuthData.

Then you would be able to access it using:

    $as = new SimpleSAML_Auth_Source(...);
    $as->getAuthData('saml:sp:AuthnContext');

Original comment by olavmrk@gmail.com on 13 Feb 2012 at 7:31

GoogleCodeExporter commented 8 years ago 
Ok, I think it's done.

Thanks,
Gyula Szabó

Original comment by gyufi@sztaki.hu on 13 Feb 2012 at 11:31

Attachments:

GoogleCodeExporter commented 8 years ago 
Thanks! Applied as r3031.

Original comment by olavmrk@gmail.com on 13 Feb 2012 at 12:52