dex icon indicating copy to clipboard operation
dex copied to clipboard

Better error message for SAML callback handler

Open xtremerui opened this issue 2 years ago • 2 comments

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

xtremerui avatar Mar 10 '22 20:03 xtremerui

I strongly agree with this proposal!

gain620 avatar Feb 27 '23 18:02 gain620

Good idea

PierreBls avatar Nov 09 '23 15:11 PierreBls