Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Wrong parameters order and wrong naming#3618
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Wrong parameters order and wrong naming on "Using Password Encoders" section
cordoval commentedFeb 28, 2014
expedient shipment /o/ :baby: :+1: |
weaverryan commentedMar 8, 2014
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 Assuming I'm reading all of this correctly, I suggest 3 changes:
@robertodormepoco does this make sense? Would you be willing to make these changes (assuming nobody disagrees with me)? Thanks! |
xabbuh commentedMar 10, 2014
👍 for@weaverryan's suggestion |
robertodormepoco commentedMar 11, 2014
@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 commentedMar 12, 2014
@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 publicfunction 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 What do you think? |
wouterj commentedMay 21, 2014
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! :) |
Wrong parameters order and wrong naming on "Using Password Encoders" section