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
This commit is contained in:
Zachary Wasserman 2017-01-09 20:42:50 -08:00 committed by Mike Arpaia
parent 71def50756
commit 60428e01c4
16 changed files with 442 additions and 46 deletions

View file

@ -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',

View file

@ -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();

View file

@ -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('/')); });
}

View file

@ -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;
});
};
};

View file

@ -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);
});
});
});
});
});

View file

@ -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;
}

View file

@ -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,
});
});
});
});

View file

@ -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';

View file

@ -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,
});
});
});
});

View file

@ -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)

View file

@ -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
////////////////////////////////////////////////////////////////////////////////

View file

@ -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")

View file

@ -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
}

View file

@ -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 {

View file

@ -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)
})
}
}

View file

@ -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 {