diff --git a/server/datastore/inmem/users.go b/server/datastore/inmem/users.go index 560b4adac..fa93e2db6 100644 --- a/server/datastore/inmem/users.go +++ b/server/datastore/inmem/users.go @@ -114,3 +114,29 @@ func (d *Datastore) SaveUser(user *kolide.User) error { d.users[user.ID] = user return nil } + +func (d *Datastore) DeleteUserByID(id uint) error { + d.mtx.Lock() + defer d.mtx.Unlock() + + if _, ok := d.users[id]; !ok { + return notFound("User").WithID(id) + } + delete(d.users, id) + return nil +} + +func (d *Datastore) DeleteUsers(ids []uint) (uint, error) { + d.mtx.Lock() + defer d.mtx.Unlock() + + deleted := uint(0) + for _, id := range ids { + if _, ok := d.users[id]; ok { + delete(d.users, id) + deleted++ + } + } + + return deleted, nil +} diff --git a/server/datastore/mysql/users.go b/server/datastore/mysql/users.go index d9686d8ea..d8f4e4b46 100644 --- a/server/datastore/mysql/users.go +++ b/server/datastore/mysql/users.go @@ -4,31 +4,84 @@ import ( "database/sql" "fmt" + "github.com/jmoiron/sqlx" "github.com/kolide/fleet/server/kolide" "github.com/pkg/errors" ) -// NewUser creates a new user +// NewUser creates a new user. If a user with the same username was +// soft-deleted, NewUser will replace the old one. func (d *Datastore) NewUser(user *kolide.User) (*kolide.User, error) { - sqlStatement := ` - INSERT INTO users ( - password, - salt, - name, - username, - email, - admin, - enabled, - admin_forced_password_reset, - gravatar_url, - position, - sso_enabled - ) VALUES (?,?,?,?,?,?,?,?,?,?,?) - ` - result, err := d.db.Exec(sqlStatement, user.Password, user.Salt, user.Name, - user.Username, user.Email, user.Admin, user.Enabled, - user.AdminForcedPasswordReset, user.GravatarURL, user.Position, user.SSOEnabled) + var ( + deletedUser kolide.User + sqlStatement string + ) + tx, err := d.db.Beginx() if err != nil { + return nil, errors.Wrap(err, "begin NewUser transaction") + } + + defer func() { + if err != nil { + rbErr := tx.Rollback() + // It seems possible that there might be a case in + // which the error we are dealing with here was thrown + // by the call to tx.Commit(), and the docs suggest + // this call would then result in sql.ErrTxDone. + if rbErr != nil && rbErr != sql.ErrTxDone { + panic(fmt.Sprintf("got err '%s' rolling back after err '%s'", rbErr, err)) + } + } + }() + + err = tx.Get(&deletedUser, + "SELECT * FROM users WHERE username = ? AND deleted", user.Username) + switch err { + case nil: + sqlStatement = ` + REPLACE INTO users ( + password, + salt, + name, + username, + email, + admin, + enabled, + admin_forced_password_reset, + gravatar_url, + position, + sso_enabled, + deleted + ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?) + ` + case sql.ErrNoRows: + sqlStatement = ` + INSERT INTO users ( + password, + salt, + name, + username, + email, + admin, + enabled, + admin_forced_password_reset, + gravatar_url, + position, + sso_enabled, + deleted + ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?) + ` + default: + return nil, errors.Wrap(err, "check for existing user") + } + deleted := false + result, err := tx.Exec(sqlStatement, user.Password, user.Salt, user.Name, + user.Username, user.Email, user.Admin, user.Enabled, + user.AdminForcedPasswordReset, user.GravatarURL, user.Position, + user.SSOEnabled, deleted) + if err != nil && isDuplicate(err) { + return nil, alreadyExists("User", deletedUser.ID) + } else if err != nil { return nil, errors.Wrap(err, "create new user") } @@ -119,3 +172,34 @@ func (d *Datastore) SaveUser(user *kolide.User) error { return nil } + +// DeleteUserByID (soft) deletes the existing user object with the provided ID. +func (d *Datastore) DeleteUserByID(id uint) error { + return d.deleteEntity("users", id) +} + +// DeleteUsers (soft) deletes the existing user objects with the provided IDs. +// The number of deleted queries is returned along with any error. +func (d *Datastore) DeleteUsers(ids []uint) (uint, error) { + sql := ` + UPDATE users + SET deleted_at = NOW(), deleted = true + WHERE id IN (?) AND NOT deleted + ` + query, args, err := sqlx.In(sql, ids) + if err != nil { + return 0, errors.Wrap(err, "building delete users query") + } + + result, err := d.db.Exec(query, args...) + if err != nil { + return 0, errors.Wrap(err, "updating delete users query") + } + + deleted, err := result.RowsAffected() + if err != nil { + return 0, errors.Wrap(err, "fetching delete users rows effected") + } + + return uint(deleted), nil +} diff --git a/server/kolide/users.go b/server/kolide/users.go index 2b50ba26c..d9030d8c9 100644 --- a/server/kolide/users.go +++ b/server/kolide/users.go @@ -19,6 +19,11 @@ type UserStore interface { UserByEmail(email string) (*User, error) UserByID(id uint) (*User, error) SaveUser(user *User) error + // DeleteUserByID (soft) deletes an existing user object by ID. + DeleteUserByID(id uint) error + // DeleteUsers (soft) deletes the existing user objects with the provided + // IDs. The number of deleted users is returned along with any error. + DeleteUsers(ids []uint) (uint, error) // PendingEmailChange creates a record with a pending email change for a user identified // by uid. The change record is keyed by a unique token. The token is emailed to the user // with a link that they can use to confirm the change. @@ -84,6 +89,13 @@ type UserService interface { // ChangeUserEmail is used to confirm new email address and if confirmed, // write the new email address to user. ChangeUserEmail(ctx context.Context, token string) (string, error) + + // DeleteUserByID (soft) deletes an existing user object by ID. + DeleteUserByID(ctx context.Context, id uint) error + + // DeleteUsers (soft) deletes the existing user objects with the provided + // IDs. The number of deleted users is returned along with any error. + DeleteUsers(ctx context.Context, ids []uint) (uint, error) } // User is the model struct which represents a kolide user diff --git a/server/mock/datastore_users.go b/server/mock/datastore_users.go index c70f88488..a84f77a5c 100644 --- a/server/mock/datastore_users.go +++ b/server/mock/datastore_users.go @@ -18,6 +18,10 @@ type UserByIDFunc func(id uint) (*kolide.User, error) type SaveUserFunc func(user *kolide.User) error +type DeleteUserByIDFunc func(id uint) error + +type DeleteUsersFunc func(ids []uint) (uint, error) + type PendingEmailChangeFunc func(userID uint, newEmail string, token string) error type ConfirmPendingEmailChangeFunc func(userID uint, token string) (string, error) @@ -41,6 +45,12 @@ type UserStore struct { SaveUserFunc SaveUserFunc SaveUserFuncInvoked bool + DeleteUserByIDFunc DeleteUserByIDFunc + DeleteUserByIDFuncInvoked bool + + DeleteUsersFunc DeleteUsersFunc + DeleteUsersFuncInvoked bool + PendingEmailChangeFunc PendingEmailChangeFunc PendingEmailChangeFuncInvoked bool @@ -78,6 +88,16 @@ func (s *UserStore) SaveUser(user *kolide.User) error { return s.SaveUserFunc(user) } +func (s *UserStore) DeleteUserByID(id uint) error { + s.DeleteUserByIDFuncInvoked = true + return s.DeleteUserByIDFunc(id) +} + +func (s *UserStore) DeleteUsers(ids []uint) (uint, error) { + s.DeleteUsersFuncInvoked = true + return s.DeleteUsersFunc(ids) +} + func (s *UserStore) PendingEmailChange(userID uint, newEmail string, token string) error { s.PendingEmailChangeFuncInvoked = true return s.PendingEmailChangeFunc(userID, newEmail, token) diff --git a/server/service/endpoint_users.go b/server/service/endpoint_users.go index 2544bcfa1..5eb17f354 100644 --- a/server/service/endpoint_users.go +++ b/server/service/endpoint_users.go @@ -221,6 +221,57 @@ func makeModifyUserEndpoint(svc kolide.Service) endpoint.Endpoint { } } +//////////////////////////////////////////////////////////////////////////////// +// Delete User By ID +//////////////////////////////////////////////////////////////////////////////// + +type deleteUserByIDRequest struct { + ID uint +} + +type deleteUserByIDResponse struct { + Err error `json:"error,omitempty"` +} + +func (r deleteUserByIDResponse) error() error { return r.Err } + +func makeDeleteUserByIDEndpoint(svc kolide.Service) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(deleteUserByIDRequest) + err := svc.DeleteUserByID(ctx, req.ID) + if err != nil { + return deleteUserByIDResponse{Err: err}, nil + } + return deleteUserByIDResponse{}, nil + } +} + +//////////////////////////////////////////////////////////////////////////////// +// Delete Users +//////////////////////////////////////////////////////////////////////////////// + +type deleteUsersRequest struct { + IDs []uint `json:"ids"` +} + +type deleteUsersResponse struct { + Deleted uint `json:"deleted"` + Err error `json:"error,omitempty"` +} + +func (r deleteUsersResponse) error() error { return r.Err } + +func makeDeleteUsersEndpoint(svc kolide.Service) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(deleteUsersRequest) + deleted, err := svc.DeleteUsers(ctx, req.IDs) + if err != nil { + return deleteUsersResponse{Err: err}, nil + } + return deleteUsersResponse{Deleted: deleted}, nil + } +} + //////////////////////////////////////////////////////////////////////////////// // Perform Required Password Reset //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/handler.go b/server/service/handler.go index f120a83be..54c442734 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -25,6 +25,8 @@ type KolideEndpoints struct { GetUser endpoint.Endpoint ListUsers endpoint.Endpoint ModifyUser endpoint.Endpoint + DeleteUserByID endpoint.Endpoint + DeleteUsers endpoint.Endpoint AdminUser endpoint.Endpoint EnableUser endpoint.Endpoint RequirePasswordReset endpoint.Endpoint @@ -123,6 +125,8 @@ 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, canModifyUser(makeModifyUserEndpoint(svc))), + DeleteUserByID: authenticatedUser(jwtKey, svc, mustBeAdmin(makeDeleteUserByIDEndpoint(svc))), + DeleteUsers: authenticatedUser(jwtKey, svc, mustBeAdmin(makeDeleteUsersEndpoint(svc))), AdminUser: authenticatedUser(jwtKey, svc, mustBeAdmin(makeAdminUserEndpoint(svc))), EnableUser: authenticatedUser(jwtKey, svc, mustBeAdmin(makeEnableUserEndpoint(svc))), RequirePasswordReset: authenticatedUser(jwtKey, svc, mustBeAdmin(makeRequirePasswordResetEndpoint(svc))), @@ -208,6 +212,8 @@ type kolideHandlers struct { GetUser http.Handler ListUsers http.Handler ModifyUser http.Handler + DeleteUserByID http.Handler + DeleteUsers http.Handler AdminUser http.Handler EnableUser http.Handler RequirePasswordReset http.Handler @@ -296,6 +302,8 @@ func makeKolideKitHandlers(e KolideEndpoints, opts []kithttp.ServerOption) *koli GetUser: newServer(e.GetUser, decodeGetUserRequest), ListUsers: newServer(e.ListUsers, decodeListUsersRequest), ModifyUser: newServer(e.ModifyUser, decodeModifyUserRequest), + DeleteUserByID: newServer(e.DeleteUserByID, decodeDeleteUserByIDRequest), + DeleteUsers: newServer(e.DeleteUsers, decodeDeleteUsersRequest), RequirePasswordReset: newServer(e.RequirePasswordReset, decodeRequirePasswordResetRequest), PerformRequiredPasswordReset: newServer(e.PerformRequiredPasswordReset, decodePerformRequiredPasswordResetRequest), EnableUser: newServer(e.EnableUser, decodeEnableUserRequest), @@ -423,11 +431,13 @@ 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}", h.DeleteUserByID).Methods("DELETE").Name("delete_user") r.Handle("/api/v1/kolide/users/{id}/enable", h.EnableUser).Methods("POST").Name("enable_user") r.Handle("/api/v1/kolide/users/{id}/admin", h.AdminUser).Methods("POST").Name("admin_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") + r.Handle("/api/v1/kolide/users/delete", h.DeleteUsers).Methods("POST").Name("delete_users") r.Handle("/api/v1/kolide/sessions/{id}", h.GetSessionInfo).Methods("GET").Name("get_session_info") r.Handle("/api/v1/kolide/sessions/{id}", h.DeleteSession).Methods("DELETE").Name("delete_session") diff --git a/server/service/handler_test.go b/server/service/handler_test.go index 0ffbc7cf9..ae597eb03 100644 --- a/server/service/handler_test.go +++ b/server/service/handler_test.go @@ -51,6 +51,14 @@ func TestAPIRoutes(t *testing.T) { verb: "PATCH", uri: "/api/v1/kolide/users/1", }, + { + verb: "DELETE", + uri: "/api/v1/kolide/users/1", + }, + { + verb: "POST", + uri: "/api/v1/kolide/users/delete", + }, { verb: "POST", uri: "/api/v1/kolide/login", diff --git a/server/service/service_users.go b/server/service/service_users.go index 1b8c3b52d..d92eb834e 100644 --- a/server/service/service_users.go +++ b/server/service/service_users.go @@ -362,6 +362,14 @@ func (svc service) RequestPasswordReset(ctx context.Context, email string) error return svc.mailService.SendEmail(resetEmail) } +func (svc service) DeleteUserByID(ctx context.Context, id uint) error { + return svc.ds.DeleteUserByID(id) +} + +func (svc service) DeleteUsers(ctx context.Context, ids []uint) (uint, error) { + return svc.ds.DeleteUsers(ids) +} + // saves user in datastore. // doesn't need to be exposed to the transport // the service should expose actions for modifying a user instead diff --git a/server/service/service_users_test.go b/server/service/service_users_test.go index 86b0aeee1..b358851b5 100644 --- a/server/service/service_users_test.go +++ b/server/service/service_users_test.go @@ -737,3 +737,54 @@ func TestUserPasswordRequirements(t *testing.T) { }) } } + +func TestDeleteUserByID(t *testing.T) { + ds, err := inmem.New(config.TestConfig()) + require.Nil(t, err) + createTestUsers(t, ds) + + svc, err := newTestService(ds, nil) + assert.Nil(t, err) + + ctx := context.Background() + + users, err := ds.ListUsers(kolide.ListOptions{}) + originalUsersLength := len(users) + assert.NotZero(t, originalUsersLength) + + err = svc.DeleteUserByID(ctx, users[len(users)-1].ID) + assert.Nil(t, err) + + users, err = ds.ListUsers(kolide.ListOptions{}) + assert.Nil(t, err) + assert.Len(t, users, originalUsersLength-1) + +} + +func TestDeleteUsers(t *testing.T) { + ds, err := inmem.New(config.TestConfig()) + require.Nil(t, err) + createTestUsers(t, ds) + + svc, err := newTestService(ds, nil) + assert.Nil(t, err) + + ctx := context.Background() + + users, err := ds.ListUsers(kolide.ListOptions{}) + originalUsersLength := len(users) + assert.NotZero(t, originalUsersLength) + + usersToDelete := make([]uint, originalUsersLength) + for i, user := range users { + usersToDelete[i] = user.ID + } + + deleted, err := svc.DeleteUsers(ctx, usersToDelete) + assert.Nil(t, err) + assert.Equal(t, uint(originalUsersLength), deleted) + + users, err = ds.ListUsers(kolide.ListOptions{}) + assert.Nil(t, err) + assert.Len(t, users, 0) +} diff --git a/server/service/transport_users.go b/server/service/transport_users.go index 2e740d41f..1c692aacf 100644 --- a/server/service/transport_users.go +++ b/server/service/transport_users.go @@ -72,6 +72,24 @@ func decodeModifyUserRequest(ctx context.Context, r *http.Request) (interface{}, return req, nil } +func decodeDeleteUserByIDRequest(ctx context.Context, r *http.Request) (interface{}, error) { + id, err := idFromRequest(r, "id") + if err != nil { + return nil, err + } + var req deleteUserByIDRequest + req.ID = id + return req, nil +} + +func decodeDeleteUsersRequest(ctx context.Context, r *http.Request) (interface{}, error) { + var req deleteUsersRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return nil, err + } + return req, nil +} + func decodeChangePasswordRequest(ctx context.Context, r *http.Request) (interface{}, error) { var req changePasswordRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { diff --git a/server/service/transport_users_test.go b/server/service/transport_users_test.go index 6910651e3..9327cf630 100644 --- a/server/service/transport_users_test.go +++ b/server/service/transport_users_test.go @@ -120,3 +120,42 @@ func TestDecodeModifyUserRequest(t *testing.T) { request, ) } + +func TestDecodeDeleteUserByIDRequest(t *testing.T) { + router := mux.NewRouter() + router.HandleFunc("/api/v1/kolide/users/{id}", func(writer http.ResponseWriter, request *http.Request) { + r, err := decodeDeleteUserByIDRequest(context.Background(), request) + assert.Nil(t, err) + + params := r.(deleteUserByIDRequest) + assert.Equal(t, uint(1), params.ID) + }).Methods("DELETE") + + request := httptest.NewRequest("DELETE", "/api/v1/kolide/users/1", nil) + router.ServeHTTP( + httptest.NewRecorder(), + request, + ) +} + +func TestDecodeDeleteUsersRequest(t *testing.T) { + router := mux.NewRouter() + router.HandleFunc("/api/v1/kolide/users/delete", func(writer http.ResponseWriter, request *http.Request) { + r, err := decodeDeleteUsersRequest(context.Background(), request) + assert.Nil(t, err) + + params := r.(deleteUsersRequest) + assert.Equal(t, []uint{1, 2, 3, 4}, params.IDs) + }).Methods("POST") + + var body bytes.Buffer + body.Write([]byte(`{ + "IDs": [1, 2, 3, 4] + }`)) + + request := httptest.NewRequest("POST", "/api/v1/kolide/users/delete", &body) + router.ServeHTTP( + httptest.NewRecorder(), + request, + ) +}