Wrong matching e-mail (false positive)

[hoffmann]

Here is a tough one, and unfortunately it is quite urgent.

We have two users (Anne Green and Anne Green), who requested an account within some months distance. Due to operator error the second request got merged into the first account, although it is indeed a different person.

Everything has been cleaned up so far, I thought, but it seems to be impossible to request an account with the e-mail of the second requester again. See files attached:

indico1.png - Form filled for new user

indico2.png - user information of the existing user

When requesting the account for the new user, we get the message that the user already exists with address 'anne.green@…' and the usual suggestion to re-sent the password.

[hoffmann]

User registration form filled for new user

[hoffmann]

User information of existing user

[hoffmann]

Trying to investigate in the database, the match does not seem to run correctly:

In [119]: ah=AvatarHolder()

In [120]: ah.match({'email':'agreen@physics'})[0].getEmail()
Out[120]: 'anne.green@nottingham.ac.uk'

In [121]: ah.match({'email':'agreen@physics'})[0].getSecondaryEmails()
Out[121]: []

Why does match() deliver the avatar for anne.green@nottingham, when asked for agreen@physics???

[hoffmann]

Tried /opt/indico/bin/maintenance/indexes/reindexusers.py, no change.

Oh, and yes: match() only returns one object:

In [4]: ah.match({'email':'agreen@physics'})
Out[4]: [<MaKaC.user.Avatar object at 0xa3d5a6c>]

[hoffmann]

I am not sure, if I can debug this until the end, but in case someone catches up, this may help save some time:
browser/indico/indico/MaKaC/user.py indicates how the matching is done in the AvatarHolder? class. Indeed it uses the indexes, which seems to confirm that the indexing is corrupted:

In [16]: IndexesHolder().getById('email').matchUser('agreen@physics')
Out[16]: ['269']

In [17]: ah.getById(IndexesHolder().getById('email').matchUser('agreen@physics')[0]).getEmail()
Out[17]: 'anne.green@nottingham.ac.uk'

See you later ... ?

[hoffmann]

Well, I beleive I found the flaw in the design, looking at the source for the EmailIndex class:

indexUser will add email and secondaryEmails (plural!) to the index for a given user.

unindexUser will remove email and secondaryEmails from the index for a given user.

In between, whenever the secondaryEmails change or are removed, they apparently are not removed from the index, although the update of email seems to trigger a correct index update.

[hoffmann]

Replying to hoffmann:

getEmails() should be used instead of the successive getEmail() and getSecondaryEmails() calls.

[hoffmann]

If you want a simple test case:

• Create a user (default InDiCo? Authenticator db)
• Go to ​http://your.indico.site/indico/userDetails.py?userId=XXX
• Edit secondary email there: 123@…
• Test with indico_shell for example ei.matchUser('123@domain.net')
• It is empty.

Conclusion: Secondary emails that have been modified through the user interface are not updated in EmailIndex.

[hoffmann]

Here is the workaround to cleanup the database. Sorry, I cannot provide more right now. Probably both the userDetails and userMerge interface code should be re-read carefully regarding index update. Also a repair script alike reindexusers.py in browser/indico/bin/maintenance/indexes could be helpful.

This is my recipe for "repair":

ei=IndexesHolder().getById('email')
n=ei.matchUser('agreen@physics') # partial match, OK!
ei._withdrawItem('agreen@physics.edu.au', n[0]) # use full address (index value) here!
dbi.commit()

[arescope]

Hi,

There was a bug that when editing the secondary email in the user profile page the reindexing was not being applied. I already made the patch.

With that patch all the related problems will not occur again.

Best regards,

Alberto

[hoffmann]

I would also fix the syntax in indexUser and unindexUser as described above (use getEmails(), not getEmail() and conditionally getSecondaryEmails).

But do as you want.

[arescope]

Replying to hoffmann:

I would also fix the syntax in indexUser and unindexUser as described above (use getEmails(), not getEmail() and conditionally getSecondaryEmails).

But do as you want.

It was already patched like that :-). I forgot to tell you :-)

[hoffmann]

Replying to arescope:

It was already patched like that :-). I forgot to tell you :-)

+1 :-)

[jbenito]

commit: c69c96395895e756dd64977d7b8432b4895aa3d4

[Slavon]

​http://howtotreatpot.tumblr.com/

[Slavon]

​http://pndsorethroatby.tumblr.com/