Conversation
…url doesnt show password token
|
Can you add a description to the pull request? (template format, for example) |
config/routes.rb
Outdated
| devise_scope :user do | ||
| get '/users/password/new' => 'users/reset_passwords#new', as: :new_user_password | ||
| get '/users/password/edit' => 'users/reset_passwords#edit', as: :edit_user_password | ||
| get '/users/password/change_password' => 'users/reset_passwords#change_password', as: :change_user_password |
|
Sorta curious about the introduction of a feature flag. Main concern from me is that we're skipping a lot of feature test coverage until it becomes the default, which I assume we might plan to do only after it's already gone live in production? |
This was to protect against any weirdness taht could show mid deploy. If users are trying to confirm during the middle of the deploy i was thinking they would be stuck in a weird flow where their token is stored in the session but directed to the old instance where we are still relying on the query params to populate it. i can edit the feature tests so that we are testing this new functionality outright as well as current functionality though. good callout. |
|
Gotcha, maybe we can make the default |
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
LGTM 👍
Will there be a quick follow-on pull request to remove the feature flag we added here for the initial deploy?
| ) | ||
|
|
||
| if result.success? | ||
| session.delete(:reset_password_token) if session[:reset_password_token] |
There was a problem hiding this comment.
In the follow-up pull request which removes the feature flag, we could probably remove the conditional here. I expect it's a safe operation regardless if the value exists or not, and we should expect that it would in most (all?) cases.
There was a problem hiding this comment.
Yup here is the followup PR to this #8286
4 participants
🎫 Ticket
LG-8534: Move reset token out of url, store in session
-->
🛠 Summary of changes
This will allow users to redirect to a url without the password as well as store the password token in the session.
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
Users are properly redirected to change_password url with the token not visible anymore
Screenshot
Below is screenshot of the change password page with the new url.
