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)
Change History (20)
Changed 2 years ago by hoffmann
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@…'
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???
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>]
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 ... ?
comment:4 follow-up: ↓ 5 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:
- 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.
NB: Same operation with primary email address updates the index correctly! (Do not forget dbi.sync() in indico_shell, unless you reopen it.)
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: ↓ 13 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: ↓ 14 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
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
User registration form filled for new user