Skip to content

Conversation

@GeorgKott
Copy link
Contributor

password validation has error when email doesn't contain the @ symbol

@datamweb datamweb added the GPG-Signing needed Pull requests that need GPG-Signing label Apr 24, 2023
@paulbalandan
Copy link
Member

How can an email address not contain the @ symbol?

@GeorgKott
Copy link
Contributor Author

How can an email address not contain the @ symbol?

Because we check the email for a valid value and then check the password with the same email value, even if the email is invalid. And any value can be sent from the frontend

@datamweb
Copy link
Collaborator

@GeorgKott Thanks to PR.

Please answer these questions?

  1. Do you use service('passwords') directly?
  2. Why not use defined rules for email validation?
    /**
    * The validation rules for email
    *
    * @var string[]
    */
    public array $emailValidationRules = [
    'required',
    'max_length[254]',
    'valid_email',
    ];

Please sign the all commits:

https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

and please write php unit test see:

https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#unit-testing

@datamweb datamweb added the tests needed Pull requests that need tests label Apr 24, 2023
@kenjis
Copy link
Member

kenjis commented Apr 24, 2023

Because we check the email for a valid value and then check the password with the same email value, even if the email is invalid. And any value can be sent from the frontend

If the email is invalid, wouldn't the validation fail and complete at that point?
Please write test code to reproduce that error.

@datamweb datamweb added the waiting for info Issues or pull requests that need further clarification from the author label Apr 28, 2023
@GeorgKott GeorgKott closed this Apr 30, 2023
@GeorgKott GeorgKott deleted the fix_validation_password branch May 1, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GPG-Signing needed Pull requests that need GPG-Signing tests needed Pull requests that need tests waiting for info Issues or pull requests that need further clarification from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants