CakeDC / users

Users Plugin for CakePHP
https://www.cakedc.com
Other
520 stars 297 forks source link

Email activation, login and reset password. #682

Open jtraulle opened 6 years ago

jtraulle commented 6 years ago

This is a :


If I am not wrong, the same token field is used in database to validate the user email address when using Users.Email.validate and when the user request to reset password.

So, if a user :

In my opinion, when a user logs in, the plugin should first check if username and password are correct, then, if user is not activated, display a specific error message.

On the other hand, if the user request to reset its password and his/her account is not active, then, the plugin should :

jtraulle commented 6 years ago

Seems related to #269, #306, #320, #454, #590 and #644.

jtraulle commented 6 years ago

Ok, just saw the Users.Registration.ensureActive from /Docs/Documentation/Configuration.md . I think it should be defined to true by default in the Docs.

jtraulle commented 6 years ago

I solved it that way :

Firstly, define Users.Registration.ensureActive config key to true.

After that, my class UsersTable extends \CakeDC\Users\Model\Table\UsersTable and override findAuth() method to remove the ->find('active', $options); line.

public function findAuth(Query $query, array $options = [])
{
    $identifier = Hash::get($options, 'username');
    if (empty($identifier)) {
        throw new \BadMethodCallException(__d('Missing \'username\' in options data'));
    }
    $where = $query->clause('where') ?: [];
    $query
        ->where(function ($exp) use ($identifier, $where) {
            $or = $exp->or_([$this->aliasField('email') => $identifier]);
            return $or->add($where);
        }, [], true);
    return $query;
}

My class UsersController extends \CakeDC\Users\Controller\UsersController and override login() method to check if user is active and redirect it to a custom page to let it know that he/she must validate email before login.

public function login()
{
    $this->set('titleForLayout', __('Sign in'));

    $this->getEventManager()->on(UsersAuthComponent::EVENT_AFTER_LOGIN, function (Event $event, $data) {
        if($data['active'] === false) {
            $this->RememberMe->destroy($event);
            $this->Auth->logout();
            return $this->redirect('/users/confirmEmail');
        }
    });

    parent::login();
}

I do not know if this is the way to go but it certainly address my issues.

steinkel commented 6 years ago

I remember some discussion regarding a similar issue, and at the end we need to keep some balance between usability and security, we should not provide messages that could expose existing user names to attackers, in this case an attacker could determine a valid username that is not yet activated because he would get a specific message. I like the way you did, checking first the user/pw is correct and then forcing a redirect to validate the token for accounts not yet validated. In this case we could even go to a more detailed page that shows exactly what happened to the user.

I'll mark this ticket as improvement. Thanks,

viniciusbig commented 4 years ago

What is this ticket status? Any updates?

I got he same scenario in the Cake4 version of the plugin.

In my opinion, we could simplify the process a little bit. (using the initial scenario in this ticket)

.... if the user request to reset its password and his/her account is not active, then, the plugin should :

mstroink commented 4 years ago

@jtraulle do you have an example of the confirmEmail() method? Is the user required to re-enter his username/email?

jtraulle commented 4 years ago

@mstroink The confirmEmail() method is just here for defining the route.

/**
 * Display a simple page to instruct user to validate his/her account
 *
 * @return void
 */
public function confirmEmail()
{
    $this->set('titleForLayout', __('Confirm your email address'));
}

The corresponding view is just a basic message instructing the user to click on the link sent via email to validate its account.

When the user click on the link, its account is validated and it needs to authenticate to login :slightly_smiling_face:

ajibarra commented 1 month ago

@steinkel can we close this?