Reliable updating of email-based indices

pepyatka / pepyatka-server

Server for Pepyatka

79 stars 27 forks source link

Reliable updating of email-based indices #266

Closed indeyets closed 9 years ago

indeyets commented 9 years ago

drop old index if email has changed
do not create index for empty email

berkus commented 9 years ago

Look gut now.

berkus commented 9 years ago

Lets not merge it at the moment.

indeyets commented 9 years ago

This pull-request needs one more fix: during the update, we need to be sure, that email-index is not taken. otherwise we can end up with situation, when several users have same email address, but index points only to the last one, meaning that other accounts are not able to "restore" the password

indeyets commented 9 years ago

the check should, probably, go to User.prototype.emailIsValid

epicmonkey commented 9 years ago

Need a couple of tests:

userA, userB are in the system, userA changes email to newEmail, userB can change email to userA
userA, userB are in the system, userA changes email to newEmail, userB changes email to userA, userA can reset password over newEmail
userA, userB are in the system, userA changes email to newEmail, userB changes email to userA, userB can reset password over email userA
userA, userB are in the system, userB changes email to userA -> error

indeyets commented 9 years ago

@epicmonkey I am not sure what would be the proper way to test that proper user's password is reset

epicmonkey commented 9 years ago

Let's just make sure you can fire controller's action like here: https://github.com/pepyatka/pepyatka-server/blob/development/test/functional/passwords.js#L32