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

password reset #366

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

password reset #366

wants to merge 3 commits into from

Conversation

dmtroyer
Copy link
Contributor

@dmtroyer dmtroyer commented Dec 5, 2015

Start on password reset.

@dmtroyer dmtroyer self-assigned this Dec 5, 2015
@MatthewVita
Copy link
Contributor

issue is here: #367

@dmtroyer dmtroyer mentioned this pull request Dec 10, 2015
@MatthewVita
Copy link
Contributor

@dmtroyer okay so I started on this and immediately ran into something that I wanted to discuss with you before moving forward.

Basically implementing the "Forgot Password" stuff is simple by calling $auth.requestPasswordReset(passwordResetPayload). However, the email that goes to the user contains a customizable link for the user to reset their password... for example:

<a href="http://{host}/dashboard/users/password/edit?reset_password_token=YcKXiKpWyeUsrh7EuPL1">Change my password</a>

Assume that we change that url to http://{host}/#/users/password/reset?reset_password_token=YcKXiKpWyeUsrh7EuPL1 and on the Angular side, bring in the value of the token via the $routeParams...

Now see the following doc: https://github.com/lynndylanhurley/ng-token-auth#authupdatepassword (the function we'd most likely use to handle this). I have 2 problems with it:

  1. It only applies to authenticated users (clearly someone who forgot their password would not be authenticated... doesn't seem like there's any other method that we can use OOTB for this use case either)
  2. There is no parameter that passes along the reset_password_token.

... am I missing something or will we have to call a devise endpoint manually with this information?

@dmtroyer
Copy link
Contributor Author

@MatthewVita I gotcha...

The link in the email shouldn't be to the page where the user will edit their password, but rather to the api/v1/auth/password/edit route, with the reset_password_token and redirect_url params set. Once the user clicks the link and makes that GET request, it authorizes them to edit their password, authenticates or quasi-authenticates them and forwards them to redirect_url, which should be where the user edits their password using the $auth.updatePassword method without needing a current password.

Granted, I didn't test any of this so take it with a grain of salt :-), but that should get you down the right track.

The docs on the devise-token-auth side can be quasi helpful.

@MatthewVita
Copy link
Contributor

@dmtroyer thanks for the rundown... things are a tad more clear now. I read over the devise-token-auth code a bit and it seems like the two major drivers of this are:

I'm not sure we'll get the authentication/quasi-authentication we're looking for OOTB as you described it, but playing around/reading more into the docs/code will be the only way to find out.

Will update with my findings.

@MatthewVita
Copy link
Contributor

Method of note: https://github.com/lynndylanhurley/devise_token_auth/blob/57cb7f9e61935e91045dfd2574c5bceae986b7c6/app/models/devise_token_auth/concerns/user.rb#L92

View of note:
app/views/devise/passwords/edit.html.erb (need to clear this out since we're going to rely on the redirect alone)

@MatthewVita
Copy link
Contributor

Trying out the example app (http://ng-token-auth-demo.herokuapp.com/) to see the strategy there.

The reset password link in the email is:

http://devise-token-auth-demo.herokuapp.com/auth/password/edit with the following query params:

1

after clicking it, it is:

2

...here's the request when PUTing the new password

asdfasdfasdfasdfasdf

@MatthewVita
Copy link
Contributor

Okay, so I'm on the right track with calling out a passwordResetSuccessUrl on the frontend because it is

[...] the URL to which the API should redirect after users visit the links contained in password-reset emails.

...however, even when providing this url, the redirect_url never seems to be included in the email link. For example "www.google.com" is correctly picked up in the headers but is not added into the config query string:

...
provider: email
redirect-url: www.google.com
client-config: default

<p>Hello [email protected]!</p>

<p>Someone has requested a link to change your password. You can do this through the link below.</p>

<p><a href="http://localhost:3000/dashboard/users/password/edit?reset_password_token=wkrTqovVJ2frFKX3934s">Change my password</a></p>

Therefore, I am stuck at the part where the "user is redirect to client..." of the following figure:

graphic

@MatthewVita
Copy link
Contributor

As seen below, using the live sample app, the redirect_url is in place and is redirecting correctly. However, the "one time auth" isn't working; one cannot reset the password :(

(right click to view full size gif)

z

As seen in our app, the redirect_url isn't being placed in the url correctly. User can't reset their password on whatever custom URL we want them to be directed to (in our case it will be something like http://localhost:3000/#/users/password/reset?allow_reset=yes, but I'm just using http://www.google.com for now)

(right click to view full size gif)

a

...As such, I'm blocked on this feature. I'm going to submit an issue to the folks at ng-token-auth and will report back.

@timothyfcook, @dmtroyer

@MatthewVita
Copy link
Contributor

@dmtroyer
Copy link
Contributor Author

@MatthewVita I took a quick peek and added the devise_token_auth mailer views which include the redirect_url param and things seem to be happier. I also had roll back to the default passwordResetSuccessUrl as URI didn't seem to like it without a prepending http://. Lastly, I had to comment out some of the old devise configuration on /dashboard as it was conflicting with the edit_password_url helper in the mailer view.

That should help get you on the right track.

@MatthewVita
Copy link
Contributor

@dmtroyer you're awesome. I missed this because I was focused on reviewing the code over at the ng-token and gem repos and didn't see that our view was overriding the defaults >.<.

However, this is still problematic (see the "UPDATE" I added to the issue) because the route gets messed up (lynndylanhurley/ng-token-auth#249). And even with that, the other issue I opened still needs address (lynndylanhurley/ng-token-auth#248)

This feature is blocked for now. I'm going to "groom" our MVP backlog and probably start on the dashboard > location area :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants