Love Fuel?    Donate

FuelPHP Forums

Ask your question about FuelPHP in the appropriate forum, or help others by answering their questions.
E-mail is not unique?
  • 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.

    Is this a mistake or some design feature?
  • 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 think, it should be two separate indexes on username and on email...
  • That doesn't really make any difference, it will remain unique, even with a second index.
  • Really?
    Is not now able to make two or more different accounts with the same email?
  • HarroHarro
    Accepted Answer
    No. email is used as a login name by Auth, so it must be unique.
  • 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:
    oil refine migrate --packages=auth

    That index (username,email) not means username is unique and email is unique. That index said that both together are unique.

    Example: Add username "usernameA" and mail "adressA". Now I can add:

    username / email

    usernameA adressB
    usernameA adressC
    usernameB / adressA
    usernameC / adressA

    Thats all valid and insert new users. So I have 5 users with identical username or email-adress.
  • 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.
  • Yes, simpleauth check for identical e-mail-adresses. So thats fine.

    Auth::create_user('UsernameA', '123456', 'mail@mail.tld');
    Auth::create_user('UsernameB', '123456', 'mail@mail.tld');

    I get for second create_user() a exception with
    throw new \SimpleUserUpdateException('Email address already exists', 2);


    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.
  • No, I just said that simpleauth check for it. So you can't add dublicates with correct auth-function call :)

    ->where('username', '=', $username)
    ->or_where('email', '=', $email)

    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.

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

In this Discussion