Zizaco / confide

Confide is a authentication solution for Laravel 4
1.19k stars 258 forks source link

Eliminate dependency on email address as a reliable handle for user #523

Open tblanchard opened 9 years ago

tblanchard commented 9 years ago

Not all my users have email, some have more than one email. Since an email is just a bit of contact info, I don't have it as a field in user. I have it as a related table so users can have multiple. This keeps confide from being useful to me and anyone else who didn't tie email address to user identity.

The most egregious evidence of this is

    public function userByResetPasswordToken($token)
    {
        $email = $this->passService->getEmailByToken($token);
        if ($email) {
            return $this->repo->getUserByEmail($email);
        }
        return false;
    }

because I can't look up a user by email. This change set adds the primary key for the user to the password reset table.

    public function requestChangePassword(RemindableInterface $user)
    {
        $token = $this->generateToken();

        $values = array(
            'user_id' => $user->getAuthIdentifier(),
            'email'=> $user->getReminderEmail(),
            'token'=> $token,
            'created_at'=> new DateTime
        );

        $table = $this->getTable();

        $this->app['db']
            ->connection($user->getConnectionName())
            ->table($table)
            ->insert($values);

        $this->sendEmail($user, $token);

        return $token;
    }

and so now the user is looked up by primary key instead of by email. There are a few more changes around making email optional in the database when generating controllers and database tables and there is more support for using username (although this isn't necessarily unique in my system either).

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-23.51%) to 74.83% when pulling bbfa55ff8408913eefec504f83c9f71235ab9873 on tblanchard:master into f9c3d09f68589cc3242a3527464bb871d9815a0f on Zizaco:master.