Skip to content

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jun 10, 2022

Fixes #224

@kenjis kenjis mentioned this pull request Jun 10, 2022
@kenjis kenjis added the bug Something isn't working label Jun 10, 2022
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is optional, workaround looks good.

@datamweb
Copy link
Collaborator

datamweb commented Jun 11, 2022

@kenjis It seems that changes to file NothingPersonalValidator.php should be applied to this PR.

/**
* Returns true if $password contains no part of the username
* or the user's email. Otherwise, it returns false.
* If true is returned the password will be passed to next validator.
* If false is returned the validation process will be immediately stopped.
*/

@kenjis
Copy link
Member Author

kenjis commented Jun 11, 2022

@datamweb What do you mean?
NothingPersonalValidator is enabled by default, and I did not see any errors.

By the way, the file has functions staring with \.

$password = \strtolower($password);

@datamweb
Copy link
Collaborator

What do you mean?

@kenjis I have such an error!! and I used the following code in 55 and 163 to fix it.

         if(is_null($user->username)){
             return true;
         }

err

@kenjis
Copy link
Member Author

kenjis commented Jun 11, 2022

@datamweb Oh, thanks! I'm using PHP 8.0, so I did not notice.

kenjis added 3 commits June 12, 2022 13:42
ErrorException: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
@kenjis kenjis force-pushed the fix-email-only-register branch from 065b99e to b41b4b7 Compare June 12, 2022 04:53
@kenjis
Copy link
Member Author

kenjis commented Jun 12, 2022

@datamweb Fixed the error. Is this okay?

@datamweb
Copy link
Collaborator

Is this okay?

@kenjis is okay.
Just for user friendliness, it's not better to make changes in

$html .= "<tr><td>Username</td><td>{$user->username}</td></tr>";

create?

@kenjis
Copy link
Member Author

kenjis commented Jun 12, 2022

If the username is null, the toolbar output will be blank.
I'm okay with it.

Screenshot 2022-06-12 16 42 15

@datamweb
Copy link
Collaborator

Thanks @kenjis.

$userName = \strtolower($user->username);
$email = \strtolower($user->email);
$userName = strtolower($user->username ?? '');
$email = strtolower($user->email);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we anticipate users trying to work without emails?

Suggested change
$email = strtolower($user->email);
$email = strtolower($user->email ?? '');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is out of scope of this PR.

The current implementation requires email. If we support users without email, I guess more to work.

@kenjis kenjis requested a review from lonnieezell June 12, 2022 23:25
@kenjis kenjis merged commit e61edc3 into codeigniter4:develop Jun 13, 2022
@kenjis kenjis deleted the fix-email-only-register branch June 13, 2022 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Register without username
5 participants