Code review comment for lp:~mhall119/django-openid-auth/fixes-642132

Revision history for this message
Anthony Lenton (elachuni) wrote :

Hi Michael,

Thanks for your work on this. The code around lines 8-10 of the diff would need to be a bit more robust when updating the User's username, as there might be some other user in the system with the new username already.

I think the underlying problem is what we really want to do in this case (ie if 'joesmith' changes his openid username to 'joebobs', but you already have a 'joebobs' in your system). Skipping the username update in that case would work but it'll leave you with out-of-sync data. Refusing to sign the user in would be quite violent, and merging the two accounts would be definitely a bad idea. What would work in your use case?

Sites with a single OpenID provider would be less affected by this issue, but it could still bite you.

Otoh, I'm not sure why django-openid-auth doesn't currently update username together with fullname and email when OPENID_UPDATE_DETAILS_FROM_SREG is True, but using a second setting for this as your code does seems like a good way to make it work for everybody.

« Back to merge proposal