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 ability to customize the access token response #1429

Closed
wants to merge 1 commit into from

Conversation

ddubson
Copy link
Contributor

@ddubson ddubson commented Oct 30, 2023

Fixes gh-925

Referenced in gh-1369

Open questions:

  • Does extracting an authentication success handler for OAuth 2 Token Endpoint filter enough to make access token response customizable? Or is there a need for a customization function for adding to additionalParameters.
  • If the Authentication is of incorrect type, what should the HTTP response be?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 30, 2023
@jgrandja
Copy link
Collaborator

jgrandja commented Nov 6, 2023

@ddubson

is there a need for a customization function for adding to additionalParameters

Maybe. I see you exposed setAccessTokenHttpResponseConverter(), which would allow for adding additional parameters. I would suggest writing a test for the use case to see how easy it is. From there, we can decide if another hook is needed to allow for easier customization.

@jgrandja jgrandja added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 6, 2023
@jgrandja jgrandja self-assigned this Nov 6, 2023
@jgrandja jgrandja added this to the 1.3.x milestone Nov 6, 2023
@ddubson
Copy link
Contributor Author

ddubson commented Nov 16, 2023

@ddubson

is there a need for a customization function for adding to additionalParameters

Maybe. I see you exposed setAccessTokenHttpResponseConverter(), which would allow for adding additional parameters. I would suggest writing a test for the use case to see how easy it is. From there, we can decide if another hook is needed to allow for easier customization.

I was able to successfully customize the access token HTTP response converter and inject custom parameters (with test case exercising the customization) -- it's not the prettiest implementation, but it is concise and, in my opinion, sufficient enough for the amount of demand for this kind of functionality.

@ddubson ddubson changed the title Draft: Add OAuth2TokenEndpointAuthenticationSuccessHandler Add OAuth2TokenEndpointAuthenticationSuccessHandler Nov 16, 2023
@ddubson ddubson requested a review from jgrandja November 16, 2023 15:50
@ddubson ddubson marked this pull request as ready for review November 16, 2023 15:51
@jgrandja jgrandja modified the milestones: 1.3.x, 1.3.0-M1 Nov 21, 2023
Copy link
Collaborator

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @ddubson. Please see review comments.

@ddubson ddubson force-pushed the issue-1369-pt2 branch 2 times, most recently from 109fbec to 13abb3e Compare December 13, 2023 16:52
@ddubson ddubson requested a review from jgrandja December 13, 2023 16:54
@jgrandja jgrandja modified the milestones: 1.3.0-M1, 1.3.0-M2 Jan 15, 2024
@ddubson ddubson force-pushed the issue-1369-pt2 branch 2 times, most recently from 2b3109d to f06d8fa Compare January 16, 2024 17:56
@jgrandja jgrandja changed the title Add OAuth2TokenEndpointAuthenticationSuccessHandler Add OAuth2AccessTokenResponseAuthenticationSuccessHandler Jan 17, 2024
Copy link
Collaborator

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there @ddubson ! After you update based on review comments, we should be good to merge.

@jgrandja jgrandja changed the title Add OAuth2AccessTokenResponseAuthenticationSuccessHandler Add ability to customize the access token response Jan 29, 2024
@jgrandja jgrandja closed this in d4ae69b Jan 29, 2024
jgrandja added a commit that referenced this pull request Jan 29, 2024
@jgrandja
Copy link
Collaborator

Thanks for the updates @ddubson. This is now merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Simplify customizing the access token response
3 participants