Describe the bug I wondered if OidcIdToken should implement equals. While running some test, I realized that the claims on OidcIdToken are not compared when using assertEquals.

Interestingly, OAuth2AccessToken which follows the same pattern does implement it.

Expected behavior claims (and potentially other fields) are respected when comparing via equals. This might be considered a breaking change.

Comment From: jzheaux

Hi, @MatthiasWinzeler. It may be valuable, though I'm not sure yet. Usually, we try and wait to add code until there is a clear reason. Is there something that you are trying to do in your production code that needs or is expecting OidcIdToken#equals to compare the claims?

If not, then I suggest we close this for now and reopen once the need arises.

This might be considered a breaking change.

That's correct. Unless it is tied to problematic behavior, we'd defer this change to Spring Security 7.

Comment From: MatthiasWinzeler

@jzheaux For me, the benefit would mainly be in testing - so that assertEquals can be used. I was recently building an application based on Spring Authorization Server and tried to compare its OAuth2Authorization (which holds OidcIdToken next to many other fields). assertEquals for two OAuth2Authorizations was failing (since the ID token claims were not the same) but it was very hard to track down the difference. Every other field (for example the OAuthAccessToken was properly implementing equals, so I thought it was just forgotten on OidcIdToken.

That's my reason - I think there might be others. I am far away from state of the art Java best practices these days, but isn't it a code smell to not override equals when a subclass adds fields?

Comment From: jzheaux

Thanks for the extra detail, @MatthiasWinzeler. Yes, I agree it is smelly in this case, so I've slated this for the 7.0 release.

Comment From: baezzys

Hi @jzheaux. Can I try this issue?

Comment From: jzheaux

Sure, @baezzys! The contribution is much appreciated. Remember that in this particular case (due to the breaks-passivity label), it won't be merged until Spring Security 7.