Wrong parameters order and wrong naming

[robertodormepoco]

Wrong parameters order and wrong naming on "Using Password Encoders" section

[cordoval]

expedient shipment /o/ :baby: :+1:

[weaverryan]

Actually, I'm not sure if this is correct, but it is confusing. Here's the codeblock currently:

    // fetch a user of type Acme\Entity\LegacyUser
    $user = ...

    $encoder = $encoderFactory->getEncoder($user);

    // will return $weakEncoder (see above)

    $encodedPassword = $encoder->encodePassword($password, $user->getSalt());

    // check if the password is valid:

    $validPassword = $encoder->isPasswordValid(
        $user->getPassword(),
        $password,
        $user->getSalt());

At the top, we're pretending to get some user out of the database, and of course, passwords are encoded in the database. We also have some magic $password variable, which is some plain text password (though that's not really explained) - you can see because we use it to create a new, encoded password. And finally, we're really showing 2 independent things here: (1) how you might encode a password and (2) (unrelated) how you might later check if a password is valid. But because it's one code block, it looks like we're using encodePassword so that we can then do isPasswordValid.

Assuming I'm reading all of this correctly, I suggest 3 changes:

1. Add a $plainPassword = '...'; // a password that was somehow submitted, e.g. when registering to the top of the code block

2. Rename all $password to this $plainPassword

3. Split this into 2 separate code blocks between encodePassword and isPasswordValid. In the second code-block, let's repeat the lines where we fetch the $user and show the $plainPassword.

@robertodormepoco does this make sense? Would you be willing to make these changes (assuming nobody disagrees with me)?

Thanks!

[xabbuh]

👍 for @weaverryan's suggestion

[robertodormepoco]

@weaverryan $user may be a new user (not in the persistent layer yet, i think the verb fetch is misused), and then the code shows how to get the right password encoder for the class of that $user

the latter check of a valid generated password may be a test case for the isPasswordValid method, it doesn't look misleading to me

[weaverryan]

@robertodormepoco Fair enough on the "fetch" part - it's true that you may be creating a new user here (or using some other type of storage altogether) :).

But, the code in this PR has a problem. The signature to isPasswordValid looks like this:

public function isPasswordValid($encoded, $raw, $salt)

And with this PR, the code says:

$validPassword = $encoder->isPasswordValid(
    $encodedPassword,
    $user->getPassword(),
    $user->getSalt());

The problem is that this means that we're saying that $user->getPassword() is the plaintext password - but that's never true. This line was really meant to be an example of how you'd check a password when logging in - by comparing the plain text password (which was somehow submitted by a user) to the encoded password (which is what you have stored on the user.

What do you think?

[wouterj]

This PR is replaced by #3858.

Thank you for notifying us to this doc issue :) If you do not agree with my changes, feel free to comment! :)