Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a password service #83

Closed
ekryski opened this Issue Feb 26, 2016 · 24 comments

Comments

Projects
None yet
@ekryski
Copy link
Member

ekryski commented Feb 26, 2016

It's a common need to have password reset when using local auth. As talked about in feathersjs/feathers#237 it would be great to support password reset out of the box.

We've talked a lot about this and I think it makes sense to be part of this repo simply because it's easier for us to configure and automatically configure it with sane defaults if local authentication is enabled. We can always move it out later.

It would use a communication plugin (ie. Twilio or Sendgrid) to send reset password emails or texts.

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Feb 27, 2016

Sounds good to me. Let's discuss workflow.

  • Have a password-resets collection in the db that will store the userId and change password secret/code. I would prefer it to be separate from the users collection.
  • passwordService.create() will require an email / username / phone number to create a new code and associate it with the user.
  • Create an after-create hook for each type of communication service.
  • Have some kind of target attribute that can be used to specify which communication service is used to send the password reset link/code and we want our apps to support multiple options. For example type:'twilio' or type:'sendgrid' / type='postmark' Then the after-hooks can read this attribute to know when to act, or if there's no target specified, they all act.
  • Possible user interactions:
    • After user types in their username/email they see a page where they can enter their code.
    • User clicks a link without a code: They see a page where they can enter a code.
    • User clicks a link that includes a code: They see a page with password/confirm inputs.

Do we want to include a basic version of the needed pages?

  • One for entering email or phone number to start the process.
  • One that says to check their sms/email to check for a code / link and they can either type in the code or click the link.
  • One that gives them a confirmation with a gold star for all their success in resetting their password.

@ekryski ekryski modified the milestone: 1.0 release Feb 28, 2016

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Mar 8, 2016

With the above, I'm not intending to lay that out as doctrine and anybody against it is an apostate. :) I was more hoping to spark a conversation that will get us to a decided-upon API quicker.

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Mar 8, 2016

And my long comments can probably be ignored if we can handle this using #96.

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Mar 8, 2016

@marshallswain yup no worries. Some great ideas in there. Sorry I haven't responded I've been busy with other stuff and wanted to spike it out before responding. I see #96 playing into it but not necessarily closing this off.

@kristianmandrup

This comment has been minimized.

Copy link

kristianmandrup commented Mar 20, 2016

What is the status on this?

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Mar 20, 2016

@kristianmandrup currently in progress. I'm hoping to wrap up a PR today.

@kristianmandrup

This comment has been minimized.

Copy link

kristianmandrup commented Mar 20, 2016

@ekryski Awesome!!!

@beeplin

This comment has been minimized.

Copy link

beeplin commented Mar 21, 2016

@ekryski keen to see the PR ~

@Immortalin

This comment has been minimized.

Copy link

Immortalin commented Mar 26, 2016

@ekryski can a normal(one that doesn't depend on third party services) email target be added e.g. nodemailer or emailjs etc.?

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Mar 26, 2016

@Immortalin yup for sure. The notification engine you want to use will be entirely up to you. It's just a trigger with a hook.

@ekryski ekryski modified the milestones: 1.0, 0.8 Mar 26, 2016

@ekryski ekryski added the Feature label Mar 26, 2016

@nicksrandall

This comment has been minimized.

Copy link

nicksrandall commented Apr 14, 2016

I would prefer a solution that doesn't require an extra database table. I would create a token that is hmac signed and cryptographically stores userId, password hash, and expiration date (24 hrs?). We could then send the user an email with a "reset" link that includes the token in the url. When they click the link we can verify the token by checking the expiration and that they haven't already reset the password (the password hash would no longer match). If the token is valid then we display a reset password form that the can be used to create a new password. We would need to make sure that when the form is submitted to include the token as well as the new password. Finally, we could verify the token one more time and update the user db with the new password. I hope that makes sense. FWIW, I'm fairly certain that this is how djengo handles password resets.

@rickmed

This comment has been minimized.

Copy link

rickmed commented Jun 5, 2016

has this been implemented somewhere else?

@rohitketkar87

This comment has been minimized.

Copy link

rohitketkar87 commented Jul 14, 2016

hi...
Is password service available in feathers?

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Jul 14, 2016

As far as I know, this hasn't been implemented, yet.

@beeplin

This comment has been minimized.

Copy link

beeplin commented Oct 17, 2016

feathers-service-verify-reset seems quite promising. and has additional features like verification, beside resetting password.

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Oct 17, 2016

Definitely! We're currently wrapping a major overhaul of auth. Once things are solidified we'll be looking to get @eddyystop's repo (or one very similar to it) as a core repo.

@ekryski ekryski modified the milestones: 0.8, 2.0 Nov 21, 2016

@paulophp

This comment has been minimized.

Copy link

paulophp commented Nov 23, 2016

Hey guys! Sorry to bother but, could someone guide me how I could patch passwords of users in my app? I am already using my feathersjs app in production, and it happened that a user forgot his password, if I could at least overwrite it form him, it would be awesome!

@mandricore

This comment has been minimized.

Copy link

mandricore commented Nov 23, 2016

Good to hear!

@eddyystop

This comment has been minimized.

Copy link
Member

eddyystop commented Nov 23, 2016

@paulophp You can use eddyystop/feathers-services-verify-rest for that. Its being pulled into core in a few days as feathersjs/feathers-authentication-management, having undergone major refactoring and some nice extensions.

You can 'manually' patch his password for now using logic in https://github.com/feathersjs/feathers-authentication-management/blob/master/src/helpers.js#L17

@paulophp

This comment has been minimized.

Copy link

paulophp commented Nov 23, 2016

@eddyystop thank you very much for the reply!
I just added auth.hashPassword() at before patch hook, then I patched the user sending the new password and it worked fine!! :)

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Dec 22, 2016

https://github.com/feathersjs/feathers-authentication-management is published and live. You can view the current docs in the repo for now. We'll be updating them for clarity and consistency over the next couple days as we prep the docs for the new Auk release but I think we can close this out now.

@ekryski ekryski closed this Dec 22, 2016

@dericcain

This comment has been minimized.

Copy link

dericcain commented Apr 21, 2017

@ekryski Hi! It looks like the link is not longer valid. Are the official docs for this the one's in the repo?

@eddyystop

This comment has been minimized.

Copy link
Member

eddyystop commented Apr 21, 2017

@dericcain The README is the place to go for now.

@ekryski

This comment has been minimized.

Copy link
Member Author

ekryski commented Apr 26, 2017

Updated my comment. Thanks @dericcain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.