From 60428e01c4920984bb64a572559e2241e8e6e86f Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Mon, 9 Jan 2017 20:42:50 -0800 Subject: [PATCH] Fix required password reset flow (#833) Permissions errors were preventing users from completing this flow - Add separate endpoint for performing required password reset - Rewrite frontend reset to use this endpoint Fixes #792 --- frontend/kolide/endpoints.js | 1 + frontend/kolide/index.js | 13 ++ .../ResetPasswordPage/ResetPasswordPage.jsx | 10 +- frontend/redux/nodes/auth/actions.js | 37 ++++++ frontend/redux/nodes/auth/actions.tests.js | 113 ++++++++++++++++++ frontend/redux/nodes/auth/reducer.js | 22 ++++ frontend/redux/nodes/auth/reducer.tests.js | 51 ++++++++ .../redux/nodes/entities/users/actions.js | 1 + .../nodes/entities/users/reducers.tests.js | 72 +++++------ server/kolide/users.go | 5 + server/service/endpoint_users.go | 27 +++++ server/service/handler.go | 19 ++- server/service/logging_users.go | 22 ++++ server/service/service_users.go | 28 +++++ server/service/service_users_test.go | 59 +++++++++ server/service/transport_users.go | 8 ++ 16 files changed, 442 insertions(+), 46 deletions(-) create mode 100644 frontend/redux/nodes/auth/actions.tests.js diff --git a/frontend/kolide/endpoints.js b/frontend/kolide/endpoints.js index 0f2c075ae5..5349022651 100644 --- a/frontend/kolide/endpoints.js +++ b/frontend/kolide/endpoints.js @@ -11,6 +11,7 @@ export default { LOGOUT: '/v1/kolide/logout', ME: '/v1/kolide/me', PACKS: '/v1/kolide/packs', + PERFORM_REQUIRED_PASSWORD_RESET: '/v1/kolide/perform_required_password_reset', QUERIES: '/v1/kolide/queries', RESET_PASSWORD: '/v1/kolide/reset_password', RUN_QUERY: '/v1/kolide/queries/run', diff --git a/frontend/kolide/index.js b/frontend/kolide/index.js index e36c9a088c..74405fde8c 100644 --- a/frontend/kolide/index.js +++ b/frontend/kolide/index.js @@ -395,6 +395,19 @@ class Kolide extends Base { return helpers.addGravatarUrlToResource(updatedUser); }); } + + // Perform a password reset for the currently logged in user that has had a + // reset required + performRequiredPasswordReset = ({ password }) => { + const { PERFORM_REQUIRED_PASSWORD_RESET } = endpoints; + const performRequiredPasswordResetEndpoint = this.baseURL + PERFORM_REQUIRED_PASSWORD_RESET; + + return this.authenticatedPost(performRequiredPasswordResetEndpoint, JSON.stringify({ new_password: password })) + .then((response) => { + const { user: updatedUser } = response; + return helpers.addGravatarUrlToResource(updatedUser); + }); + } } export default new Kolide(); diff --git a/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx b/frontend/pages/ResetPasswordPage/ResetPasswordPage.jsx index 9f6aca717b..ad721f5933 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 { updateUser } from '../../redux/nodes/auth/actions'; +import { performRequiredPasswordReset } from '../../redux/nodes/auth/actions'; import userInterface from '../../interfaces/user'; export class ResetPasswordPage extends Component { @@ -35,7 +35,7 @@ export class ResetPasswordPage extends Component { const { dispatch, token, user } = this.props; if (user) { - return this.updateUser(formData); + return this.loggedInUser(formData); } const resetPasswordData = { @@ -55,12 +55,12 @@ export class ResetPasswordPage extends Component { return dispatch(push(location)); } - updateUser = (formData) => { - const { dispatch, user } = this.props; + loggedInUser = (formData) => { + const { dispatch } = this.props; const { new_password: password } = formData; const passwordUpdateParams = { password }; - return dispatch(updateUser(user, passwordUpdateParams)) + return dispatch(performRequiredPasswordReset(passwordUpdateParams)) .then(() => { return dispatch(push('/')); }); } diff --git a/frontend/redux/nodes/auth/actions.js b/frontend/redux/nodes/auth/actions.js index 7accb5e8ab..40b379dd92 100644 --- a/frontend/redux/nodes/auth/actions.js +++ b/frontend/redux/nodes/auth/actions.js @@ -12,6 +12,10 @@ 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'; +// Actions for a user resetting their password after a required reset +export const PERFORM_REQUIRED_PASSWORD_RESET_REQUEST = 'PERFORM_REQUIRED_PASSWORD_RESET_REQUEST'; +export const PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS = 'PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS'; +export const PERFORM_REQUIRED_PASSWORD_RESET_FAILURE = 'PERFORM_REQUIRED_PASSWORD_RESET_FAILURE'; export const clearAuthErrors = { type: CLEAR_AUTH_ERRORS }; export const loginRequest = { type: LOGIN_REQUEST }; @@ -143,3 +147,36 @@ export const logoutUser = () => { }); }; }; + +export const performRequiredPasswordResetRequest = { type: PERFORM_REQUIRED_PASSWORD_RESET_REQUEST }; + +export const performRequiredPasswordResetSuccess = (user) => { + return { + type: PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS, + payload: { user }, + }; +}; + +export const performRequiredPasswordResetFailure = (errors) => { + return { + type: PERFORM_REQUIRED_PASSWORD_RESET_FAILURE, + payload: { errors }, + }; +}; + +export const performRequiredPasswordReset = (resetParams) => { + return (dispatch) => { + dispatch(performRequiredPasswordResetRequest); + + return Kolide.performRequiredPasswordReset(resetParams) + .then((updatedUser) => { + dispatch(performRequiredPasswordResetSuccess(updatedUser)); + }) + .catch((response) => { + const errorsObject = formatErrorResponse(response); + dispatch(performRequiredPasswordResetFailure(errorsObject)); + + throw response; + }); + }; +}; diff --git a/frontend/redux/nodes/auth/actions.tests.js b/frontend/redux/nodes/auth/actions.tests.js new file mode 100644 index 0000000000..a762b795af --- /dev/null +++ b/frontend/redux/nodes/auth/actions.tests.js @@ -0,0 +1,113 @@ +import expect, { restoreSpies, spyOn } from 'expect'; + +import * as Kolide from 'kolide'; + +import { reduxMockStore } from 'test/helpers'; + +import { + performRequiredPasswordReset, + PERFORM_REQUIRED_PASSWORD_RESET_REQUEST, + PERFORM_REQUIRED_PASSWORD_RESET_FAILURE, + PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS, +} from './actions'; + +const store = { entities: { invites: {}, users: {} } }; +const user = { id: 1, email: 'zwass@kolide.co', force_password_reset: false }; + +describe('Auth - actions', () => { + describe('dispatching the perform required password reset action', () => { + describe('successful request', () => { + beforeEach(() => { + spyOn(Kolide.default, 'performRequiredPasswordReset').andCall(() => { + return Promise.resolve({ ...user, force_password_reset: false }); + }); + }); + + afterEach(restoreSpies); + + const resetParams = { password: 'foobar' }; + + it('calls the resetFunc', () => { + const mockStore = reduxMockStore(store); + + return mockStore.dispatch(performRequiredPasswordReset(resetParams)) + .then(() => { + expect(Kolide.default.performRequiredPasswordReset).toHaveBeenCalledWith(resetParams); + }); + }); + + it('dispatches the correct actions', () => { + const mockStore = reduxMockStore(store); + + const expectedActions = [ + { type: PERFORM_REQUIRED_PASSWORD_RESET_REQUEST }, + { + type: PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS, + payload: { user: { ...user, force_password_reset: false } }, + }, + ]; + + return mockStore.dispatch(performRequiredPasswordReset(resetParams)) + .then(() => { + expect(mockStore.getActions()).toEqual(expectedActions); + }); + }); + }); + + describe('unsuccessful request', () => { + const errors = [ + { + name: 'base', + reason: 'Unable to reset password', + }, + ]; + const errorResponse = { + message: { + message: 'Unable to perform reset', + errors, + }, + }; + const resetParams = { password: 'foobar' }; + + beforeEach(() => { + spyOn(Kolide.default, 'performRequiredPasswordReset').andCall(() => { + return Promise.reject(errorResponse); + }); + }); + + afterEach(restoreSpies); + + it('calls the resetFunc', () => { + const mockStore = reduxMockStore(store); + + return mockStore.dispatch(performRequiredPasswordReset(resetParams)) + .then(() => { + throw new Error('promise should have failed'); + }) + .catch(() => { + expect(Kolide.default.performRequiredPasswordReset).toHaveBeenCalledWith(resetParams); + }); + }); + + it('dispatches the correct actions', () => { + const mockStore = reduxMockStore(store); + + const expectedActions = [ + { type: PERFORM_REQUIRED_PASSWORD_RESET_REQUEST }, + { + type: PERFORM_REQUIRED_PASSWORD_RESET_FAILURE, + payload: { errors: { base: 'Unable to reset password' } }, + }, + ]; + + return mockStore.dispatch(performRequiredPasswordReset(resetParams)) + .then(() => { + throw new Error('promise should have failed'); + }) + .catch(() => { + expect(mockStore.getActions()).toEqual(expectedActions); + }); + }); + }); + }); +}); diff --git a/frontend/redux/nodes/auth/reducer.js b/frontend/redux/nodes/auth/reducer.js index 171cadfc1a..cd2fc4a40d 100644 --- a/frontend/redux/nodes/auth/reducer.js +++ b/frontend/redux/nodes/auth/reducer.js @@ -9,6 +9,9 @@ import { LOGOUT_FAILURE, LOGOUT_REQUEST, LOGOUT_SUCCESS, + PERFORM_REQUIRED_PASSWORD_RESET_REQUEST, + PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS, + PERFORM_REQUIRED_PASSWORD_RESET_FAILURE, } from './actions'; export const initialState = { @@ -67,6 +70,25 @@ const reducer = (state = initialState, action) => { loading: false, errors: action.payload.errors, }; + case PERFORM_REQUIRED_PASSWORD_RESET_REQUEST: + return { + ...state, + errors: {}, + loading: true, + }; + case PERFORM_REQUIRED_PASSWORD_RESET_SUCCESS: + return { + ...state, + errors: {}, + loading: false, + user: action.payload.user, + }; + case PERFORM_REQUIRED_PASSWORD_RESET_FAILURE: + return { + ...state, + loading: false, + errors: action.payload.errors, + }; default: return state; } diff --git a/frontend/redux/nodes/auth/reducer.tests.js b/frontend/redux/nodes/auth/reducer.tests.js index 6082f3be4f..34b7e1855f 100644 --- a/frontend/redux/nodes/auth/reducer.tests.js +++ b/frontend/redux/nodes/auth/reducer.tests.js @@ -14,6 +14,9 @@ import { logoutUser, LOGOUT_REQUEST, LOGOUT_SUCCESS, + performRequiredPasswordResetRequest, + performRequiredPasswordResetSuccess, + performRequiredPasswordResetFailure, } from './actions'; import reducer, { initialState } from './reducer'; import { @@ -167,4 +170,52 @@ describe('Auth - reducer', () => { .catch(done); }); }); + + context('perform required password reset', () => { + const user = { id: 1, email: 'zwass@kolide.co', force_password_reset: true }; + + it('updates state when request is dispatched', () => { + const initState = { + ...initialState, + user, + }; + const newState = reducer(initState, performRequiredPasswordResetRequest); + + expect(newState).toEqual({ + ...initState, + loading: true, + }); + }); + + it('updates state when request is successful', () => { + const initState = { + ...initialState, + user, + loading: true, + }; + const newUser = { ...user, force_password_reset: false }; + const newState = reducer(initState, performRequiredPasswordResetSuccess(newUser)); + + expect(newState).toEqual({ + ...initState, + loading: false, + user: newUser, + }); + }); + + it('updates state when request fails', () => { + const initState = { + ...initialState, + loading: true, + }; + const errors = { base: 'Unable to reset password' }; + const newState = reducer(initState, performRequiredPasswordResetFailure(errors)); + + expect(newState).toEqual({ + ...initState, + errors, + loading: false, + }); + }); + }); }); diff --git a/frontend/redux/nodes/entities/users/actions.js b/frontend/redux/nodes/entities/users/actions.js index a0796a86fc..b708ee2aa4 100644 --- a/frontend/redux/nodes/entities/users/actions.js +++ b/frontend/redux/nodes/entities/users/actions.js @@ -4,6 +4,7 @@ import { formatErrorResponse } from 'redux/nodes/entities/base/helpers'; import config from './config'; +// Actions for admin to require password reset for a user export const REQUIRE_PASSWORD_RESET_REQUEST = 'REQUIRE_PASSWORD_RESET_REQUEST'; export const REQUIRE_PASSWORD_RESET_SUCCESS = 'REQUIRE_PASSWORD_RESET_SUCCESS'; export const REQUIRE_PASSWORD_RESET_FAILURE = 'REQUIRE_PASSWORD_RESET_FAILURE'; diff --git a/frontend/redux/nodes/entities/users/reducers.tests.js b/frontend/redux/nodes/entities/users/reducers.tests.js index 9bc6956b90..db60fb5f76 100644 --- a/frontend/redux/nodes/entities/users/reducers.tests.js +++ b/frontend/redux/nodes/entities/users/reducers.tests.js @@ -10,47 +10,49 @@ import { const user = { id: 1, email: 'zwass@kolide.co', force_password_reset: false }; describe('Users - reducer', () => { - const initialState = { - loading: false, - errors: {}, - data: { - [user.id]: user, - }, - }; - - it('updates state when request is dispatched', () => { - const newState = reducer(initialState, requirePasswordResetRequest); - - expect(newState).toEqual({ - ...initialState, - loading: true, - }); - }); - - it('updates state when request is successful', () => { - const initState = { - ...initialState, - loading: true, - }; - const newUser = { ...user, force_password_reset: true }; - const newState = reducer(initState, requirePasswordResetSuccess(newUser)); - - expect(newState).toEqual({ - ...initState, + describe('require password reset', () => { + const initialState = { loading: false, + errors: {}, data: { - [user.id]: newUser, + [user.id]: user, }, + }; + + it('updates state when request is dispatched', () => { + const newState = reducer(initialState, requirePasswordResetRequest); + + expect(newState).toEqual({ + ...initialState, + loading: true, + }); }); - }); - it('updates state when request fails', () => { - const errors = { base: 'Unable to require password reset' }; - const newState = reducer(initialState, requirePasswordResetFailure(errors)); + it('updates state when request is successful', () => { + const initState = { + ...initialState, + loading: true, + }; + const newUser = { ...user, force_password_reset: true }; + const newState = reducer(initState, requirePasswordResetSuccess(newUser)); - expect(newState).toEqual({ - ...initialState, - errors, + expect(newState).toEqual({ + ...initState, + loading: false, + data: { + [user.id]: newUser, + }, + }); + }); + + it('updates state when request fails', () => { + const errors = { base: 'Unable to require password reset' }; + const newState = reducer(initialState, requirePasswordResetFailure(errors)); + + expect(newState).toEqual({ + ...initialState, + errors, + }); }); }); }); diff --git a/server/kolide/users.go b/server/kolide/users.go index 871fb5049f..2337070a00 100644 --- a/server/kolide/users.go +++ b/server/kolide/users.go @@ -53,6 +53,11 @@ type UserService interface { // The updated user is returned. RequirePasswordReset(ctx context.Context, uid uint, require bool) (*User, error) + // PerformRequiredPasswordReset resets a password for a user that is in + // the required reset state. It must be called with the logged in + // viewer context of that user. + PerformRequiredPasswordReset(ctx context.Context, password string) (*User, error) + // ResetPassword validates the provided password reset token and // updates the user's password. ResetPassword(ctx context.Context, token, password string) (err error) diff --git a/server/service/endpoint_users.go b/server/service/endpoint_users.go index ce25232cb3..234b1370ba 100644 --- a/server/service/endpoint_users.go +++ b/server/service/endpoint_users.go @@ -175,6 +175,33 @@ func makeModifyUserEndpoint(svc kolide.Service) endpoint.Endpoint { } } +//////////////////////////////////////////////////////////////////////////////// +// Perform Required Password Reset +//////////////////////////////////////////////////////////////////////////////// + +type performRequiredPasswordResetRequest struct { + Password string `json:"new_password"` + ID uint `json:"id"` +} + +type performRequiredPasswordResetResponse struct { + User *kolide.User `json:"user,omitempty"` + Err error `json:"error,omitempty"` +} + +func (r performRequiredPasswordResetResponse) error() error { return r.Err } + +func makePerformRequiredPasswordResetEndpoint(svc kolide.Service) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(performRequiredPasswordResetRequest) + user, err := svc.PerformRequiredPasswordReset(ctx, req.Password) + if err != nil { + return performRequiredPasswordResetResponse{Err: err}, nil + } + return performRequiredPasswordResetResponse{User: user}, nil + } +} + //////////////////////////////////////////////////////////////////////////////// // Require Password Reset //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/handler.go b/server/service/handler.go index f3ce35a19e..54a2730d27 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -25,6 +25,7 @@ type KolideEndpoints struct { ListUsers endpoint.Endpoint ModifyUser endpoint.Endpoint RequirePasswordReset endpoint.Endpoint + PerformRequiredPasswordReset endpoint.Endpoint GetSessionsForUserInfo endpoint.Endpoint DeleteSessionsForUser endpoint.Endpoint GetSessionInfo endpoint.Endpoint @@ -87,12 +88,15 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint // stricter/different checks and should NOT also use // canPerformActions (these other checks should also call // canPerformActions if that is appropriate). - Me: authenticatedUser(jwtKey, svc, canPerformActions(makeGetSessionUserEndpoint(svc))), - ChangePassword: authenticatedUser(jwtKey, svc, canPerformActions(makeChangePasswordEndpoint(svc))), - GetUser: authenticatedUser(jwtKey, svc, canReadUser(makeGetUserEndpoint(svc))), - ListUsers: authenticatedUser(jwtKey, svc, canPerformActions(makeListUsersEndpoint(svc))), - ModifyUser: authenticatedUser(jwtKey, svc, validateModifyUserRequest(makeModifyUserEndpoint(svc))), - RequirePasswordReset: authenticatedUser(jwtKey, svc, mustBeAdmin(makeRequirePasswordResetEndpoint(svc))), + Me: authenticatedUser(jwtKey, svc, canPerformActions(makeGetSessionUserEndpoint(svc))), + ChangePassword: authenticatedUser(jwtKey, svc, canPerformActions(makeChangePasswordEndpoint(svc))), + GetUser: authenticatedUser(jwtKey, svc, canReadUser(makeGetUserEndpoint(svc))), + ListUsers: authenticatedUser(jwtKey, svc, canPerformActions(makeListUsersEndpoint(svc))), + ModifyUser: authenticatedUser(jwtKey, svc, validateModifyUserRequest(makeModifyUserEndpoint(svc))), + RequirePasswordReset: authenticatedUser(jwtKey, svc, mustBeAdmin(makeRequirePasswordResetEndpoint(svc))), + // PerformRequiredPasswordReset needs only to authenticate the + // logged in user + PerformRequiredPasswordReset: authenticatedUser(jwtKey, svc, makePerformRequiredPasswordResetEndpoint(svc)), GetSessionsForUserInfo: authenticatedUser(jwtKey, svc, canReadUser(makeGetInfoAboutSessionsForUserEndpoint(svc))), DeleteSessionsForUser: authenticatedUser(jwtKey, svc, canModifyUser(makeDeleteSessionsForUserEndpoint(svc))), GetSessionInfo: authenticatedUser(jwtKey, svc, mustBeAdmin(makeGetInfoAboutSessionEndpoint(svc))), @@ -152,6 +156,7 @@ type kolideHandlers struct { ListUsers http.Handler ModifyUser http.Handler RequirePasswordReset http.Handler + PerformRequiredPasswordReset http.Handler GetSessionsForUserInfo http.Handler DeleteSessionsForUser http.Handler GetSessionInfo http.Handler @@ -213,6 +218,7 @@ func makeKolideKitHandlers(ctx context.Context, e KolideEndpoints, opts []kithtt ListUsers: newServer(e.ListUsers, decodeListUsersRequest), ModifyUser: newServer(e.ModifyUser, decodeModifyUserRequest), RequirePasswordReset: newServer(e.RequirePasswordReset, decodeRequirePasswordResetRequest), + PerformRequiredPasswordReset: newServer(e.PerformRequiredPasswordReset, decodePerformRequiredPasswordResetRequest), GetSessionsForUserInfo: newServer(e.GetSessionsForUserInfo, decodeGetInfoAboutSessionsForUserRequest), DeleteSessionsForUser: newServer(e.DeleteSessionsForUser, decodeDeleteSessionsForUserRequest), GetSessionInfo: newServer(e.GetSessionInfo, decodeGetInfoAboutSessionRequest), @@ -303,6 +309,7 @@ func attachKolideAPIRoutes(r *mux.Router, h *kolideHandlers) { r.Handle("/api/v1/kolide/reset_password", h.ResetPassword).Methods("POST").Name("reset_password") r.Handle("/api/v1/kolide/me", h.Me).Methods("GET").Name("me") r.Handle("/api/v1/kolide/change_password", h.ChangePassword).Methods("POST").Name("change_password") + r.Handle("/api/v1/kolide/perform_required_password_reset", h.PerformRequiredPasswordReset).Methods("POST").Name("perform_required_password_reset") r.Handle("/api/v1/kolide/users", h.ListUsers).Methods("GET").Name("list_users") r.Handle("/api/v1/kolide/users", h.CreateUser).Methods("POST").Name("create_user") diff --git a/server/service/logging_users.go b/server/service/logging_users.go index 743c5fa943..e3d5a896ff 100644 --- a/server/service/logging_users.go +++ b/server/service/logging_users.go @@ -260,3 +260,25 @@ func (mw loggingMiddleware) RequestPasswordReset(ctx context.Context, email stri err = mw.Service.RequestPasswordReset(ctx, email) return err } + +func (mw loggingMiddleware) PerformRequiredPasswordReset(ctx context.Context, password string) (*kolide.User, error) { + var ( + resetBy = "unauthenticated" + err error + ) + vc, ok := viewer.FromContext(ctx) + if ok { + resetBy = vc.Username() + } + defer func(begin time.Time) { + _ = mw.logger.Log( + "method", "PerformRequiredPasswordReset", + "err", err, + "reset_by", resetBy, + "took", time.Since(begin), + ) + }(time.Now()) + + user, err := mw.Service.PerformRequiredPasswordReset(ctx, password) + return user, err +} diff --git a/server/service/service_users.go b/server/service/service_users.go index f29fffe3ac..f4e0c8cbea 100644 --- a/server/service/service_users.go +++ b/server/service/service_users.go @@ -180,6 +180,34 @@ func (svc service) ResetPassword(ctx context.Context, token, password string) er return nil } +func (svc service) PerformRequiredPasswordReset(ctx context.Context, password string) (*kolide.User, error) { + vc, ok := viewer.FromContext(ctx) + if !ok { + return nil, errNoContext + } + user := vc.User + + if !user.AdminForcedPasswordReset { + return nil, errors.New("user does not require password reset") + } + + // prevent setting the same password + if err := user.ValidatePassword(password); err == nil { + return nil, newInvalidArgumentError("new_password", "cannot reuse old password") + } + + user.AdminForcedPasswordReset = false + err := svc.setNewPassword(ctx, user, password) + if err != nil { + return nil, errors.Wrap(err, "setting new password") + } + + // Sessions should already have been cleared when the reset was + // required + + return user, nil +} + func (svc service) RequirePasswordReset(ctx context.Context, uid uint, require bool) (*kolide.User, error) { user, err := svc.ds.UserByID(uid) if err != nil { diff --git a/server/service/service_users_test.go b/server/service/service_users_test.go index 58072bf6a1..02b47da90e 100644 --- a/server/service/service_users_test.go +++ b/server/service/service_users_test.go @@ -426,3 +426,62 @@ func TestRequirePasswordReset(t *testing.T) { }) } } + +func TestPerformRequiredPasswordReset(t *testing.T) { + ds, err := inmem.New(config.TestConfig()) + require.Nil(t, err) + svc, err := newTestService(ds, nil) + require.Nil(t, err) + + createTestUsers(t, ds) + + for _, tt := range testUsers { + t.Run(tt.Username, func(t *testing.T) { + if !tt.Enabled { + return + } + + user, err := ds.User(tt.Username) + require.Nil(t, err) + + ctx := context.Background() + + _, err = svc.RequirePasswordReset(ctx, user.ID, true) + require.Nil(t, err) + + // should error when not logged in + _, err = svc.PerformRequiredPasswordReset(ctx, "new_pass") + require.NotNil(t, err) + + session, err := ds.NewSession(&kolide.Session{ + UserID: user.ID, + }) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: user, Session: session}) + + // should error when reset not required + _, err = svc.RequirePasswordReset(ctx, user.ID, false) + require.Nil(t, err) + _, err = svc.PerformRequiredPasswordReset(ctx, "new_pass") + require.NotNil(t, err) + + _, err = svc.RequirePasswordReset(ctx, user.ID, true) + require.Nil(t, err) + + // should error when using same password + _, err = svc.PerformRequiredPasswordReset(ctx, tt.PlaintextPassword) + require.NotNil(t, err) + + // should succeed with good new password + u, err := svc.PerformRequiredPasswordReset(ctx, "new_pass") + require.Nil(t, err) + assert.False(t, u.AdminForcedPasswordReset) + + ctx = context.Background() + + // Now user should be able to login with new password + u, _, err = svc.Login(ctx, tt.Username, "new_pass") + require.Nil(t, err) + assert.False(t, u.AdminForcedPasswordReset) + }) + } +} diff --git a/server/service/transport_users.go b/server/service/transport_users.go index 7d61640751..06613e9f1d 100644 --- a/server/service/transport_users.go +++ b/server/service/transport_users.go @@ -69,6 +69,14 @@ func decodeRequirePasswordResetRequest(ctx context.Context, r *http.Request) (in return req, nil } +func decodePerformRequiredPasswordResetRequest(ctx context.Context, r *http.Request) (interface{}, error) { + var req performRequiredPasswordResetRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return nil, errors.Wrap(err, "decoding JSON") + } + return req, nil +} + func decodeForgotPasswordRequest(ctx context.Context, r *http.Request) (interface{}, error) { var req forgotPasswordRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil {