Banno / getsentry-ldap-auth

A Sentry extension to add an LDAP server as an authention source.
Apache License 2.0
163 stars 55 forks source link

Login: UserEmail return more than 2 values #18

Closed prawn-cake closed 7 years ago

prawn-cake commented 7 years ago

After upgrading sentry to the latest (8.11.0) version and the plugin to the 2.3, I started getting some weird issues, I ended up with duplicates of UserEmail entries for my users.

Possible reason is here

Or at least you misuse .get(...) method. This at least should be as follows:

try:
    userEmail = UserEmail.objects.get(user=user)
except UserEmail.DoesNotExist:
    userEmail = UserEmail.objects.create(user=user)
plumdog commented 7 years ago

I would like "me too" this issue, but with a slight tweak. I added an alternate email address to my account (when logged in to an LDAP account). When I next logged in it 500ed, saying that the UserEmail.objects.get(user=user) returned 2 results.

I think maybe this should look something like:

userEmail, _ = UserEmail.objects.get_or_create(user=user)
userEmail.email = ldap_user.attrs.get('mail', ' ')[0] or ''
userEmail.save()

Or possibly update_or_create if that exists in all Django versions we need to support.

prawn-cake commented 7 years ago

And now sentry supports multiple emails per user (primary and alternatives). Preparing some pull request for that

barronhagerman commented 7 years ago

I don't have a good dev environment for this project right now, but I'm pretty sure this was fixed by #17.