admin forced password reset #1834
Conversation
| post 'register' => 'users#create' | ||
| get 'reset' => 'users#reset' | ||
| post 'reset' => 'users#reset' | ||
| post 'reset/key/:key' => 'users#reset' | ||
| get 'reset/key/:key' => 'users#reset' |
There was a problem hiding this comment.
I had to change this verb and the above to get in order to get them to work on my local machine. Since they aren't really the parts I had to work on I assume they are correct? And I will revert them back after I have finished.
There was a problem hiding this comment.
I think you fixed a latent bug related to the Rails 4 upgrade-- i've spent much of the day chasing a few of these which we missed when doing the routes.rb refactoring. Thank you!
app/controllers/admin_controller.rb
Outdated
| PasswordResetMailer.reset_notify(user, key) unless user.nil? # respond the same to both successes and failures; security | ||
| end | ||
| flash[:notice] = I18n.t('users_controller.password_reset_email') | ||
| redirect_to "/login" |
There was a problem hiding this comment.
I think the page should not redirect to the log in page since it's the admin that's starting the password reset
There was a problem hiding this comment.
Hmm, can you detect that it's not a typical user and redirect to... the profile page?
config/routes.rb
Outdated
| @@ -222,6 +222,7 @@ | |||
| put 'moderate/spam/:id' => 'admin#mark_spam' | |||
| put 'moderate/publish/:id' => 'admin#publish' | |||
| put 'admin/promote/moderator/:id' => 'admin#promote_moderator' | |||
| get 'resetuserpassword/:id' => 'admin#reset_user_password' | |||
There was a problem hiding this comment.
I think this name might not be that great and another path name would be good.
There was a problem hiding this comment.
maybe... /user/reset/password ?
Generated by 🚫 Danger |
|
Very cool and apologies for the routes.rb conflicts -- as i mentioned i've been fixing lots of things in that file. Please try to preserve your changes and rebase them into the latest version. I'm happy to help with this of course! Many thanks! |
|
This is looking good -- actually we don't want moderators to be able to force reset -- sorry if that was wrong in the request. Is this otherwise good to go? Awesome! |
|
oh sorry that last commit might be misleading. I meant that an Admin can reset a moderator's password if they need to. Or do we not want that to be a possibility? I think for the most part it is finished. I'm just thinking about changing the flash message that pops up when the Admin resets the password because right now it's the same message as if a user reset it themselves (something like "You should receive an email.....") and maybe "@user's email has been reset. They should receive instructions...." |
|
@jywarren I just changed the message so I think the whole PR is good to go! Let me know if there is anything you need me to change |
Kevin-Kawai
changed the title
app/controllers/admin_controller.rb
Outdated
| @@ -43,6 +43,22 @@ def demote_basic | |||
| redirect_to '/profile/' + @user.username + '?_=' + Time.now.to_i.to_s | |||
| end | |||
|
|
|||
| def reset_user_password | |||
| if current_user && (current_user.role == 'moderator' || current_user.role == 'admin') | |||
There was a problem hiding this comment.
Hi, doesn't this mean that moderators would be able to reset passwords? If we could just change this to admins only, that'd be great. Thanks!!!
There was a problem hiding this comment.
I apologize, you are correct. This would allow moderator's to reset a user's password. I will fix this after work and push the changes.
|
@jywarren Sorry about that last bit of confusion. I fixed the controller so the moderator doesn't have the ability to reset passwords. Everything else should be good to go! Let me know if there's anything else. |
|
Super, thanks so much!!! 🎉 |
* added admin req pass change action and button on view * renamed admin force reset route and changed redirect page after reset * Made it so admin cannot repeatedly press reset button * added test and changed route name to follow convention * made it so a moderator acc can be force reset as well * changed alert message when admin resets password * fixed merge conflict mistake * removed moderator ability to reset from controller
Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!
This is the branch for the issue #232. It allows an admin to start the password reset procedure for a user through their profile page.
rake test:allPlease be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays. Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.
Thanks!