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

Invalid_request failures in JwtTokenValidators are always turned into invalid_token errors #10337

Closed
jason076 opened this issue Oct 1, 2021 · 8 comments · Fixed by #10500
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: bug A general bug

Comments

@jason076
Copy link
Contributor

jason076 commented Oct 1, 2021

Describe the bug
Returning any failure in a OAuth2TokenValidator validate function always results in a InvalidBearerTokenException with the error code BearerTokenErrorCodes.INVALID_TOKEN and returns a 401. The Error handling does not respect the Error code the validate function returns. This leads to OAuth2ErrorCodes.INVALID_REQUEST get converted to BearerTokenErrorCodes.INVALID_TOKEN.

To Reproduce
Return OAuth2ErrorCodes.INVALID_REQUEST in OAuth2TokenValidator validate. This was already the case in an bug I reported earlier: #10319

Expected behavior
If I return OAuth2ErrorCodes.INVALID_REQUEST failure from OAuth2TokenValidator validate I expect an OAuth2AuthenticationException with the BeareErrorCode BearerTokenErrorCodes.INVALID_REQUEST and the status code 400.

Sample
The problematic line in the sourcode is the following

Each BadJwtException is converted to InvalidBearerTokenException with the error code BearerTokenErrorCodes.INVALID_TOKEN

@jason076 jason076 added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Oct 1, 2021
@eleftherias eleftherias added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 1, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Oct 1, 2021

Hi, @jason076, thanks for the report.

If I return OAuth2ErrorCodes.INVALID_REQUEST failure from OAuth2TokenValidator validate I expect an OAuth2AuthenticationException with the BeareErrorCode BearerTokenErrorCodes.INVALID_TOKEN

Why would you want the error to change? The errors are defined as follows (emphasis mine):

invalid_request
The request is missing a required parameter, includes an
unsupported parameter or parameter value, repeats the same
parameter, uses more than one method for including an access
token, or is otherwise malformed. The resource server SHOULD
respond with the HTTP 400 (Bad Request) status code.

invalid_token
The access token provided is expired, revoked, malformed, or
invalid
for other reasons. The resource SHOULD respond with
the HTTP 401 (Unauthorized) status code. The client MAY
request a new access token and retry the protected resource
request.

So I'm not sure why you'd want to return an invalid_request when the token is invalid.

and the status code 400.

The above says that invalid_token should yield a 401 and invalid_request should yield a 400. Because of that, I don't think we want to send invalid_token with a 400 by default.

I believe you can customize the behavior in your own BearerTokenAuthenticationEntryPoint.

Have I misunderstood your use case?

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed type: bug A general bug labels Oct 1, 2021
@jason076
Copy link
Contributor Author

jason076 commented Oct 1, 2021

Sorry, I had a Mistake in the expected behavior section.
I will correct my post.
Here is the correct version:

** Expected behavior **
If I return OAuth2ErrorCodes.INVALID_REQUEST failure from OAuth2TokenValidator validate I expect an OAuth2AuthenticationException with the BearerErrorCode BearerTokenErrorCodes.INVALID_REQUEST and the status code 400.

** Background **
Earlier I reported that an expired token lead to an OAuth2ErrorCodes.INVALID_REQUEST failure. #10319

In our project I wondered why the server is returning 401 despite this bug. I expected that if a OAuth2TokenValidator returns OAuth2ErrorCodes.INVALID_REQUEST the server would return 400. So I debuged the code and found that in line


the INVALID_REQUEST is turned into an INVALID_TOKEN effectively changing the status from 400 to 401.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 1, 2021
@jason076
Copy link
Contributor Author

jason076 commented Oct 1, 2021

So in short the current code does the following OAuth2TokenValidator.validate() returns OAuth2ErrorCodes.INVALID_REQUEST but the server response is BearerTokenErrorCodes.INVALID_TOKEN 401. So i think it is a bug. It should be 400 for INVALID_REQUEST.

@jzheaux
Copy link
Contributor

jzheaux commented Oct 5, 2021

Ah, gotcha, @jason076. Thanks for clarifying.

It seems to me that if the validator determines the token to be invalid, an INVALID_TOKEN should be returned. If the validator is returning an INVALID_REQUEST then, I'd expect the validator's behavior to be corrected.

Taking a look at JwtClaimValidator, I think it incorrectly returns INVALID_REQUEST.

Can you submit a PR to correct JwtClaimValidator, or can you explain why validators should return an INVALID_REQUEST?

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Oct 5, 2021
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Oct 12, 2021
@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Oct 19, 2021
@jason076
Copy link
Contributor Author

Sorry for the late Feedback. I will submit a PR in the next days.

@jason076
Copy link
Contributor Author

I have submitted the PR #10500

jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request error on claim validation failure.
But validators have to return invalid_token errors on failure according to:
https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.
Also see gh-10337

Closes gh-10337
jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request error on claim validation failure.
But validators have to return invalid_token errors on failure according to:
https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.
Also see gh-10337

Closes gh-10337
jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request error on claim validation failure.
But validators have to return invalid_token errors on failure according to:
https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.
Also see gh-10337

Closes gh-10337
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label Nov 29, 2021
jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request error on claim validation failure.
But validators have to return invalid_token errors on failure according to:
https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.
Also see gh-10337

Closes gh-10337
jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request error on claim validation failure.
But validators have to return invalid_token errors on failure according to:
https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.
Also see gh-10337

Closes gh-10337
jzheaux pushed a commit that referenced this issue Nov 29, 2021
Previously JwtClaimValidator returned the invalid_request
error on claim validation failure.

But validators have to return invalid_token errors on failure
according to:

https://datatracker.ietf.org/doc/html/rfc6750#section-3.1.

Closes gh-10337
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants