Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UsersTable doesn't have any email validation rules #1106

Open
mehov opened this issue Nov 28, 2024 · 6 comments
Open

UsersTable doesn't have any email validation rules #1106

mehov opened this issue Nov 28, 2024 · 6 comments

Comments

@mehov
Copy link

mehov commented Nov 28, 2024

Shouldn't \CakeDC\Users\Model\Table\UsersTable offer validation rules for email?

\Cake\Validation\Validator::email() can be used. (CakePHP Book → Modelless Forms → Creating a Form is an example.)

For an experiment, I added:

$validator
    ->email('email');

... right here:

public function validationDefault(Validator $validator): Validator

Worked perfectly for me, but I understand I'm probably missing a bigger picture as this could be not for every use case.

And yet, for a rather common use case where email is needed, I would argue the plugin should somehow recognise it and enable the email validation out of the box.

@mehov
Copy link
Author

mehov commented Nov 28, 2024

At the same time, username is required:

->notEmptyString('username');

… which clearly some people don't want and this plugin actually allows using email instead.

I have all of that configured and the users controller, processing the login request, does recognise that. So the UsersTable validation rules should recognise that too?

My current workaround when saving a user that shouldn't have a username is:

$validator = $this->Users->getValidator('default');
$validator->remove('username');
$this->setValidator('default', $validator);
$user = $this->Users->newEntity($data);
$result = $this->Users->save($user);

@steinkel
Copy link
Member

The plugins provides "one way" of doing things, but all projects are different, for example your case where you don't need the username but only the email. That's the reason we've tried to make it extendable https://github.com/CakeDC/users/blob/14.next-cake5/Docs/Documentation/Extending-the-Plugin.md in an easy way, so you can use what you like and replace what you don't need. I think you are looking for a custom UsersTable, implementing your own validation rules and columns.

@mehov
Copy link
Author

mehov commented Nov 28, 2024

Right, I just believed I am not asking for anything niche enough to warrant a custom table. After all, I am wanting to use email instead of username, which is a concept already known, common enough, and covered in the plugin documentation.

You even already have a flag like this implemented:

public bool $isValidateEmail = false;

if ($this->isValidateEmail) {
$rules->add($rules->isUnique(['email']), '_isUniqueEmail', [
'errorField' => 'email',
'message' => __d('cake_d_c/users', 'Email already exists'),
]);
}

Shouldn't isValidateEmail imply $validator->email('email') then?

You also have these flags:

## Using the user's email to login
You need to configure 2 things (version 9.0.4):
* Change the Password identifier fields and the Authenticator for Forms
configuration to let it use the email instead of the username for
user identify. Add this to your config/users.php:
```php
'Auth.Identifiers.Password.fields.username' => 'email',
'Auth.Authenticators.Form.fields.username' => 'email',
```

If (either of?) these are present, can't we tell confidently enough that email needs to be validated?

I understand though that deciding at this point that username is not wanted maybe is a stretch.

Either way, in my case I am happy with the workaround of $validator->remove('username') for now.

UPD simply removing the username validator rule is not enough, will also have to:

  • provide an empty value to satisfy database column not having a default value, and
  • hijack the table rules checking to prevent Username already exists error when another record with empty username already exists.
// Get default UsersTable validator from cakedc/users
$validator = $this->Users->getValidator('default');
// Remove the username field we don't need
$validator->remove('username');
// Write the validator back to the table
$this->setValidator('default', $validator);

// Force accepting empty username
\Cake\Event\EventManager::instance()->on('Model.beforeRules', function($event) {
    if (!($event->getSubject() instanceof \CakeDC\Users\Model\Table\UsersTable)) {
        return;
    }
    $event->stopPropagation();
    return true;
});

// Create a new User entity
$user = $this->Users->newEntity([
    'email' => $data['email'],
    'username' => '',
]);
// Save
$userResult = $this->Users->save($user);

Finally, if we can eventually set the email to be preferred, it would be only fair to apply the same rigorous checks to it being unique etc., as there are for the username.

@steinkel
Copy link
Member

I see your point, and I agree we add validation for the email field by default. Thank you.

@mehov
Copy link
Author

mehov commented Nov 28, 2024

Thank you!

@mehov
Copy link
Author

mehov commented Nov 29, 2024

@steinkel okay, after sleeping on it, I have to agree extending \CakeDC\Users\Model\Table\UsersTable with a custom class makes more sense for now given the circumstances, at least until this is implemented in the plugin.

If someone else finds this, here's my users table that prioritises email over username:

<?php

namespace App\Model\Table;

use Cake\Validation\Validator;
use Cake\ORM\RulesChecker;

class UsersTable extends \CakeDC\Users\Model\Table\UsersTable
{

    public function initialize(array $config): void
    {
        parent::initialize($config);
        $this->setEntityClass('CakeDC/Users.User');
    }

    /**
     * @param \Cake\Validation\Validator $validator
     * @return \Cake\Validation\Validator
     */
    public function validationDefault(Validator $validator): Validator
    {
        $validator = parent::validationDefault($validator);
        $validator->remove('username');
        $validator
            ->requirePresence('email', 'create')
            ->notEmptyString('email');
        return $validator;
    }

    /**
     * @param \Cake\ORM\RulesChecker $rules
     * @return \Cake\ORM\RulesChecker
     */
    public function buildRules(RulesChecker $rules): RulesChecker
    {
        $rules->add($rules->isUnique(['email']), '_isUnique', [
            'errorField' => 'email',
            'message' => __d('cake_d_c/users', 'Email already exists'),
        ]);
        return $rules;
    }

}

Note: Because there's no way to tell the parent buildRules() not to enforce isUnique(['username']), I had to overwrite it completely. And because (according to plugin code search) $isValidateEmail is not used (as in, read) anywhere else, I decided not to have it in my table class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants