Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CorsErrorResponseException prints wrong http code in the exception message #29451

Closed
1 of 2 tasks
spensk8 opened this issue May 10, 2024 · 2 comments · Fixed by #29649
Closed
1 of 2 tasks

CorsErrorResponseException prints wrong http code in the exception message #29451

spensk8 opened this issue May 10, 2024 · 2 comments · Fixed by #29649

Comments

@spensk8
Copy link

spensk8 commented May 10, 2024

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

core

Describe the bug

When a CorsErrorResponseException is thrown, the message in the exception and the stack trace is always HTTP 500 Internal Server Error, independently of the actual http error code passed to the CorsErrorResponseException constructor.

We use Datadog to monitor our Keycloak instance. The direct impact that we're having with this bug is that Datadog interprets any CorsErrorResponseException as an error and is causing false positives.

Version

24.0.4

Regression

  • The issue is a regression

Expected behavior

The error message in the exception should not be misleading. It should either include the correct http code or a more informative message.

Actual behavior

The error message in the exception is always HTTP 500 Internal Server Error

How to Reproduce?

It can be reproduced with anything that throws a CorsErrorResponseException

For example, invoking the token endpoint with grant_type=refresh_token and passing an expired refresh_token

The log level must be debug to see the stack trace

Anything else?

I believe that the solution may be to invoke the superclass with the status and maybe also with the errorDescription so that the message is more informative. For example:

public CorsErrorResponseException(Cors cors, String error, String errorDescription, Response.Status status) {
    super(errorDescription, status);
    this.cors = cors;
    this.error = error;
    this.errorDescription = errorDescription;
    this.status = status;
}

There are other classes that also extend WebApplicationException which may benefit from the same super calling.

@spensk8
Copy link
Author

spensk8 commented May 14, 2024

I've tested it now in the latest release (24.0.4) and the bug is still present.
I changed the initial issue description with the latest version information

@keycloak-github-bot
Copy link

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

froque added a commit to froque/keycloak that referenced this issue May 17, 2024
Frameworks like Datadog dd-trace-java java agent inspect the known WebApplicationException
and mark the exception as an HTTP 500, because that is the default for the
non argument constructor.

keycloak#29451

Signed-off-by: Filipe Roque <froque@premium-minds.com>
ahus1 pushed a commit that referenced this issue May 17, 2024
Frameworks like Datadog dd-trace-java java agent inspect the known WebApplicationException
and mark the exception as an HTTP 500, because that is the default for the
non argument constructor.

#29451

Signed-off-by: Filipe Roque <froque@premium-minds.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants