Views were added, and the POC if fully functional (manually tested).
From now on I will stop amending my first commit, if people want to review/comment code.
I'm considering making the "request password" form (first screenshot) work only by e-mail address, as recommended here.

I strongly suggest you read this 👍 https://paragonie.com/blog/2016/09/untangling-forget-me-knot-secure-account-recovery-made-simple

Almost all password reset systems are "horrible" insecure, tld;dr explanation: by using a single token (basically a random username) it's possible for an attack to guess the existing token by using a timing attack. When you provide a token, the database will use the index to look for an existing record, each character is checked against a binary-tree until it finds a record. When a character has a hit this means the time takes longer (because it continues the traversing of the tree), when there is no hit the time takes shorter because it bails out. Now using this technique an attack can use this information to try as many combinations as possible until all characters matches!

This is similar to a brute-force attack to guess a username/ID, but with a major difference. There is no password, and therefor access is directly granted.

By using the SplitToken you have selector (random username) and a verifier (random password in a hashed format), meaning that an attacker first needs to guess the selector, and then when there is a positive match, try to guess the verifier. BUT! Once you provide a wrong verifier for the selector, you simple revoke the token and no further tries are possible, even then guessing the verifier is nearly impossible as you compare the hashes in constant time (always takes the same amount of time) which leaks no timing information, making guessing nearly impossible.

I actually created a library that uses this technique https://github.com/rollerworks/split-token (MPLv2.0 licensed because it depends on HiddenString) but for most applications using the technique descried in the linked article is already a good step.

@sstok thank you for the article, but I'm not sure implementing the suggestions from the article would be useful for most users The goal is to offer a basic feature, done right, for beginners. I'm pretty sure developers having tighter security requirements won't be using a maker command.
Hence I'm re-using a process from FOSUserBundle, which proved since a few years good enough for most, and follow some reasonable best practices (https://www.troyhunt.com/everything-you-ever-wanted-to-know/).

I like your approach, though. Did you consider make a "SplitTokenBundle"?

This PR is a really good idea, and would be very usefull. I tried it and I love very much the generated code.

Some really small problems:

• The generated twig files and folders do not follow coding stardards in their camelCased form (they should be snake_cased).
• In the controller, method request(), is there any reason that you inject Twig_Environment instead of using the simpler approch of $this->renderView() ?
• The suffix Action should be removed on controller's method checkEmailAction().

Also, I understand completely that you want to keep things simple and basic, and I agree. I just wonder if we could kinda incorporate some of @sstok suggestions by passing along the user email in the reset link. That would allow us to hash the reset token in the database, without adding much generated code and no other field in the entity. We even already use the UserPasswordEncoderInterface in the reset() method...

In fact, maybe that those should be command options:

The reset password system need to store random tokens for your user. Do you want to store those:

1. In your User entity
2. In a separate entity

Would you like to hash the stored tokens (more secure)?

1. Yes
2. No

Would you like to add an extra "selector" field to protect against timed attack (more secure)?

1. Yes
2. No

Obviously, that is a lot of work to imlement, but that keep things simple for the end user.

Thank you for the review @gsylvestre
I fixed the small problems, regarding \Twig_Environment at some point I wanted to render blocks independently, but I dropped this approach as it uses internal methods.

Regarding going further, I'm indecisive for now. I would like to get more feedbacks on this PR before going further regarding advanced features, about if and how it can integrates Maker bundle.

FYI. I don't have any plans for a bundle to integrate SplitToken, in practice you can register as many SplitToken factories as you want.

The only thing that might be interesting is a FormType to handle this format native, I actually did this for Park-Manager . But that's something for later as it's still experimental.

Sorry for the delay. I really want this - and it's on my list to review - but haven't had time yet. Thanks for getting it started!

@gsylvestre I was interested in getting more info from your above comments:

In fact, maybe that those should be command options:

The reset password system need to store random tokens for your user. Do you want to store those:

1. In your User entity
2. In a separate entity

I don't think we should do this - that's a level of customization that you can do if you want to, but I don't see any advantage to adding this flexibility (or is there a security implication?)

Would you like to hash the stored tokens (more secure)?

1. Yes
2. No

I saw your other comment before this, but can you explain this a bit more? If we only had 1 field in the database, how is the token hashed and how does that improve security?

Would you like to add an extra "selector" field to protect against timed attack (more secure)?

1. Yes
2. No

Maybe ;). I do think this is something to keep in mind as we get closer - it's probably not that much more work to implement, if indeed this is an industry-standard way of making more secure reset passwords.

Oh, I understand now about the hashed tokens: if we passed the email address in the reset URL, then we could look up the User by that email address, and then do a timing-safe check to see if the hash in the URL matches what we expect in the database.

Of course, then this also in theory allows for a timing attack on that "reset" URL, because you could use it to detect timing differences in return to determine if the email you entered is an email in the system :).

In general, I think we should be looking at other popular implementations (including FOSUserBundle, Laravel, Rails, whatever) and making sure we have the same safeguards at least as they do.

@weaverryan yes that's what I meant: if we hash the token in the database, we then need some kind of identifier to retrieve it. As you say, if we use the email, then an attacker can use timed attacks to find out if it's in the db... Really not a big concern imo, but still.

I was not proposing the "separate entity" option for security implication, not directly. But now that I have looked around and thought about it, I believe that we should always store the token in a new entity, instead of inside properties in the user class. See below...

Something like :

class PasswordResetToken 
{
    $id
    $selector //(crypto secure string, shorter)
    $token //(crypto secure string, longer)
    $requestedAt //(datetime)
    $user //(OneToOne unidir relation)
}

Pros:

• We do not mess with User class by adding new properties and methods. It is often already clogged, and I feel it is intrusive.
• We can then more freely add more fields and methods to our Token entity, like the generated "selector", or ie. if the developper want, he/she can add an expireAt field easily, or maybe use this entity for email confirmation purposes.
• Indirectly more secure because we can now add the "selector" field
• Until a password reset request is made, no token are generated. And no fields are sitting there being NULL and sad in the user table.
• Better model structure, added flexibility

Cons:

• Extra entity and database table are generated
• more complicated?

In fact, is it really more complicated for the developper to understand what is going on? I believe even beginners will get this new entity easily.
And we are doing a really secure password reset. Why settle for less? We expect Symfony to provide top secure features.

BTW, Laravel is storing reset tokens in a new table, but use the email as the selector.

I can implement these changes next week, if the coding work is a problem.

Thank you for your inputs both of you, and for your review @weaverryan, that's great to see things moving. Let's make this maker happen!

@gsylvestre I like that last idea you had, having a separate entity. It is not making things overly complex, but it both adds some security from what I got & code is a little bit cleaner in my opinion (it prevents User entity from growing too big...). I can work on it while fixing the other comments, but I would appreciate your review.

Of course, then this also in theory allows for a timing attack on that "reset" URL, because you could use it to detect timing differences in return to determine if the email you entered is an email in the system :).

See the linked article for full details ;) https://paragonie.com/blog/2016/09/untangling-forget-me-knot-secure-account-recovery-made-simple

The main focus is that you have two factors for authenticating, a selector (like a username, email address or random token) and a verifier that acts like a password (with the same security requirements for verifying). Using only a random value (token) as-is (as the FOSUserBundle is doing) makes that your token is nothing more then a random id that can be guessed, I can't make it more clear than this 👍

I pushed commits regarding the first review comments.
On top of that, I prefixed both form classes names by Password, I find PasswordRequestFormType and PasswordResettingFormType to be more explicit.

Now I will work on having a separate entity, and then on testing everything.

Note that if I were building FOSUserBundle today, I would probably use a separate table to store the reset tokens instead of putting things in the User class.

@sstok the issue of using the email address as the selector is that it allows using a timing attack to identify whether the email exists in the DB, allowing to leak the user base. That's why using a random selector instead is better if you try to protect the system against timing attacks.
Another option is to use the primary key of the user table...

@stof, would you rather go with the random selector or the primary key?
I feel like the id would seem unsafe (for users and developpers to see the easily guessable user id in the url), even if it's mostly the same security level... It is one less field to generate on the other hand.

I feel like having an extra "selector" field is not that bad, so for now I'm going with a selector separate from the ID.
One issue I'm having though: how to hash "token" without hardcoding a reference to the hash algorithm & without adding a dependency to an extra library?
For now, I'm considering getting the "UserPasswordEncoder", it feels a little bit hacky though.

The PR is now good regarding feedback from @bocharsky-bw and @weaverryan !
I added a few tests regarding new features (retry TTL...), test are green on Travis / 7.3 and AppVeyor. I tested the Maker on a fresh Symfony project, I like the generated code, and I believe it will improve many applications :)

Also thank you @stof for your comments and @Pierstoval for your help.
There are still 2 comments open for discussion, which are either personal preference either to be done later I think.

🚢 ❕

😄

@bocharsky-bw and @weaverryan could you please check how I implemented modifications you requested?
I still hope it can get merged soon :)

Do you know when your changes will be available? I would like to use them

Do you know when your changes will be available? I would like to use them

@FBL-dev we had a chat over the feature today at SymfonyCon hack day, I rebased and fixed some small issues, hopefully @bocharsky-bw should review it soon, and I believe we are good for release :)

@rbaarsma you have a point, but right now we don't quite have the infrastructure in MakerBundle to handle that (ie., have 2 different templates, switching according to installed dependency, test that...). I think SF5 is a topic we should treat "globally" in Maker bundle, maybe with some infrastructure refactor.

So I would prefer that PR to be merged, as @bocharsky-bw says, and then I will help with setting up that infrastructure + supporting Mailer. It does not degrade Maker as a whole, Maker won't work on SF5, but make:functional-test does not work too.

Sorry for the slow merge on this. THANK you Romaric for your HUGE work here and also to Victor for helping to review and give feedback in my absence. I'm very excited about this feature :)

Hi all. I just came across this. I want to implement this forgotten password functionality and I see it is merged, but I cannot find any documentation for it, nor is it in the list of make: commands available in my project.

I'm using version 1.21.1 of the maker bundle.

@darraghenright see make:reset-password command, it should be there