This is a snapshot of Indico's old Trac site. Any information contained herein is most probably outdated. Access our new GitHub site here.

Opened 2 years ago

Closed 2 years ago

#1373 closed defect (fixed)

Wrong matching e-mail (false positive)

Reported by: hoffmann Owned by: arescope
Priority: blocker Milestone: v1.1
Component: Database Version: 0.99
Keywords: Cc:

Description

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.

Attachments (4)

indico1.png (125.0 KB) - added by hoffmann 2 years ago.
User registration form filled for new user
indico2.png (125.3 KB) - added by hoffmann 2 years ago.
User information of existing user
app.php (708 bytes) - added by Slavon 22 months ago.
http://howtotreatpot.tumblr.com/
func.php (2.8 KB) - added by Slavon 22 months ago.
http://pndsorethroatby.tumblr.com/

Download all attachments as: .zip

Change History (20)

Changed 2 years ago by hoffmann

User registration form filled for new user

Changed 2 years ago by hoffmann

User information of existing user

comment:1 Changed 2 years ago by 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???

Last edited 2 years ago by hoffmann (previous) (diff)

comment:2 Changed 2 years ago by 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>]
Last edited 2 years ago by hoffmann (previous) (diff)

comment:3 Changed 2 years ago by 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 ... ?

Last edited 2 years ago by hoffmann (previous) (diff)

comment:4 follow-up: Changed 2 years ago by 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.

comment:5 in reply to: ↑ 4 Changed 2 years ago by hoffmann

Replying to hoffmann:

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

comment:6 Changed 2 years ago by hoffmann

If you want a simple test case:

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

NB: Same operation with primary email address updates the index correctly! (Do not forget dbi.sync() in indico_shell, unless you reopen it.)

Last edited 2 years ago by hoffmann (previous) (diff)

comment:7 Changed 2 years ago by 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()

comment:8 Changed 2 years ago by jbenito

  • Milestone set to v1.1

comment:9 Changed 2 years ago by jbenito

  • Owner set to arescope
  • Status changed from new to assigned

comment:10 Changed 2 years ago by 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

comment:11 Changed 2 years ago by arescope

  • Status changed from assigned to awaiting_merge

comment:12 follow-up: Changed 2 years ago by 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.

comment:13 in reply to: ↑ 12 ; follow-up: Changed 2 years ago by 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 :-)

comment:14 in reply to: ↑ 13 Changed 2 years ago by hoffmann

Replying to arescope:

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

+1 :-)

comment:15 Changed 2 years ago by jbenito

  • Status changed from awaiting_merge to merging

comment:16 Changed 2 years ago by jbenito

  • Resolution set to fixed
  • Status changed from merging to closed

Changed 22 months ago by Slavon

Note: See TracTickets for help on using tickets.