From d2e31146c669433a4715fc6cc7e13c01a3e38ed6 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Thu, 20 Oct 2016 15:09:51 -0400 Subject: [PATCH] Fix bugs with password resets (#330) - Permissions error with admin forced password reset - Redirecting on successful admin forced password reset - URL fix for forgot password reset - Use JWT key for password reset request --- .../ResetPasswordPage/ResetPasswordPage.jsx | 4 +- frontend/redux/nodes/auth/actions.js | 40 +++++++++++++++++++ frontend/redux/nodes/auth/reducer.js | 16 ++++++++ server/contexts/viewer/viewer.go | 5 ++- server/kolide/emails.go | 4 +- server/service/service_users.go | 8 ++-- 6 files changed, 68 insertions(+), 9 deletions(-) diff --git a/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx b/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx index bea7e32739..ea883de864 100644 --- a/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx +++ b/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx @@ -7,7 +7,7 @@ import debounce from '../../utilities/debounce'; import { resetPassword } from '../../redux/nodes/components/ResetPasswordPage/actions'; import ResetPasswordForm from '../../components/forms/ResetPasswordForm'; import StackedWhiteBoxes from '../../components/StackedWhiteBoxes'; -import userActions from '../../redux/nodes/entities/users/actions'; +import { updateUser } from '../../redux/nodes/auth/actions'; export class ResetPasswordPage extends Component { static propTypes = { @@ -53,7 +53,7 @@ export class ResetPasswordPage extends Component { const { new_password: password } = formData; const passwordUpdateParams = { password }; - return dispatch(userActions.update(user, passwordUpdateParams)) + return dispatch(updateUser(user, passwordUpdateParams)) .then(() => { return dispatch(push('/')); }); } diff --git a/frontend/redux/nodes/auth/actions.js b/frontend/redux/nodes/auth/actions.js index 2b1047aa69..efb65188f7 100644 --- a/frontend/redux/nodes/auth/actions.js +++ b/frontend/redux/nodes/auth/actions.js @@ -1,11 +1,15 @@ import md5 from 'js-md5'; import Kolide from '../../../kolide'; +import userActions from '../../../redux/nodes/entities/users/actions'; export const CLEAR_AUTH_ERRORS = 'CLEAR_AUTH_ERRORS'; export const LOGIN_REQUEST = 'LOGIN_REQUEST'; export const LOGIN_SUCCESS = 'LOGIN_SUCCESS'; export const LOGIN_FAILURE = 'LOGIN_FAILURE'; +export const UPDATE_USER_REQUEST = 'UPDATE_USER_REQUEST'; +export const UPDATE_USER_SUCCESS = 'UPDATE_USER_SUCCESS'; +export const UPDATE_USER_FAILURE = 'UPDATE_USER_FAILURE'; export const LOGOUT_REQUEST = 'LOGOUT_REQUEST'; export const LOGOUT_SUCCESS = 'LOGOUT_SUCCESS'; export const LOGOUT_FAILURE = 'LOGOUT_FAILURE'; @@ -73,6 +77,42 @@ export const loginUser = (formData) => { }; }; +export const updateUserRequest = { type: UPDATE_USER_REQUEST }; +export const updateUserSuccess = (user) => { + return { + type: UPDATE_USER_SUCCESS, + payload: { + user, + }, + }; +}; +export const updateUserFailure = (error) => { + return { + type: UPDATE_USER_FAILURE, + payload: { + error, + }, + }; +}; +export const updateUser = (targetUser, formData) => { + return (dispatch) => { + return new Promise((resolve, reject) => { + dispatch(updateUserRequest); + return dispatch(userActions.update(targetUser, formData)) + .then((response) => { + const { user } = response; + dispatch(updateUserSuccess(user)); + return resolve(user); + }) + .catch((response) => { + const { error } = response; + dispatch(updateUserFailure(error)); + return reject(error); + }); + }); + }; +}; + export const logoutFailure = (error) => { return { type: LOGOUT_FAILURE, diff --git a/frontend/redux/nodes/auth/reducer.js b/frontend/redux/nodes/auth/reducer.js index fed4d760a4..a3b33524e1 100644 --- a/frontend/redux/nodes/auth/reducer.js +++ b/frontend/redux/nodes/auth/reducer.js @@ -3,6 +3,9 @@ import { LOGIN_FAILURE, LOGIN_REQUEST, LOGIN_SUCCESS, + UPDATE_USER_FAILURE, + UPDATE_USER_REQUEST, + UPDATE_USER_SUCCESS, LOGOUT_FAILURE, LOGOUT_REQUEST, LOGOUT_SUCCESS, @@ -23,6 +26,7 @@ const reducer = (state = initialState, action) => { }; case LOGIN_REQUEST: case LOGOUT_REQUEST: + case UPDATE_USER_REQUEST: return { ...state, loading: true, @@ -39,6 +43,18 @@ const reducer = (state = initialState, action) => { loading: false, error: action.payload.error, }; + case UPDATE_USER_SUCCESS: + return { + ...state, + loading: false, + user: action.payload.user, + }; + case UPDATE_USER_FAILURE: + return { + ...state, + loading: false, + error: action.payload.error, + }; case LOGOUT_SUCCESS: return { ...state, diff --git a/server/contexts/viewer/viewer.go b/server/contexts/viewer/viewer.go index 418bb812d2..12bc200fcc 100644 --- a/server/contexts/viewer/viewer.go +++ b/server/contexts/viewer/viewer.go @@ -105,7 +105,10 @@ func (v Viewer) CanPerformReadActionOnUser(uid uint) bool { // ability to perform write actions on the given user func (v Viewer) CanPerformWriteActionOnUser(uid uint) bool { if v.User != nil { - return v.CanPerformActions() && (v.IsUserID(uid) || v.IsAdmin()) + // By not requiring v.CanPerformActions() here, we allow the + // user to update their password when they are in the forced + // password reset state. + return v.IsUserID(uid) || v.IsAdmin() } return false } diff --git a/server/kolide/emails.go b/server/kolide/emails.go index c8990b61d5..b1b567484b 100644 --- a/server/kolide/emails.go +++ b/server/kolide/emails.go @@ -47,9 +47,9 @@ type PasswordResetRequest struct { } const passwordResetTemplate = ` -Your requested a password reset, +You requested a password reset, Follow the link below to reset your password: -http://localhost:8080/reset_password?token={{.Token}} +http://localhost:8080/login/reset?token={{.Token}} ` func (r PasswordResetRequest) Message() ([]byte, error) { diff --git a/server/service/service_users.go b/server/service/service_users.go index 192c5706c0..61586de363 100644 --- a/server/service/service_users.go +++ b/server/service/service_users.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "time" + jwt "github.com/dgrijalva/jwt-go" "github.com/kolide/kolide-ose/server/contexts/viewer" "github.com/kolide/kolide-ose/server/kolide" "golang.org/x/net/context" @@ -146,8 +147,8 @@ func (svc service) ResetPassword(ctx context.Context, token, password string) er } func (svc service) RequestPasswordReset(ctx context.Context, email string) error { - // the password reset is different depending on wether performed by an admin - // or a user + // the password reset is different depending on whether performed by an + // admin or a user // if an admin requests a password reset, then no token is // generated, instead the AdminForcedPasswordReset flag is set user, err := svc.ds.UserByEmail(email) @@ -168,8 +169,7 @@ func (svc service) RequestPasswordReset(ctx context.Context, email string) error } } - // TODO: change this to jwt key - token, err := generateRandomText(svc.config.App.TokenKeySize) + token, err := jwt.New(jwt.SigningMethodHS256).SignedString([]byte(svc.config.App.TokenKey)) if err != nil { return err }