fix(api): change user password update flow (#2247)

* fix(api): change password update flow

* feat(update-password): add current password confirmation

api/http/handler/users/handler.go

@@ -26,7 +26,7 @@ type Handler struct {
 }
 
 // NewHandler creates a handler to manage user operations.
-func NewHandler(bouncer *security.RequestBouncer) *Handler {
+func NewHandler(bouncer *security.RequestBouncer, rateLimiter *security.RateLimiter) *Handler {
 	h := &Handler{
 		Router: mux.NewRouter(),
 	}
@@ -43,7 +43,7 @@ func NewHandler(bouncer *security.RequestBouncer) *Handler {
 	h.Handle("/users/{id}/memberships",
 		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.userMemberships))).Methods(http.MethodGet)
 	h.Handle("/users/{id}/passwd",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.userPassword))).Methods(http.MethodPost)
+		rateLimiter.LimitAccess(bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.userUpdatePassword)))).Methods(http.MethodPut)
 	h.Handle("/users/admin/check",
 		bouncer.PublicAccess(httperror.LoggerHandler(h.adminCheck))).Methods(http.MethodGet)
 	h.Handle("/users/admin/init",

api/http/handler/users/user_password.go

@@ -1,57 +0,0 @@
-package users
-
-import (
-	"net/http"
-
-	"github.com/asaskevich/govalidator"
-	"github.com/portainer/portainer"
-	httperror "github.com/portainer/portainer/http/error"
-	"github.com/portainer/portainer/http/request"
-	"github.com/portainer/portainer/http/response"
-)
-
-type userPasswordPayload struct {
-	Password string
-}
-
-func (payload *userPasswordPayload) Validate(r *http.Request) error {
-	if govalidator.IsNull(payload.Password) {
-		return portainer.Error("Invalid password")
-	}
-	return nil
-}
-
-type userPasswordResponse struct {
-	Valid bool `json:"valid"`
-}
-
-// POST request on /api/users/:id/passwd
-func (handler *Handler) userPassword(w http.ResponseWriter, r *http.Request) *httperror.HandlerError {
-	userID, err := request.RetrieveNumericRouteVariableValue(r, "id")
-	if err != nil {
-		return &httperror.HandlerError{http.StatusBadRequest, "Invalid user identifier route variable", err}
-	}
-
-	var payload userPasswordPayload
-	err = request.DecodeAndValidateJSONPayload(r, &payload)
-	if err != nil {
-		return &httperror.HandlerError{http.StatusBadRequest, "Invalid request payload", err}
-	}
-
-	var password = payload.Password
-
-	u, err := handler.UserService.User(portainer.UserID(userID))
-	if err == portainer.ErrObjectNotFound {
-		return &httperror.HandlerError{http.StatusNotFound, "Unable to find a user with the specified identifier inside the database", err}
-	} else if err != nil {
-		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a user with the specified identifier inside the database", err}
-	}
-
-	valid := true
-	err = handler.CryptoService.CompareHashAndData(u.Password, password)
-	if err != nil {
-		valid = false
-	}
-
-	return response.JSON(w, &userPasswordResponse{Valid: valid})
-}

api/http/handler/users/user_update_password.go

@@ -0,0 +1,74 @@
+package users
+
+import (
+	"net/http"
+
+	"github.com/asaskevich/govalidator"
+	"github.com/portainer/portainer"
+	httperror "github.com/portainer/portainer/http/error"
+	"github.com/portainer/portainer/http/request"
+	"github.com/portainer/portainer/http/response"
+	"github.com/portainer/portainer/http/security"
+)
+
+type userUpdatePasswordPayload struct {
+	Password    string
+	NewPassword string
+}
+
+func (payload *userUpdatePasswordPayload) Validate(r *http.Request) error {
+	if govalidator.IsNull(payload.Password) {
+		return portainer.Error("Invalid current password")
+	}
+	if govalidator.IsNull(payload.NewPassword) {
+		return portainer.Error("Invalid new password")
+	}
+	return nil
+}
+
+// PUT request on /api/users/:id/passwd
+func (handler *Handler) userUpdatePassword(w http.ResponseWriter, r *http.Request) *httperror.HandlerError {
+	userID, err := request.RetrieveNumericRouteVariableValue(r, "id")
+	if err != nil {
+		return &httperror.HandlerError{http.StatusBadRequest, "Invalid user identifier route variable", err}
+	}
+
+	tokenData, err := security.RetrieveTokenData(r)
+	if err != nil {
+		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to retrieve user authentication token", err}
+	}
+
+	if tokenData.Role != portainer.AdministratorRole && tokenData.ID != portainer.UserID(userID) {
+		return &httperror.HandlerError{http.StatusForbidden, "Permission denied to update user", portainer.ErrUnauthorized}
+	}
+
+	var payload userUpdatePasswordPayload
+	err = request.DecodeAndValidateJSONPayload(r, &payload)
+	if err != nil {
+		return &httperror.HandlerError{http.StatusBadRequest, "Invalid request payload", err}
+	}
+
+	user, err := handler.UserService.User(portainer.UserID(userID))
+	if err == portainer.ErrObjectNotFound {
+		return &httperror.HandlerError{http.StatusNotFound, "Unable to find a user with the specified identifier inside the database", err}
+	} else if err != nil {
+		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a user with the specified identifier inside the database", err}
+	}
+
+	err = handler.CryptoService.CompareHashAndData(user.Password, payload.Password)
+	if err != nil {
+		return &httperror.HandlerError{http.StatusForbidden, "Specified password do not match actual password", portainer.ErrUnauthorized}
+	}
+
+	user.Password, err = handler.CryptoService.Hash(payload.NewPassword)
+	if err != nil {
+		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to hash user password", portainer.ErrCryptoHashFailure}
+	}
+
+	err = handler.UserService.UpdateUser(user.ID, user)
+	if err != nil {
+		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to persist user changes inside the database", err}
+	}
+
+	return response.Empty(w)
+}

api/http/server.go

@@ -163,7 +163,7 @@ func (server *Server) Start() error {
 	var uploadHandler = upload.NewHandler(requestBouncer)
 	uploadHandler.FileService = server.FileService
 
-	var userHandler = users.NewHandler(requestBouncer)
+	var userHandler = users.NewHandler(requestBouncer, rateLimiter)
 	userHandler.UserService = server.UserService
 	userHandler.TeamService = server.TeamService
 	userHandler.TeamMembershipService = server.TeamMembershipService

app/portainer/__module.js

@@ -327,9 +327,6 @@ angular.module('portainer.app', [])
         controller: 'UpdatePasswordController'
       },
       'sidebar@': {}
-    },
-    params: {
-      password: ''
     }
   };
 

app/portainer/rest/user.js

@@ -6,10 +6,9 @@ angular.module('portainer.app')
     query: { method: 'GET', isArray: true },
     get: { method: 'GET', params: { id: '@id' } },
     update: { method: 'PUT', params: { id: '@id' }, ignoreLoadingBar: true },
+    updatePassword: { method: 'PUT', params: { id: '@id', entity: 'passwd' } },
     remove: { method: 'DELETE', params: { id: '@id'} },
     queryMemberships: { method: 'GET', isArray: true, params: { id: '@id', entity: 'memberships' } },
-    // RPCs should be moved to a specific endpoint
-    checkPassword: { method: 'POST', params: { id: '@id', entity: 'passwd' }, ignoreLoadingBar: true },
     checkAdminUser: { method: 'GET', params: { id: 'admin', entity: 'check' }, isArray: true, ignoreLoadingBar: true },
     initAdminUser: { method: 'POST', params: { id: 'admin', entity: 'init' }, ignoreLoadingBar: true }
   });

app/portainer/services/api/userService.js

@@ -73,24 +73,12 @@ angular.module('portainer.app')
   };
 
   service.updateUserPassword = function(id, currentPassword, newPassword) {
-    var deferred = $q.defer();
+    var payload = {
+      Password: currentPassword,
+      NewPassword: newPassword
+    };
 
-    Users.checkPassword({id: id}, {password: currentPassword}).$promise
+    return Users.updatePassword({ id: id }, payload).$promise;
-    .then(function success(data) {
-      if (!data.valid) {
-        deferred.reject({invalidPassword: true});
-      } else {
-        return service.updateUser(id, newPassword, undefined);
-      }
-    })
-    .then(function success() {
-      deferred.resolve();
-    })
-    .catch(function error(err) {
-      deferred.reject({msg: 'Unable to update user password', err: err});
-    });
-
-    return deferred.promise;
   };
 
   service.userMemberships = function(id) {

app/portainer/views/account/account.html

@@ -20,12 +20,6 @@
             </div>
           </div>
           <!-- !current-password-input -->
-          <div class="form-group" ng-if="invalidPassword">
-            <div class="col-sm-12">
-              <i class="fa fa-times red-icon" aria-hidden="true"></i>
-              <span class="small text-muted">Current password is not valid</span>
-            </div>
-          </div>
           <!-- new-password-input -->
           <div class="form-group">
             <label for="new_password" class="col-sm-2 control-label text-left">New password</label>

app/portainer/views/account/accountController.js

@@ -8,19 +8,13 @@ function ($scope, $state, Authentication, UserService, Notifications, SettingsSe
   };
 
   $scope.updatePassword = function() {
-    $scope.invalidPassword = false;
-
     UserService.updateUserPassword($scope.userID, $scope.formValues.currentPassword, $scope.formValues.newPassword)
     .then(function success() {
       Notifications.success('Success', 'Password successfully updated');
       $state.reload();
     })
     .catch(function error(err) {
-      if (err.invalidPassword) {
+      Notifications.error('Failure', err, err.msg);
-        $scope.invalidPassword = true;
-      } else {
-        Notifications.error('Failure', err, err.msg);
-      }
     });
   };
 

app/portainer/views/auth/authController.js

@@ -30,7 +30,7 @@ function ($q, $scope, $state, $transition$, $sanitize, Authentication, UserServi
         return $q.reject();
       })
       .then(function success() {
-        $state.go('portainer.updatePassword', { password: $sanitize(password) });
+        $state.go('portainer.updatePassword');
       })
       .catch(function error() {
         $scope.state.AuthenticationError = 'Invalid credentials';

app/portainer/views/update-password/updatePassword.html

@@ -15,11 +15,19 @@
             </div>
           </div>
           <!-- !note -->
+          <!-- current-password-input -->
+          <div class="form-group">
+            <label for="current_password" class="col-sm-4 control-label text-left">Current password</label>
+            <div class="col-sm-8">
+              <input type="password" class="form-control" ng-model="formValues.CurrentPassword" id="current_password" auto-focus required>
+            </div>
+          </div>
+          <!-- !current-password-input -->
           <!-- new-password-input -->
           <div class="form-group">
             <label for="password" class="col-sm-4 control-label text-left">Password</label>
             <div class="col-sm-8">
-              <input type="password" class="form-control" ng-model="formValues.Password" id="password" auto-focus required>
+              <input type="password" class="form-control" ng-model="formValues.Password" id="password" required>
             </div>
           </div>
           <!-- !new-password-input -->

app/portainer/views/update-password/updatePasswordController.js

@@ -1,22 +1,22 @@
 angular.module('portainer.app')
-.controller('UpdatePasswordController', ['$scope', '$state', '$transition$', 'UserService', 'Authentication', 'Notifications',
+.controller('UpdatePasswordController', ['$scope', '$state', '$transition$', '$sanitize', 'UserService', 'Authentication', 'Notifications',
-function UpdatePasswordController($scope, $state, $transition$, UserService, Authentication, Notifications) {
+function UpdatePasswordController($scope, $state, $transition$, $sanitize, UserService, Authentication, Notifications) {
 
   $scope.formValues = {
+    CurrentPassword: '',
     Password: '',
     ConfirmPassword: ''
   };
 
   $scope.state = {
-    actionInProgress: false,
+    actionInProgress: false
-    currentPassword: ''
   };
 
   $scope.updatePassword = function() {
     var userId = Authentication.getUserDetails().ID;
 
     $scope.state.actionInProgress = true;
-    UserService.updateUserPassword(userId, $scope.state.currentPassword, $scope.formValues.Password)
+    UserService.updateUserPassword(userId, $sanitize($scope.formValues.CurrentPassword), $scope.formValues.Password)
     .then(function success() {
       $state.go('portainer.home');
     })
@@ -32,8 +32,6 @@ function UpdatePasswordController($scope, $state, $transition$, UserService, Aut
     if (!Authentication.isAuthenticated()) {
       $state.go('portainer.auth');
     }
-
-    $scope.state.currentPassword = $transition$.params().password;
   }
 
   initView();