Skip to content

Remove the need for @JsonSerialize when serializing authorization proxy objects with Jackson #15687

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 30, 2024
Merged

Remove the need for @JsonSerialize when serializing authorization proxy objects with Jackson #15687

merged 1 commit into from
Aug 30, 2024

Conversation

kse-music
Copy link
Contributor

Closes gh-15661

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 25, 2024
@kse-music kse-music changed the title Improve Cglib Object serialize Reduce the need for @JsonSerialize when serialize cglib object Aug 25, 2024
@kse-music kse-music changed the title Reduce the need for @JsonSerialize when serialize cglib object Reduce the need for @JsonSerialize when serialize proxy objects generated by AuthorizeReturnObject Aug 26, 2024
@kse-music kse-music force-pushed the gh-15661 branch 3 times, most recently from afff6f6 to 023387f Compare August 26, 2024 04:15
@kse-music
Copy link
Contributor Author

@jzheaux I wonder why the spring-security-saml2-service-provider:opensaml4Test task execution failed , and I didn’t make any changes.

jzheaux
jzheaux requested changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@jzheaux jzheaux 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 PR, @kse-music! Please see my inline feedback.

@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 26, 2024
@jzheaux jzheaux added this to the 6.4.x milestone Aug 26, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Aug 26, 2024 •
edited
Loading

@kse-music there is a flaky test in that module. It has been disabled for now. Please try rebasing and push to your branch again.

@kse-music kse-music force-pushed the gh-15661 branch 4 times, most recently from 001fb02 to e7ebb32 Compare August 27, 2024 07:32
@kse-music
Copy link
Contributor Author

@jzheaux I have completed the feedback, please help review it. Thanks

@kse-music kse-music force-pushed the gh-15661 branch 2 times, most recently from e625dff to a8e073e Compare August 28, 2024 07:05
@jzheaux
Copy link
Contributor

jzheaux commented Aug 29, 2024

@kse-music, having researched this a bit further, I believe there is a more elegant way to achieve this and that is by configuring with ProxyFactory#setOpaque(true). This will keep from exposing the advice methods on the CGLIB proxy altogether, preventing the cycle.

Would you be able to update this PR to set the opaque value to true for both ProxyFactory instances in AuthorizationAdvisorProxyFactory? Also, please add a test in AuthorizationAdvisorProxyFactory that confirms that JSON serialization works with a POJO and an out-of-the-box ObjectMapper.

Please also remove the entire Working with Jackson section as no Jackson configuration is necessary in this case.

@kse-music
Copy link
Contributor Author

@jzheaux PR has been updated

@jzheaux jzheaux changed the title Reduce the need for @JsonSerialize when serialize proxy objects generated by AuthorizeReturnObject Remove the need for @JsonSerialize when serializing authorization proxy objects with Jackson Aug 30, 2024
@jzheaux jzheaux removed this from the 6.4.x milestone Aug 30, 2024
@jzheaux jzheaux added this to the 6.4.0-M4 milestone Aug 30, 2024
@jzheaux jzheaux merged commit fd05c5a into spring-projects:main Aug 30, 2024
6 checks passed
@jzheaux
Copy link
Contributor

jzheaux commented Aug 30, 2024

Thanks, @kse-music! This is now merged into main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jzheaux jzheaux jzheaux approved these changes

Assignees

@jzheaux jzheaux

Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Projects
None yet
Milestone
6.4.0-M4
Development

Successfully merging this pull request may close these issues.

Investigate how to reduce the need for @JsonSerialize
3 participants
@kse-music @jzheaux @spring-projects-issues