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

Throw invalid_grant when invalid token request with PKCE #581

Conversation

Kehrlann
Copy link
Contributor

  • When providing an invalid authorization_code in a token request with a PKCE code_verifier, throw invalid_grant, as per RFC 6749. It is consistent with behavior in OAuth2AuthorizationCodeAuthenticationProvider#authenticate
  • When providing an invalid code_verifier, throw invalid_grant, as per RFC7636.

There is an additional corner-case in OAuth2ClientAuthenticationProvider#authenticatePkceIfAvailable, when there is no code_challenge in the authorization request and the client requires PKCE.

  • In the "normal" case, that should not be possible because the OAuth2AuthorizationCodeRequestAuthenticationProvider#authenticateAuthorizationRequestwill throw an invalid_request`.
  • There might be an edge case where the client configuration changes between the time an authorization code is issued and the time it is exchanged for a token.

I am unsure what we should throw in this case, I put invalid_grant for consistency, feel free to change it.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 17, 2022
@jgrandja jgrandja closed this in a1e513b Jan 24, 2022
@jgrandja jgrandja self-assigned this Jan 24, 2022
@jgrandja jgrandja added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 24, 2022
@jgrandja jgrandja added this to the 0.2.2 milestone Jan 24, 2022
@jgrandja
Copy link
Collaborator

Thanks for this fix @Kehrlann ! This is now in main.

doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants