I looked at migration for users table and sow that: \DBUtil::create_index($table, array('username', 'email'), 'username', 'UNIQUE');
It means we must have unique pair of id + email, but not id or email separately. It seemed have broken logic. It means we can have two or more different users with the same email, it is not good, I think.
This is because Auth::login() accepts both a username and an email address as valid login credential. Lots of applications use email for login these days.
This means email has to be unique. Nobody is stopping you from removing this restriction, but that means login via email may fail for non-unique email addresses, as Auth uses a "limit 1" when trying to find the user.
I want report this bug too. I have created a register and confirmation page and after add my first user and look in database, I see that mistake. I have created users-table with:
If you use Auth, you should use Auth to create users, and not use direct database access.
It is not a requirement of the user table, we have auth drivers in use that authenticate on username only, so no problem with duplicate emails. The index is just there to speed up lookups, not to enforce a constraint.
If you have this constraint, simply create the indexes you want in a separate app migration.
I use email als login (username_post_key='email'), there is no speed up, because the index begins with username and without username mysql not use this as a index. When username is unqiue, there is no speed up for field email as pair. Username is "unique" so second column is needless. ;)
The same email multiple times in database is fine? How would you reset a password for a single user account that have multiple entries in database with same email-adress?
Again, if you want to use it differently from the default behaviour, nothing is stopping you from altering it. You can't please all of the people all of the time.
The create_user() method of the Auth package will ensure that both username and email are unique. You only run into a problem if you bypass Auth and start adding or altering data directly into the database. It isn't designed for that, but if you do, know the constraints before you do.
It is pointless to knowingly corrupt the integrity of the user table just because you can, and then say it doesn't work the way you expect or want it to.
You need fetch the email and that field "email" is not unique or a index. So maybe you should add a unique to email as second index?
I do not want to be annoying, but a unique index for (username,email) make no sense. username must be unique in your system, so here is no benefit to add email to this username-index. It can not find more than one row for a username, so here is no "speed up", or am I completely wrong?
First, you conclude that the Simpleauth Auth driver does NOT allow you to add duplicate usernames or duplicate email addresses. And then you say that email is not unique and you want to enforce that with an index? Bit of a contradiction, you just said it was unique?
I can't tell you why that index is there, it's been there for years, and the original Author of the Auth package is no longer part of the development team. Auth probably needs a migration that drops the index and creates two new ones, one on username and one on email, but I can't oversee what side-effects that has.
That don't use email in index, because its "OR", so here is no "speed up". Its possible that he not use a Index at all, because OR said he must read all rows. Maybe is much faster to use UNION with two selects here.
In that select it should select "id,username,email" and not "*", all fields. Its not good practise and not needed.
When you set unique index on username and a second on email, than you don't need that select here. You can directly call INSERT with IGNORE and when result is empty, the username or email was already added in users.
When the result is empty (no id is added), you can take a look which one was already included, when you need this. Maybe a user should call this with a other function, when he need this info.
Instead of
if (in_array(strtolower($email), array_map('strtolower', $same_users->current())))
you can simply write strtolower($email)===strtolower($same_users{0}['email'])
Sorry, I don't want steal your time. But that auth-package should someone review. ;)
Yeah, Jelmer was well known for writing complicated code. ;)
If you feel up to it, you can always send us a PR against 1.9/develop, code reviews are always welcome.
p.s. if you do, don't alter existing migrations, but add a new one if needed. Also, take into account that something you want to add or drop may already or no longer present, so you need to capture that.
Donate
