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

Add parameter name to JwtClientAssertionAuthenticationConverter error messages #1449

Closed
felix-hellman opened this issue Nov 17, 2023 · 1 comment
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@felix-hellman
Copy link

Expected Behavior
When client assertions fail validation, the exception message contains the parameter name.

Current Behavior
When client assertions fail validation the exception message is empty.

Context
Since the validation is for internal use and does not expose what parameter has failed validation, it is required to wrap the existing converter and re-run the validation adding the failed parameter name. Adding this will save time when troubleshooting what the client is sending incorrectly without adding additional logging since the exception is already thrown.

@felix-hellman felix-hellman added the type: enhancement A general enhancement label Nov 17, 2023
felix-hellman added a commit to felix-hellman/spring-authorization-server that referenced this issue Nov 17, 2023
felix-hellman added a commit to felix-hellman/spring-authorization-server that referenced this issue Nov 17, 2023
@jgrandja
Copy link
Collaborator

@felix-hellman Thanks for the PR associated to this issue. However, the changes in the PR would make the OAuth2Error inconsistent with the other client authentication converters ClientSecretBasicAuthenticationConverter, ClientSecretPostAuthenticationConverter and PublicClientAuthenticationConverter.

Furthermore, I'm not sure enhancing the error message to include the parameter name is the long term solution we're looking. I think we'll need to address this in gh-1240 and implement a holistic solution.

I'm going to close this issue and associated PR but feel free to add any additional comments to gh-1240.

@jgrandja jgrandja self-assigned this Nov 25, 2023
@jgrandja jgrandja added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement labels Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants