Fix #2021: Allow users to be deleted

* Adds API endpoints to allow deleting users. Following patterns in the
rest of the API, to delete one user, use `DELETE
/api/v1/kolide/users/{id}" and to delete multiple users, `POST` an array
of ids to `/api/v1/kolide/users/delete`.

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

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

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

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)

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

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

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",

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