OpenConext / OpenConext-engineblock

OpenConext SAML 2.0 IdP/SP Gateway
14 stars 22 forks source link

Add specific error message for IdP cert mismatch #55

Closed baszoetekouw closed 10 years ago

baszoetekouw commented 10 years ago

Currently, if an IdP updates its certificate without notifying SURFconext, users are shown the "Invalid Idp response" error message.
It would be nice if the error message for this specific case (SAML Response signed by unknown/mismatched key) would be more specific.

relaxnow commented 10 years ago

Unfortunately SimpleSAMLphp doesn't let us inspect the Response to see if an IdP may have indicated which key the response was signed with. So without writing large chunks of that (which is already being done by someone else so in the future this may become easier) the best I could do is catch the signature validation exception and give it a separate feedback page:

screen shot 2014-10-02 at 13 34 00

relaxnow commented 10 years ago

Like #61 this appears to be a situation where the user is:

  1. An innocent bystander getting hurt because someone configured either EB or the IdP incorrectly.
  2. A SURFconext TPM or IdP TPM testing out configuration changes.

I propose changing the the message to:

Oops! I'm sorry, you appear to have run into a configuration error. Either "[[ NAME OF IDP ]]" is not setup correctly or we are. The SURFconext support staff has been warned of the issue and will investigate this as soon as possible.

If have more information or would like to know that status of this issue please contact SURFconext and give them the following information:

Timestamp: ... Request Id: .... ...

(HTTP status code 500 Internal Server Error)

baszoetekouw commented 10 years ago

Hi Boy,

Sound good! I think Eefje or Femke should review the error message tough, before its implemented.

The message suggests that SURFconext support is automatically notified of such errors; is that really the case (other than storing the error in the eb log)?

Ge, Bas.

On 02-10-2014 14:02 , Boy Baukema wrote:

Unfortunately, like #61 https://github.com/OpenConext/OpenConext-engineblock/issues/61 this appears to be a situation where the user is:

  1. An innocent bystander getting hurt because someone configured either EB or the IdP incorrectly.
  2. A SURFconext TPM or IdP TPM testing out configuration changes.

I propose changing the the message to:

Oops! I'm sorry, you appear to have run into a configuration
error. Either "[[ NAME OF IDP ]]" is not setup correctly or we
are. The SURFconext support staff has been warned of the issue and
will investigate this as soon as possible.

If have more information or would like to know that status of this
issue please contact SURFconext and give them the following
information:

Timestamp: ...
Request Id: ....
...

(HTTP status code 500 Internal Server Error)

— Reply to this email directly or view it on GitHub https://github.com/OpenConext/OpenConext-engineblock/issues/55#issuecomment-57619974.

Bas Zoetekouw SURFnet Advanced Services Tel: +31 30 2305362 Fax: +31 30 2305329 SURFnet - POBox 19035 - NL-3501 DA Utrecht - The Netherlands

relaxnow commented 10 years ago

sigh part of a larger issue I'm afraid. EB actually triggers a warning and is pretty strict about that sort of thing, it only triggers warnings if it warants TPMs looking at it. So you could say that EB is warning the TPMs. But there is currently, to my knowledge nothing set up to actually relay those warnings from the server logs to inboxes of tpms. And I don't think they are actively reviewed, do you know @thijskh ?

thijskh commented 10 years ago

They are not.

relaxnow commented 10 years ago

Bummer, part of a separate 'Monitoring' improvement project I guess. For now I think we can just change the message to:

Oops! I'm sorry, you appear to have run into a configuration error. Either "[[ NAME OF IDP ]]" or we are not setup correctly. Please contact the SURFconext support helpdesk at help@surfconext.nl and give them the following information:

Message: Invalid signature from IdP Timestamp: ... Request Id: .... ...

If this does not solve your problem.....

@thijskh agreed?

thijskh commented 10 years ago

Fine with me