Refactor require password reset into separate endpoint (#725)

- Remove require password reset from ModifyUser and
  RequestPasswordReset methods, and UserPayload struct
- Add new RequirePasswordReset method
- Refactor JS for new separate method
This commit is contained in:
Zachary Wasserman
2017-01-06 14:38:39 -08:00
committed by GitHub
parent 39c9c6b0da
commit 77e4f3d936
15 changed files with 413 additions and 90 deletions
+11
View File
@@ -384,6 +384,17 @@ class Kolide extends Base {
return helpers.addGravatarUrlToResource(updatedUser);
});
}
requirePasswordReset = (user, { require }) => {
const { USERS } = endpoints;
const requirePasswordResetEndpoint = this.endpoint(`${USERS}/${user.id}/require_password_reset`);
return this.authenticatedPost(requirePasswordResetEndpoint, JSON.stringify({ require }))
.then((response) => {
const { user: updatedUser } = response;
return helpers.addGravatarUrlToResource(updatedUser);
});
}
}
export default new Kolide();
@@ -55,7 +55,7 @@ class UserManagementPage extends Component {
onUserActionSelect = (user, action) => {
const { currentUser, dispatch } = this.props;
const { update } = userActions;
const { update, requirePasswordReset } = userActions;
if (action) {
switch (action) {
@@ -88,9 +88,9 @@ class UserManagementPage extends Component {
return dispatch(renderFlash('success', 'User promoted to admin', update(user, { admin: false })));
});
case 'reset_password':
return dispatch(update(user, { force_password_reset: true }))
return dispatch(requirePasswordReset(user, { require: true }))
.then(() => {
return dispatch(renderFlash('success', 'User forced to reset password', update(user, { force_password_reset: false })));
return dispatch(renderFlash('success', 'User required to reset password', requirePasswordReset(user, { require: false })));
});
case 'revert_invitation':
return dispatch(inviteActions.destroy(user))
@@ -225,4 +225,3 @@ const mapStateToProps = (state) => {
};
export default connect(mapStateToProps)(UserManagementPage);
@@ -3,7 +3,7 @@ import { normalize, arrayOf } from 'normalizr';
import { entitiesExceptID, formatErrorResponse } from 'redux/nodes/entities/base/helpers';
const initialState = {
export const initialState = {
loading: false,
errors: {},
data: {},
+44 -1
View File
@@ -1,3 +1,46 @@
import Kolide from 'kolide';
import { formatErrorResponse } from 'redux/nodes/entities/base/helpers';
import config from './config';
export default config.actions;
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';
export const requirePasswordResetRequest = { type: REQUIRE_PASSWORD_RESET_REQUEST };
export const requirePasswordResetSuccess = (user) => {
return {
type: REQUIRE_PASSWORD_RESET_SUCCESS,
payload: { user },
};
};
export const requirePasswordResetFailure = (errors) => {
return {
type: REQUIRE_PASSWORD_RESET_FAILURE,
payload: { errors },
};
};
export const requirePasswordReset = (user, { require }) => {
return (dispatch) => {
dispatch(requirePasswordResetRequest);
return Kolide.requirePasswordReset(user, { require })
.then((updatedUser) => {
dispatch(requirePasswordResetSuccess(updatedUser));
return updatedUser;
})
.catch((response) => {
const errorsObject = formatErrorResponse(response);
dispatch(requirePasswordResetFailure(errorsObject));
throw response;
});
};
};
export default { ...config.actions, requirePasswordReset };
@@ -0,0 +1,110 @@
import expect, { restoreSpies, spyOn } from 'expect';
import * as Kolide from 'kolide';
import { reduxMockStore } from 'test/helpers';
import {
requirePasswordReset,
REQUIRE_PASSWORD_RESET_REQUEST,
REQUIRE_PASSWORD_RESET_FAILURE,
REQUIRE_PASSWORD_RESET_SUCCESS,
} from './actions';
const store = { entities: { invites: {}, users: {} } };
const user = { id: 1, email: 'zwass@kolide.co', force_password_reset: false };
describe('Users - actions', () => {
describe('dispatching the require password reset action', () => {
describe('successful request', () => {
beforeEach(() => {
spyOn(Kolide.default, 'requirePasswordReset').andCall(() => {
return Promise.resolve({ ...user, force_password_reset: true });
});
});
afterEach(restoreSpies);
it('calls the resetFunc', () => {
const mockStore = reduxMockStore(store);
return mockStore.dispatch(requirePasswordReset(user, { require: true }))
.then(() => {
expect(Kolide.default.requirePasswordReset).toHaveBeenCalledWith(user, { require: true });
});
});
it('dispatches the correct actions', () => {
const mockStore = reduxMockStore(store);
const expectedActions = [
{ type: REQUIRE_PASSWORD_RESET_REQUEST },
{
type: REQUIRE_PASSWORD_RESET_SUCCESS,
payload: { user: { ...user, force_password_reset: true } },
},
];
return mockStore.dispatch(requirePasswordReset(user, { require: true }))
.then(() => {
expect(mockStore.getActions()).toEqual(expectedActions);
});
});
});
describe('unsuccessful request', () => {
const errors = [
{
name: 'base',
reason: 'Unable to require password reset',
},
];
const errorResponse = {
message: {
message: 'Unable to require password reset',
errors,
},
};
beforeEach(() => {
spyOn(Kolide.default, 'requirePasswordReset').andCall(() => {
return Promise.reject(errorResponse);
});
});
afterEach(restoreSpies);
it('calls the resetFunc', () => {
const mockStore = reduxMockStore(store);
return mockStore.dispatch(requirePasswordReset(user, { require: true }))
.then(() => {
throw new Error('promise should have failed');
})
.catch(() => {
expect(Kolide.default.requirePasswordReset).toHaveBeenCalledWith(user, { require: true });
});
});
it('dispatches the correct actions', () => {
const mockStore = reduxMockStore(store);
const expectedActions = [
{ type: REQUIRE_PASSWORD_RESET_REQUEST },
{
type: REQUIRE_PASSWORD_RESET_FAILURE,
payload: { errors: { base: 'Unable to require password reset' } },
},
];
return mockStore.dispatch(requirePasswordReset(user, { require: true }))
.then(() => {
throw new Error('promise should have failed');
})
.catch(() => {
expect(mockStore.getActions()).toEqual(expectedActions);
});
});
});
});
});
+34 -2
View File
@@ -1,3 +1,35 @@
import config from './config';
import {
REQUIRE_PASSWORD_RESET_FAILURE,
REQUIRE_PASSWORD_RESET_REQUEST,
REQUIRE_PASSWORD_RESET_SUCCESS,
} from './actions';
import config, { initialState } from './config';
export default config.reducer;
export default (state = initialState, { type, payload }) => {
switch (type) {
case REQUIRE_PASSWORD_RESET_REQUEST:
return {
...state,
errors: {},
loading: true,
};
case REQUIRE_PASSWORD_RESET_SUCCESS:
return {
...state,
errors: {},
loading: false,
data: {
...state.data,
[payload.user.id]: payload.user,
},
};
case REQUIRE_PASSWORD_RESET_FAILURE:
return {
...state,
loading: false,
errors: payload.errors,
};
default:
return config.reducer(state, { type, payload });
}
};
@@ -0,0 +1,56 @@
import expect from 'expect';
import reducer from './reducer';
import {
requirePasswordResetRequest,
requirePasswordResetFailure,
requirePasswordResetSuccess,
} from './actions';
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,
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,
});
});
});
+1 -5
View File
@@ -1,7 +1,6 @@
package inmem
import (
"fmt"
"time"
"github.com/kolide/kolide-ose/server/kolide"
@@ -39,10 +38,7 @@ func (d *Datastore) ListSessionsForUser(id uint) ([]*kolide.Session, error) {
sessions = append(sessions, session)
}
}
if len(sessions) == 0 {
return nil, notFound("Session").
WithMessage(fmt.Sprintf("for user id %d", id))
}
return sessions, nil
}
+28 -25
View File
@@ -19,40 +19,45 @@ type UserStore interface {
SaveUser(user *User) error
}
// UserService contains methods for managing a Kolide User
// UserService contains methods for managing a Kolide User.
type UserService interface {
// NewUser creates a new User from a request Payload
// NewUser creates a new User from a request Payload.
NewUser(ctx context.Context, p UserPayload) (user *User, err error)
// NewAdminCreatedUser allows an admin to create a new user without
// first creating and validating invite tokens.
NewAdminCreatedUser(ctx context.Context, p UserPayload) (user *User, err error)
// User returns a valid User given a User ID
// User returns a valid User given a User ID.
User(ctx context.Context, id uint) (user *User, err error)
// AuthenticatedUser returns the current user
// from the viewer context
// AuthenticatedUser returns the current user from the viewer context.
AuthenticatedUser(ctx context.Context) (user *User, err error)
// Users returns all users
// ListUsers returns all users.
ListUsers(ctx context.Context, opt ListOptions) (users []*User, err error)
// ChangePassword validates the existing password, and sets the new
// password. User is retrieved from the viewer context.
ChangePassword(ctx context.Context, oldPass, newPass string) error
// RequestPasswordReset generates a password reset request for
// a user. The request results in a token emailed to the user.
// If the person making the request is an admin the AdminForcedPasswordReset
// parameter is enabled instead of sending an email with a password reset token
// RequestPasswordReset generates a password reset request for the user
// specified by email. The request results in a token emailed to the
// user.
RequestPasswordReset(ctx context.Context, email string) (err error)
// ResetPassword validate a password reset token and updates
// a user's password
// RequirePasswordReset requires a password reset for the user
// specified by ID (if require is true). It deletes all of the user's
// sessions, and requires that their password be reset upon the next
// login. Setting require to false will take a user out of this state.
// The updated user is returned.
RequirePasswordReset(ctx context.Context, uid uint, require bool) (*User, error)
// ResetPassword validates the provided password reset token and
// updates the user's password.
ResetPassword(ctx context.Context, token, password string) (err error)
// ModifyUser updates a user's parameters given a UserPayload
// ModifyUser updates a user's parameters given a UserPayload.
ModifyUser(ctx context.Context, userID uint, p UserPayload) (user *User, err error)
}
@@ -75,16 +80,15 @@ type User struct {
// UserPayload is used to modify an existing user
type UserPayload struct {
Username *string `json:"username"`
Name *string `json:"name"`
Email *string `json:"email"`
Admin *bool `json:"admin"`
Enabled *bool `json:"enabled"`
AdminForcedPasswordReset *bool `json:"force_password_reset"`
Password *string `json:"password"`
GravatarURL *string `json:"gravatar_url"`
Position *string `json:"position"`
InviteToken *string `json:"invite_token"`
Username *string `json:"username"`
Name *string `json:"name"`
Email *string `json:"email"`
Admin *bool `json:"admin"`
Enabled *bool `json:"enabled"`
Password *string `json:"password"`
GravatarURL *string `json:"gravatar_url"`
Position *string `json:"position"`
InviteToken *string `json:"invite_token"`
}
// User creates a user from payload.
@@ -94,8 +98,7 @@ func (p UserPayload) User(keySize, cost int) (*User, error) {
Username: *p.Username,
Email: *p.Email,
Admin: falseIfNil(p.Admin),
AdminForcedPasswordReset: falseIfNil(p.AdminForcedPasswordReset),
Enabled: true,
Enabled: true,
}
if err := user.SetPassword(*p.Password, keySize, cost); err != nil {
return nil, err
-3
View File
@@ -220,9 +220,6 @@ func requireRoleForUserModification(p kolide.UserPayload) map[permission][]strin
if p.Admin != nil {
adminFields = append(adminFields, "admin")
}
if p.AdminForcedPasswordReset != nil {
adminFields = append(adminFields, "force_password_reset")
}
if len(adminFields) != 0 {
must[admin] = adminFields
}
+27
View File
@@ -175,6 +175,33 @@ func makeModifyUserEndpoint(svc kolide.Service) endpoint.Endpoint {
}
}
////////////////////////////////////////////////////////////////////////////////
// Require Password Reset
////////////////////////////////////////////////////////////////////////////////
type requirePasswordResetRequest struct {
Require bool `json:"require"`
ID uint `json:"id"`
}
type requirePasswordResetResponse struct {
User *kolide.User `json:"user,omitempty"`
Err error `json:"error,omitempty"`
}
func (r requirePasswordResetResponse) error() error { return r.Err }
func makeRequirePasswordResetEndpoint(svc kolide.Service) endpoint.Endpoint {
return func(ctx context.Context, request interface{}) (interface{}, error) {
req := request.(requirePasswordResetRequest)
user, err := svc.RequirePasswordReset(ctx, req.ID, req.Require)
if err != nil {
return requirePasswordResetResponse{Err: err}, nil
}
return requirePasswordResetResponse{User: user}, nil
}
}
////////////////////////////////////////////////////////////////////////////////
// Forgot Password
////////////////////////////////////////////////////////////////////////////////
+5
View File
@@ -24,6 +24,7 @@ type KolideEndpoints struct {
GetUser endpoint.Endpoint
ListUsers endpoint.Endpoint
ModifyUser endpoint.Endpoint
RequirePasswordReset endpoint.Endpoint
GetSessionsForUserInfo endpoint.Endpoint
DeleteSessionsForUser endpoint.Endpoint
GetSessionInfo endpoint.Endpoint
@@ -91,6 +92,7 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint
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))),
GetSessionsForUserInfo: authenticatedUser(jwtKey, svc, canReadUser(makeGetInfoAboutSessionsForUserEndpoint(svc))),
DeleteSessionsForUser: authenticatedUser(jwtKey, svc, canModifyUser(makeDeleteSessionsForUserEndpoint(svc))),
GetSessionInfo: authenticatedUser(jwtKey, svc, mustBeAdmin(makeGetInfoAboutSessionEndpoint(svc))),
@@ -149,6 +151,7 @@ type kolideHandlers struct {
GetUser http.Handler
ListUsers http.Handler
ModifyUser http.Handler
RequirePasswordReset http.Handler
GetSessionsForUserInfo http.Handler
DeleteSessionsForUser http.Handler
GetSessionInfo http.Handler
@@ -209,6 +212,7 @@ func makeKolideKitHandlers(ctx context.Context, e KolideEndpoints, opts []kithtt
GetUser: newServer(e.GetUser, decodeGetUserRequest),
ListUsers: newServer(e.ListUsers, decodeListUsersRequest),
ModifyUser: newServer(e.ModifyUser, decodeModifyUserRequest),
RequirePasswordReset: newServer(e.RequirePasswordReset, decodeRequirePasswordResetRequest),
GetSessionsForUserInfo: newServer(e.GetSessionsForUserInfo, decodeGetInfoAboutSessionsForUserRequest),
DeleteSessionsForUser: newServer(e.DeleteSessionsForUser, decodeDeleteSessionsForUserRequest),
GetSessionInfo: newServer(e.GetSessionInfo, decodeGetInfoAboutSessionRequest),
@@ -304,6 +308,7 @@ func attachKolideAPIRoutes(r *mux.Router, h *kolideHandlers) {
r.Handle("/api/v1/kolide/users", h.CreateUser).Methods("POST").Name("create_user")
r.Handle("/api/v1/kolide/users/{id}", h.GetUser).Methods("GET").Name("get_user")
r.Handle("/api/v1/kolide/users/{id}", h.ModifyUser).Methods("PATCH").Name("modify_user")
r.Handle("/api/v1/kolide/users/{id}/require_password_reset", h.RequirePasswordReset).Methods("POST").Name("require_password_reset")
r.Handle("/api/v1/kolide/users/{id}/sessions", h.GetSessionsForUserInfo).Methods("GET").Name("get_session_for_user")
r.Handle("/api/v1/kolide/users/{id}/sessions", h.DeleteSessionsForUser).Methods("DELETE").Name("delete_session_for_user")
+23 -41
View File
@@ -85,33 +85,12 @@ func (svc service) ModifyUser(ctx context.Context, userID uint, p kolide.UserPay
user.GravatarURL = *p.GravatarURL
}
if p.Password != nil {
err := user.SetPassword(
*p.Password,
svc.config.Auth.SaltKeySize,
svc.config.Auth.BcryptCost,
)
if err != nil {
return nil, err
}
user.AdminForcedPasswordReset = false
}
err = svc.saveUser(user)
if err != nil {
return nil, err
}
// https://github.com/kolide/kolide-ose/issues/351
// Calling this action last, because svc.RequestPasswordReset saves the
// user separately and we don't want to override the value set there
if p.AdminForcedPasswordReset != nil && *p.AdminForcedPasswordReset {
err = svc.RequestPasswordReset(ctx, user.Email)
if err != nil {
return nil, err
}
}
return svc.User(ctx, userID)
return user, nil
}
func (svc service) User(ctx context.Context, id uint) (*kolide.User, error) {
@@ -201,30 +180,33 @@ func (svc service) ResetPassword(ctx context.Context, token, password string) er
return nil
}
func (svc service) RequirePasswordReset(ctx context.Context, uid uint, require bool) (*kolide.User, error) {
user, err := svc.ds.UserByID(uid)
if err != nil {
return nil, errors.Wrap(err, "loading user by ID")
}
// Require reset on next login
user.AdminForcedPasswordReset = require
if err := svc.saveUser(user); err != nil {
return nil, errors.Wrap(err, "saving user")
}
if require {
// Clear all of the existing sessions
if err := svc.DeleteSessionsForUser(ctx, user.ID); err != nil {
return nil, errors.Wrap(err, "deleting user sessions")
}
}
return user, nil
}
func (svc service) RequestPasswordReset(ctx context.Context, email string) error {
// 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)
if err != nil {
return err
}
vc, ok := viewer.FromContext(ctx)
if ok {
if vc.IsAdmin() {
user.AdminForcedPasswordReset = true
if err := svc.saveUser(user); err != nil {
return err
}
// Sessions should only be cleared if this is an admin
// forced password reset
if err := svc.DeleteSessionsForUser(ctx, user.ID); err != nil {
return err
}
return nil
}
}
random, err := kolide.RandomText(svc.config.App.TokenKeySize)
if err != nil {
+54 -8
View File
@@ -19,7 +19,7 @@ import (
func TestAuthenticatedUser(t *testing.T) {
ds, err := inmem.New(config.TestConfig())
assert.Nil(t, err)
require.Nil(t, err)
createTestUsers(t, ds)
svc, err := newTestService(ds, nil)
assert.Nil(t, err)
@@ -183,12 +183,11 @@ func TestCreateUser(t *testing.T) {
for _, tt := range createUserTests {
t.Run("", func(t *testing.T) {
payload := kolide.UserPayload{
Username: tt.Username,
Password: tt.Password,
Email: tt.Email,
Admin: tt.Admin,
InviteToken: tt.InviteToken,
AdminForcedPasswordReset: tt.NeedsPasswordReset,
Username: tt.Username,
Password: tt.Password,
Email: tt.Email,
Admin: tt.Admin,
InviteToken: tt.InviteToken,
}
user, err := svc.NewUser(ctx, payload)
if tt.wantErr != nil {
@@ -207,7 +206,6 @@ func TestCreateUser(t *testing.T) {
err = user.ValidatePassword("different_password")
assert.NotNil(t, err)
assert.Equal(t, user.AdminForcedPasswordReset, *tt.NeedsPasswordReset)
assert.Equal(t, user.Admin, *tt.Admin)
})
@@ -375,3 +373,51 @@ func TestResetPassword(t *testing.T) {
})
}
}
func TestRequirePasswordReset(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) {
user, err := ds.User(tt.Username)
require.Nil(t, err)
var sessions []*kolide.Session
ctx := context.Background()
// Log user in
if tt.Enabled {
_, _, err = svc.Login(ctx, tt.Username, tt.PlaintextPassword)
require.Nil(t, err, "login unsuccesful")
sessions, err = svc.GetInfoAboutSessionsForUser(ctx, user.ID)
require.Nil(t, err)
require.Len(t, sessions, 1, "user should have one session")
}
// Reset and verify sessions destroyed
retUser, err := svc.RequirePasswordReset(ctx, user.ID, true)
require.Nil(t, err)
assert.True(t, retUser.AdminForcedPasswordReset)
checkUser, err := ds.User(tt.Username)
require.Nil(t, err)
assert.True(t, checkUser.AdminForcedPasswordReset)
sessions, err = svc.GetInfoAboutSessionsForUser(ctx, user.ID)
require.Nil(t, err)
require.Len(t, sessions, 0, "sessions should be destroyed")
// try undo
retUser, err = svc.RequirePasswordReset(ctx, user.ID, false)
require.Nil(t, err)
assert.False(t, retUser.AdminForcedPasswordReset)
checkUser, err = ds.User(tt.Username)
require.Nil(t, err)
assert.False(t, checkUser.AdminForcedPasswordReset)
})
}
}
+16
View File
@@ -4,6 +4,7 @@ import (
"encoding/json"
"net/http"
"github.com/pkg/errors"
"golang.org/x/net/context"
)
@@ -53,6 +54,21 @@ func decodeChangePasswordRequest(ctx context.Context, r *http.Request) (interfac
return req, nil
}
func decodeRequirePasswordResetRequest(ctx context.Context, r *http.Request) (interface{}, error) {
id, err := idFromRequest(r, "id")
if err != nil {
return nil, errors.Wrap(err, "getting ID from request")
}
var req requirePasswordResetRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
return nil, errors.Wrap(err, "decoding JSON")
}
req.ID = id
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 {