Skip to content

Make WebAuthnAuthenticationTokenRequest Serializable #16481

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
jzheaux opened this issue Jan 24, 2025 · 5 comments · Fixed by #16602
Closed

Make WebAuthnAuthenticationTokenRequest Serializable #16481

jzheaux opened this issue Jan 24, 2025 · 5 comments · Fixed by #16602
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Jan 24, 2025

WebAuthAuthenticationTokenRequest should be serializable. It isn't, however, since some of its components aren't serializable. Either they should also be serializable or marked as transient.

@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: bug A general bug labels Jan 24, 2025
@jzheaux jzheaux added this to the 6.4.x milestone Jan 24, 2025
@franticticktick
Copy link
Contributor

Hi @jzheaux, could you assign this ticket to me please?

@jzheaux jzheaux modified the milestones: 6.4.x, 6.4.3 Jan 25, 2025
@jzheaux
Copy link
Contributor Author

jzheaux commented Jan 25, 2025

Sure thing, @franticticktick. Will you please add it on the 6.4.x branch, please?

@franticticktick
Copy link
Contributor

@jzheaux there seems to be some confusion here. WebAuthnAuthenticationRequestToken contains RelyingPartyAuthenticationRequest, and it already contains PublicKeyCredentialRequestOptions.
This issue should be solved here, and I see that this ticket is still in progress. Besides, it was not introduced as a bug, which means the changes will be merged into main. Therefore, we need to decide which branch to add these changes to.

@jzheaux
Copy link
Contributor Author

jzheaux commented Feb 3, 2025

My concern here is that WebAuthnAuthenticationRequestToken already implements Serializable and is thus at risk of having that ID change should any of its internals change in 6.5. It would mean that folks using Java serialization for this component would break when moving from 6.4 to 6.5. I'd rather mitigate that risk by adding an ID in 6.4.x. I double-checked with @rwinch and we are on the same page.

As such the work in #16431 will need to be repeated in 6.4.x as part of this ticket.

So, let's please continue with the plan to target 6.4.x for this fix.

@franticticktick
Copy link
Contributor

@jzheaux thanks for the explanation :) I can make serializable WebAuthnAuthenticationRequestToken and its related classes like RelyingPartyAuthenticationRequest, PublicKeyCredential, AuthenticatorAssertionResponse etc. PublicKeyCredentialRequestOptions will remain as is, since it will be made serializable via #16431. But we must remember that some classes are already serializable and are located in the main branch, for example, Bytes. We need to make a backport to 6.4.x. And this should be taken into account now in #16431

franticticktick added a commit to franticticktick/spring-security that referenced this issue Feb 14, 2025
franticticktick added a commit to franticticktick/spring-security that referenced this issue Feb 14, 2025
franticticktick added a commit to franticticktick/spring-security that referenced this issue Feb 14, 2025
jzheaux added a commit that referenced this issue Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants