Clicking change password doesn't actually work.

[AshTemp]

Bug Report

What is the current behavior?

When clicking change password user is told we are unable to set the password for this email.

What is the expected behavior?

The password resets.

What steps did you take to get this behavior?

1. Open www.operationcode.org.
2. Click login.
3. Click forgot password.
4. Go to email and click link in email to change password.
5. Attempt to change password.

Additional Info

Operating System

Windows 10

Browser

Chrome

Screenshots

[apex-omontgomery]

I just tried to do it and it works for me. For some reason we thought it had to do with the FE sending a PATCH and the BE expecting a POST

I just tried this and it's working in chrome to send a PATCH.

Request URL: https://api.operationcode.org//users/password
Request Method: PATCH
Status Code: 200 
Remote Address: 18.218.241.113:443
Referrer Policy: no-referrer-when-downgrade 

I don't see how this patch could be getting to the right controller since we don't have any PATCH setup for that route.

[AshTemp]

It's still broken for me? New logs are in the channel.

…

On Fri, Aug 3, 2018 at 11:46 PM William Montgomery ***@***.***> wrote: I just tried to do it and it works for me. For some reason we thought it had to do with the FE sending a PATCH and the BE expecting a POST I just tried this and it's working in chrome to send a PATCH. Request URL: https://api.operationcode.org//users/password Request Method: PATCH Status Code: 200 Remote Address: 18.218.241.113:443 Referrer Policy: no-referrer-when-downgrade``` I don't see how this patch could be getting to the right controller since we don't have any PATCH setup for that route. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#379 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AS35GAHSMzmUsNGMsmBCLaM6F8ycEQ0cks5uNRkAgaJpZM4VuEZG> .

[AshTemp]

After further testing with Will, the user can click the password recent link from a previous email and it will work. Initially clicking the reset link on the most recent email will result in failure. However, after the user is successfully able to change the password with a previous link the most recent link will also work.

[robbkidd]

Which revision of the backend is running on the system that exhibits this behavior? (I am new here and still ignorant of the Ways of Deployment for the production system.)

Regarding PATCH/POST/PUT

While tinkering with latest master (97d360f)—I note that the devise routes handle both PATCH and PUT to route to devise/passwords#update:

> be rake routes | grep password | grep update
admin_user_password PATCH  /admin/password(.:format)     active_admin/devise/passwords#update
                    PUT    /admin/password(.:format)     active_admin/devise/passwords#update
      user_password PATCH  /users/password(.:format)     devise/passwords#update
                    PUT    /users/password(.:format)     devise/passwords#update

WEIRD DEV ENV NOTE

I had to update Devise in my local development environment, though. rake routes was erroring on parsing Devise 4.2.1's app/controllers/devise/sessions_controller.rb:

D, [2018-08-21T10:58:06.640569 #73535] DEBUG -- : /Users/rkidd/code/operationcode/operationcode_backend/.direnv/ruby/gems/devise-4.2.1/app/controllers/devise/sessions_controller.rb:5:
syntax error, unexpected '{', expecting keyword_end
...ion only: [:create, :destroy] { request.env["devise.skip_tim...
...                              ^
/Users/rkidd/code/operationcode/operationcode_backend/.direnv/ruby/gems/devise-4.2.1/app/controllers/devise/sessions_controller.rb:5:
syntax error, unexpected '}', expecting keyword_end
..."devise.skip_timeout"] = true }
...                              ^ excluded from capture: DSN not set
rake aborted!

I updated my local dev version of devise to 4.5.0 to get a change made to Devise to appease Ruby 2.5.0. I'm not sure why my local Ruby 2.3.3 was barking about that and I'm not sure what y'all are seeing in production.

[robbkidd]

A little more information: the first password reset attempt seems to actually succeed in changing the user's password despite the display of the error.

[jjhampton]

Originally I thought the front-end might have been mishandling the response and causing some type of redirect loop, but I don't think so anymore.

I think the usage of PATCH in the back-end has to be related here - I can't reproduce this consistently every time, but in local dev, after submitting the password reset form, the back-end resets the password, responds with a 302 message, but then also responds with a 404 error, which causes the front-end Promise to reject and enter the catch clause on setPassword.js, line 48 (which displays the error message).

I'm seeing ActionController::RoutingError (No route matches [PATCH] "/") in the back-end terminal after I reset the password, and in the front-end DevTools Network tab, the browser is receiving the matching error response:

error: "Not Found"
exception: "#<ActionController::RoutingError: No route matches [PATCH] "/">"
status: 404

[jjhampton]

After Googling, I think this could have something to do with the Devise gem. Seeing lots of reports that Devise is redirecting to / after users reset passwords: https://stackoverflow.com/questions/38343622/devises-sign-in-after-reset-password-doesnt-seem-to-do-what-its-name-suggests

Since this isn't a server-rendered Rails app (just a Rails API), it seems to me that the back-end should not attempt any redirect at all.

[jjhampton]

Locally, when I change the front-end API call to /users/password from a PATCH to a POST, I no longer get the 404 error response on the back-end anymore (in the Devise redirect after resetting password), and the FE Promise is resolving successfully, with no error message displayed, and the user is successfully redirected in the React app to the /login route, as intended.

I also tried changing to a PUT, and got the same redirect error in Rails: ActionController::RoutingError (No route matches [PUT] "/"):.

For some reason, only works as a POST. I think you were on to something @wimo7083 .

I also tried changing config.sign_in_after_reset_password = false in devise.rb, but that in turn only caused redirect errors of a similar type: ActionController::RoutingError (No route matches [PATCH] "/users/sign_in").

I'm also wondering if we're even hitting the right Devise endpoint - we're using the /password endpoint, when the Devise docs are saying to use /registrations: https://github.com/plataformatec/devise/wiki/How-To:-Allow-users-to-edit-their-password#solution-2

[robbkidd]

Discovered this morning that a second send-me-a-reset-email is not necessary. Only need to click the "Set Password" button again after seeing the error to get the success and redirect to /login.

[desnudopenguino]

Beyond being an annoyance, this could be a security issue if someone's email is compromised and an old password reset email is available. My initial thoughts are to correct whatever issue is preventing the newest reset link from working, setting an expiration on the link so that after x time, it won't work anymore, and setting older reset links as invalid once a new one is requested. I don't see this being too complex of a fix, but I haven't actually looked at the code yet, but I'd love to tackle the problem.

[robbkidd]

[…] correct whatever issue is preventing the newest reset link from working, setting an expiration on the link so that after x time, it won't work anymore

I agree with these.

setting older reset links as invalid once a new one is requested

This seems good, but I wonder about the consequences.

[AshTemp]

So this works for now, but we still need to eventually fix the underlying issue of the link not working the first time someone clicks it.

…

On Mon, Oct 1, 2018 at 10:14 AM Robb Kidd ***@***.***> wrote: […] correct whatever issue is preventing the newest reset link from working, setting an expiration on the link so that after x time, it won't work anymore I agree with these. setting older reset links as invalid once a new one is requested This seems good, but I wonder about the consequences. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#379 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AS35GI-VWlEYrhWn-MQOVEkw65dnWHOUks5ugiM6gaJpZM4VuEZG> .

[apex-omontgomery]

Looks like we try to be configured to 6 hours.

operationcode_backend/config/initializers/devise.rb

Line 206 in 5df843a

config.reset_password_within = 6.hours

https://secure-web.cisco.com/REALLY_LONG_UNIQUE_STRING_UNSURE_IFAUTH/https%3A%2F%2Foperationcode.org%2Freset_password%3Freset_password_token%$Some_token

For acceptance criteria:

1. Create documentation to understand password reset process.
2. Prevent errors from being sent to FE if the reset process is valid .
3. Expire links after $TIME_PERIOD.
4. Prevent multiple current-active requests for a single user.

Do these seem reasonable?