Remove deprecated org.keycloak.representations.idm.UserRepresentation#isTotp method#44004
Remove deprecated org.keycloak.representations.idm.UserRepresentation#isTotp method#44004pedroigor wants to merge 1 commit into
org.keycloak.representations.idm.UserRepresentation#isTotp method#44004Conversation
ba92394 to
3597851
Compare
3597851 to
a95d2b8
Compare
a95d2b8 to
fffa0cf
Compare
…#isTotp method Closes keycloak#43587 Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
fffa0cf to
3fc8ce7
Compare
stianst
left a comment
There was a problem hiding this comment.
Change itself looks good, but not sure if we should rather wait for 27 for this since it's a breaking change
|
I'll keep this one on the radar and mark it for 27. |
ahus1
left a comment
There was a problem hiding this comment.
I read the following in the docs:
If have JSON files you are using to import back up users that include the
totpfield, you can safely remove this field from your files as it is no longer used.
Not doing that will cause failure during user import because the field is unrecognized.
Is the data in the fields actually used? Can we just ignore it from the representation when reading it instead using JSON Ignore?
I'd say this behavior where I need to update a JSON manually that I imported from a previous version is something we usually avoid.
Can you think of a softer migration path?
I'm not sure how much it is used by people but if you look at most of places where we are using it today, I don't see any argument for keeping it:
IMO, we should just remove it. Even if it is a breaking change and forces people to manually update their representations. Hopefully, we will have some versioning in the feature that allows us to push people towards using a new version rather than breaking things. But one day people will need to update, regardless ....
As you suggested, I thought about ignoring unknown fields (not really keeping the field/getters/setters there). But we do validate the representation today, and I guess we want to continue doing so. |
Closes #43587