Better error message for SAML callback handler

xtremerui commented 2 years ago

Preflight Checklist

[X] I agree to follow the Code of Conduct that this project adheres to.
[X] I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

We embedded Dex in Concourse CI as a service provider for SAML auth flow. Recently we helped a customer with issue that SAML auth failed by seeing User session error. Turns out it was the problem in their SAML IDP that RelayState is not included in the callback request to Dex. Related code is below:

https://github.com/dexidp/dex/blob/421c26fdf56f16aae0ca375c3d11edc9ab7eb184/server/handlers.go#L402-L406

I am wondering if Dex can return a more specific error message so user would know what is missing in the callback request instead of digging it out from the source code. It's the same case for state too.

We'd happy to submit a PR if needed. Thank you!

Proposed Solution

Update the error to be User session error. RelayState is missing. or something similar.

Alternatives Considered

We could at least log it with detail message if not updating the error in response.

Additional Information

No response

gain620 commented 1 year ago

I strongly agree with this proposal!

PierreBls commented 7 months ago

Good idea

dexidp / dex