Add change password endpoint (#628)

server/kolide/users.go

@@ -38,6 +38,10 @@ type UserService interface {
 	// Users 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

server/service/endpoint_users.go

@@ -101,6 +101,29 @@ func makeListUsersEndpoint(svc kolide.Service) endpoint.Endpoint {
 	}
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Change Password
+////////////////////////////////////////////////////////////////////////////////
+
+type changePasswordRequest struct {
+	OldPassword string `json:"old_password"`
+	NewPassword string `json:"new_password"`
+}
+
+type changePasswordResponse struct {
+	Err error `json:"error,omitempty"`
+}
+
+func (r changePasswordResponse) error() error { return r.Err }
+
+func makeChangePasswordEndpoint(svc kolide.Service) endpoint.Endpoint {
+	return func(ctx context.Context, request interface{}) (interface{}, error) {
+		req := request.(changePasswordRequest)
+		err := svc.ChangePassword(ctx, req.OldPassword, req.NewPassword)
+		return changePasswordResponse{Err: err}, nil
+	}
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Reset Password
 ////////////////////////////////////////////////////////////////////////////////

server/service/handler.go

@@ -19,6 +19,7 @@ type KolideEndpoints struct {
 	ForgotPassword                 endpoint.Endpoint
 	ResetPassword                  endpoint.Endpoint
 	Me                             endpoint.Endpoint
+	ChangePassword                 endpoint.Endpoint
 	CreateUser                     endpoint.Endpoint
 	GetUser                        endpoint.Endpoint
 	ListUsers                      endpoint.Endpoint
@@ -84,6 +85,7 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint
 		// 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))),
@@ -140,6 +142,7 @@ type kolideHandlers struct {
 	ForgotPassword                 *kithttp.Server
 	ResetPassword                  *kithttp.Server
 	Me                             *kithttp.Server
+	ChangePassword                 *kithttp.Server
 	CreateUser                     *kithttp.Server
 	GetUser                        *kithttp.Server
 	ListUsers                      *kithttp.Server
@@ -198,6 +201,7 @@ func makeKolideKitHandlers(ctx context.Context, e KolideEndpoints, opts []kithtt
 		ForgotPassword:                 newServer(e.ForgotPassword, decodeForgotPasswordRequest),
 		ResetPassword:                  newServer(e.ResetPassword, decodeResetPasswordRequest),
 		Me:                             newServer(e.Me, decodeNoParamsRequest),
+		ChangePassword:                 newServer(e.ChangePassword, decodeChangePasswordRequest),
 		CreateUser:                     newServer(e.CreateUser, decodeCreateUserRequest),
 		GetUser:                        newServer(e.GetUser, decodeGetUserRequest),
 		ListUsers:                      newServer(e.ListUsers, decodeListUsersRequest),
@@ -278,6 +282,7 @@ func attachKolideAPIRoutes(r *mux.Router, h kolideHandlers) {
 	r.Handle("/api/v1/kolide/forgot_password", h.ForgotPassword).Methods("POST")
 	r.Handle("/api/v1/kolide/reset_password", h.ResetPassword).Methods("POST")
 	r.Handle("/api/v1/kolide/me", h.Me).Methods("GET")
+	r.Handle("/api/v1/kolide/change_password", h.ChangePassword).Methods("POST")
 
 	r.Handle("/api/v1/kolide/users", h.ListUsers).Methods("GET")
 	r.Handle("/api/v1/kolide/users", h.CreateUser).Methods("POST")

server/service/logging_users.go

@@ -118,12 +118,35 @@ func (mw loggingMiddleware) AuthenticatedUser(ctx context.Context) (*kolide.User
 	return user, err
 }
 
+func (mw loggingMiddleware) ChangePassword(ctx context.Context, oldPass, newPass string) error {
+	var (
+		requestedBy = "unauthenticated"
+		err         error
+	)
+	vc, ok := viewer.FromContext(ctx)
+	if ok {
+		requestedBy = vc.Username()
+	}
+
+	defer func(begin time.Time) {
+		_ = mw.logger.Log(
+			"method", "ChangePassword",
+			"err", err,
+			"requested_by", requestedBy,
+			"took", time.Since(begin),
+		)
+	}(time.Now())
+
+	err = mw.Service.ChangePassword(ctx, oldPass, newPass)
+	return err
+}
+
 func (mw loggingMiddleware) ResetPassword(ctx context.Context, token, password string) error {
 	var err error
 
 	defer func(begin time.Time) {
 		_ = mw.logger.Log(
-			"method", "ChangePassword",
+			"method", "ResetPassword",
 			"err", err,
 			"took", time.Since(begin),
 		)

server/service/metrics_users.go

@@ -87,6 +87,19 @@ func (mw metricsMiddleware) AuthenticatedUser(ctx context.Context) (*kolide.User
 	return user, err
 }
 
+func (mw metricsMiddleware) ChangePassword(ctx context.Context, oldPass, newPass string) error {
+	var err error
+
+	defer func(begin time.Time) {
+		lvs := []string{"method", "ChangePassword", "error", fmt.Sprint(err != nil)}
+		mw.requestCount.With(lvs...).Add(1)
+		mw.requestLatency.With(lvs...).Observe(time.Since(begin).Seconds())
+	}(time.Now())
+
+	err = mw.Service.ChangePassword(ctx, oldPass, newPass)
+	return err
+}
+
 func (mw metricsMiddleware) ResetPassword(ctx context.Context, token, password string) error {
 	var err error
 

server/service/service_users.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/kolide/kolide-ose/server/contexts/viewer"
 	"github.com/kolide/kolide-ose/server/kolide"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -130,33 +131,60 @@ func (svc service) ListUsers(ctx context.Context, opt kolide.ListOptions) ([]*ko
 	return svc.ds.ListUsers(opt)
 }
 
+// setNewPassword is a helper for changing a user's password. It should be
+// called to set the new password after proper authorization has been
+// performed.
+func (svc service) setNewPassword(ctx context.Context, user *kolide.User, password string) error {
+	err := user.SetPassword(password, svc.config.Auth.SaltKeySize, svc.config.Auth.BcryptCost)
+	if err != nil {
+		return errors.Wrap(err, "setting new password")
+	}
+
+	err = svc.saveUser(user)
+	if err != nil {
+		return errors.Wrap(err, "saving changed password")
+	}
+
+	return nil
+}
+
+func (svc service) ChangePassword(ctx context.Context, oldPass, newPass string) error {
+	vc, ok := viewer.FromContext(ctx)
+	if !ok {
+		return errNoContext
+	}
+
+	if err := vc.User.ValidatePassword(oldPass); err != nil {
+		return errors.Wrap(err, "password validation failed")
+	}
+
+	return errors.Wrap(svc.setNewPassword(ctx, vc.User, newPass), "setting new password")
+}
+
 func (svc service) ResetPassword(ctx context.Context, token, password string) error {
 	reset, err := svc.ds.FindPassswordResetByToken(token)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "looking up reset by token")
 	}
 	user, err := svc.User(ctx, reset.UserID)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "retrieving user")
 	}
 
-	err = user.SetPassword(password, svc.config.Auth.SaltKeySize, svc.config.Auth.BcryptCost)
+	err = svc.setNewPassword(ctx, user, password)
 	if err != nil {
-		return err
-	}
-	if err := svc.saveUser(user); err != nil {
-		return err
+		return errors.Wrap(err, "setting new password")
 	}
 
 	// delete password reset tokens for user
 	if err := svc.ds.DeletePasswordResetRequestsForUser(user.ID); err != nil {
-		return err
+		return errors.Wrap(err, "deleting password reset requests")
 	}
 
 	// Clear sessions so that any other browsers will have to log in with
 	// the new password
 	if err := svc.DeleteSessionsForUser(ctx, user.ID); err != nil {
-		return err
+		return errors.Wrap(err, "deleting user sessions")
 	}
 
 	return nil

server/service/service_users_test.go

@@ -5,13 +5,14 @@ import (
 	"testing"
 	"time"
 
-	"github.com/WatchBeam/clock"
 	"github.com/kolide/kolide-ose/server/config"
 	"github.com/kolide/kolide-ose/server/contexts/viewer"
 	"github.com/kolide/kolide-ose/server/datastore/inmem"
 	kolide_errors "github.com/kolide/kolide-ose/server/errors"
 	"github.com/kolide/kolide-ose/server/kolide"
 
+	"github.com/WatchBeam/clock"
+	pkg_errors "github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
@@ -244,11 +245,71 @@ func setupInvites(t *testing.T, ds kolide.Datastore, emails []string) map[string
 	return invites
 }
 
-func TestChangeUserPassword(t *testing.T) {
+func TestChangePassword(t *testing.T) {
+	ds, _ := inmem.New(config.TestConfig())
+	svc, _ := newTestService(ds, nil)
+	users := createTestUsers(t, ds)
+	var passwordChangeTests = []struct {
+		user        kolide.User
+		oldPassword string
+		newPassword string
+		anyErr      bool
+		wantErr     error
+	}{
+		{ // all good
+			user:        users["admin1"],
+			oldPassword: "foobar",
+			newPassword: "123cat!",
+		},
+		{ // all good
+			user:        users["user1"],
+			oldPassword: "foobar",
+			newPassword: "newpass",
+		},
+		{ // bad old password
+			user:        users["user1"],
+			oldPassword: "wrong_password",
+			newPassword: "123cat!",
+			anyErr:      true,
+		},
+		{ // missing old password
+			newPassword: "123cat!",
+			wantErr:     &invalidArgumentError{invalidArgument{name: "old_password", reason: "cannot be empty field"}},
+		},
+		{ // missing new password
+			oldPassword: "abcd",
+			wantErr:     &invalidArgumentError{invalidArgument{name: "new_password", reason: "cannot be empty field"}},
+		},
+	}
+
+	for _, tt := range passwordChangeTests {
+		t.Run("", func(t *testing.T) {
+			ctx := context.Background()
+			ctx = viewer.NewContext(ctx, viewer.Viewer{User: &tt.user})
+
+			err := svc.ChangePassword(ctx, tt.oldPassword, tt.newPassword)
+			if tt.anyErr {
+				require.NotNil(t, err)
+			} else {
+				require.Equal(t, tt.wantErr, pkg_errors.Cause(err))
+			}
+
+			if err != nil {
+				return
+			}
+
+			// Attempt login after successful change
+			_, _, err = svc.Login(context.Background(), tt.user.Username, tt.newPassword)
+			require.Nil(t, err, "should be able to login with new password")
+		})
+	}
+}
+
+func TestResetPassword(t *testing.T) {
 	ds, _ := inmem.New(config.TestConfig())
 	svc, _ := newTestService(ds, nil)
 	createTestUsers(t, ds)
-	var passwordChangeTests = []struct {
+	var passwordResetTests = []struct {
 		token       string
 		newPassword string
 		wantErr     error
@@ -272,7 +333,7 @@ func TestChangeUserPassword(t *testing.T) {
 		},
 	}
 
-	for _, tt := range passwordChangeTests {
+	for _, tt := range passwordResetTests {
 		t.Run("", func(t *testing.T) {
 			ctx := context.Background()
 			request := &kolide.PasswordResetRequest{
@@ -292,7 +353,7 @@ func TestChangeUserPassword(t *testing.T) {
 			assert.Nil(t, err)
 
 			serr := svc.ResetPassword(ctx, tt.token, tt.newPassword)
-			assert.Equal(t, tt.wantErr, serr)
+			assert.Equal(t, tt.wantErr, pkg_errors.Cause(serr))
 		})
 	}
 }

server/service/transport_users.go

@@ -32,14 +32,6 @@ func decodeListUsersRequest(ctx context.Context, r *http.Request) (interface{},
 	return listUsersRequest{ListOptions: opt}, nil
 }
 
-func decodeChangePasswordRequest(ctx context.Context, r *http.Request) (interface{}, error) {
-	var req resetPasswordRequest
-	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		return nil, err
-	}
-	return req, nil
-}
-
 func decodeModifyUserRequest(ctx context.Context, r *http.Request) (interface{}, error) {
 	id, err := idFromRequest(r, "id")
 	if err != nil {
@@ -53,6 +45,14 @@ func decodeModifyUserRequest(ctx context.Context, r *http.Request) (interface{},
 	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 {
+		return nil, err
+	}
+	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 {

server/service/transport_users_test.go

@@ -2,13 +2,13 @@ package service
 
 import (
 	"bytes"
-	"golang.org/x/net/context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 )
 
 func TestDecodeCreateUserRequest(t *testing.T) {
@@ -52,10 +52,33 @@ func TestDecodeGetUserRequest(t *testing.T) {
 
 func TestDecodeChangePasswordRequest(t *testing.T) {
 	router := mux.NewRouter()
-	router.HandleFunc("/api/v1/kolide/users/{id}/password", func(writer http.ResponseWriter, request *http.Request) {
+	router.HandleFunc("/api/v1/kolide/change_password", func(writer http.ResponseWriter, request *http.Request) {
 		r, err := decodeChangePasswordRequest(context.Background(), request)
 		assert.Nil(t, err)
 
+		params := r.(changePasswordRequest)
+		assert.Equal(t, "foo", params.OldPassword)
+		assert.Equal(t, "bar", params.NewPassword)
+	}).Methods("POST")
+
+	var body bytes.Buffer
+	body.Write([]byte(`{
+        "old_password": "foo",
+        "new_password": "bar"
+    }`))
+
+	router.ServeHTTP(
+		httptest.NewRecorder(),
+		httptest.NewRequest("POST", "/api/v1/kolide/change_password", &body),
+	)
+}
+
+func TestDecodeResetPasswordRequest(t *testing.T) {
+	router := mux.NewRouter()
+	router.HandleFunc("/api/v1/kolide/users/{id}/password", func(writer http.ResponseWriter, request *http.Request) {
+		r, err := decodeResetPasswordRequest(context.Background(), request)
+		assert.Nil(t, err)
+
 		params := r.(resetPasswordRequest)
 		assert.Equal(t, "bar", params.NewPassword)
 		assert.Equal(t, "baz", params.PasswordResetToken)

server/service/validation_users.go

@@ -37,6 +37,20 @@ func (mw validationMiddleware) NewUser(ctx context.Context, p kolide.UserPayload
 	return mw.Service.NewUser(ctx, p)
 }
 
+func (mw validationMiddleware) ChangePassword(ctx context.Context, oldPass, newPass string) error {
+	invalid := &invalidArgumentError{}
+	if oldPass == "" {
+		invalid.Append("old_password", "cannot be empty field")
+	}
+	if newPass == "" {
+		invalid.Append("new_password", "cannot be empty field")
+	}
+	if invalid.HasErrors() {
+		return invalid
+	}
+	return mw.Service.ChangePassword(ctx, oldPass, newPass)
+}
+
 func (mw validationMiddleware) ResetPassword(ctx context.Context, token, password string) error {
 	invalid := &invalidArgumentError{}
 	if token == "" {