Skip to content

Add OAuth2TokenResponseEnhancer to enhance the access token response #961

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

Closed
wants to merge 1 commit into from

Conversation

Hccake
Copy link

@Hccake Hccake commented Nov 4, 2022

Closes gh-925

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

@Hccake As mentioned in this comment:

I feel the duplication of initializing the defaults for OAuth2AccessTokenResponse.Builder is quite minimal and don't think there is much value for reuse at this point.

The proposed OAuth2TokenResponseEnhancer is a fine-grained interface and I'm not sure how much use it will receive at this point. Although AuthenticationSuccessHandler is more coarse-grained, it provides greater flexibility in customizing the access token response. I still feel the code duplication for initializing the OAuth2AccessTokenResponse.Builder in the consuming application is acceptable. Please keep in mind that this project is a framework and code customization is expected in the consuming application. We do our best to allow for code reuse but it's also expected that the consuming application will customize on their end. If the code duplication is quite an effort then we will look at improving. But in this case, the code duplication is quite minimal and adding a new interface does not make sense at this point.

Either way, I will leave gh-925 open and will monitor the comments to see if further improvements are needed.

@jgrandja jgrandja closed this Nov 10, 2022
@jgrandja jgrandja self-assigned this Nov 10, 2022
@jgrandja jgrandja added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 10, 2022
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

Successfully merging this pull request may close these issues.

3 participants