Should OidcIdToken implement equals?

[MatthiasWinzeler]

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.

[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.

[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?

[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.