feat: KMS20-4808. Session Manager/UI: on error redirect to "oops" page#300
feat: KMS20-4808. Session Manager/UI: on error redirect to "oops" page#300alexey-medvedev-sap wants to merge 24 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Co-authored-by: Chris Burkert <burkert.chris@gmail.com> Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
alexey-medvedev-sap
force-pushed
the
feature/oops-page
branch
from
2d5dddc to
93d4364
Compare
| var serviceErr *serviceerr.Error | ||
| if !errors.As(err, &serviceErr) { | ||
| serviceErr = serviceerr.ErrUnknown | ||
| } |
There was a problem hiding this comment.
It would be really good to log the original error so that it doesn't get lost.
| // ErrorCode represents a user-facing error code for UI error page redirect. | ||
| type ErrorCode string | ||
|
|
||
| const ( | ||
| ErrorCodeNoTrustConfigured ErrorCode = "NO_TRUST_CONFIGURED" | ||
| ErrorCodeTokenExchangeFailed ErrorCode = "TOKEN_EXCHANGE_FAILED" | ||
| ErrorCodeStateExpired ErrorCode = "STATE_EXPIRED" | ||
| ErrorCodeFingerprintMismatch ErrorCode = "FINGERPRINT_MISMATCH" | ||
| ErrorCodeInvalidRequest ErrorCode = "INVALID_REQUEST" | ||
| ErrorCodeServerError ErrorCode = "SERVER_ERROR" | ||
| ErrorCodeUnauthorized ErrorCode = "UNAUTHORIZED" | ||
| ErrorCodeInvalidProvider ErrorCode = "INVALID_OIDC_PROVIDER" | ||
| ) |
There was a problem hiding this comment.
We already have error codes defined. We should use those.
| // ToErrorCode maps a service error to a user-facing error code for UI redirect. | ||
| func (e *Error) ToErrorCode() ErrorCode { | ||
| switch e.Err { | ||
| case CodeNotFound: | ||
| return ErrorCodeNoTrustConfigured | ||
| case CodeInvalidGrant: | ||
| return ErrorCodeTokenExchangeFailed | ||
| case CodeStateExpired: | ||
| return ErrorCodeStateExpired | ||
| case CodeFingerprintMismatch: | ||
| return ErrorCodeFingerprintMismatch | ||
| case CodeInvalidRequest, CodeInvalidLoginCSRFToken: | ||
| return ErrorCodeInvalidRequest | ||
| case CodeUnauthorizedClient: | ||
| return ErrorCodeUnauthorized | ||
| case CodeInvalidOIDCProvider: | ||
| return ErrorCodeInvalidProvider | ||
| default: | ||
| return ErrorCodeServerError | ||
| } | ||
| } |
There was a problem hiding this comment.
It isn't proven that CodeNotFound truly represents NoTrustConfigured error. There are no such details in CodeNotFound, and we can't assume it and upgrade the error code on this level of the definitions.
There was a problem hiding this comment.
Fixed — removed the ToErrorCode() mapping entirely. We now pass the existing error code (not_found, state_expired, etc.) directly as the errorCode query parameter without any interpretation. ok so?
| q := u.Query() | ||
| q.Set("errorCode", string(errorCode)) | ||
| u.RawQuery = q.Encode() |
There was a problem hiding this comment.
We should include both errorCode and description.
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
…n-manager into feature/oops-page
…n-manager into feature/oops-page Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
…n-manager into feature/oops-page
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Co-authored-by: Chris Burkert <burkert.chris@gmail.com> Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
Signed-off-by: alexey-medvedev-sap <alexey.medvedev02@sap.com>
…n-manager into feature/oops-page
When an error occurs during the auth flow, the session manager redirects to the UI error page (error_uri) with errorCode and errorDescription query parameters instead of returning a JSON error body. The error_uri parameter is optional for backward compatibility. Signed-off-by: Alexey Medvedev <alexey.medvedev02@sap.com>
…n-manager into feature/oops-page
3 participants
When the error occures in session manager, instead of showing just the mysterious json and returning 400, redirect to the (existing or specially designed) error page showing error and description.