confide icon indicating copy to clipboard operation
confide copied to clipboard

Eliminate dependency on email address as a reliable handle for user

Open tblanchard opened this issue 9 years ago • 1 comments

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).

tblanchard avatar Mar 18 '15 21:03 tblanchard

Coverage Status

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

coveralls avatar Mar 18 '15 22:03 coveralls