From 88ea0cb64fc85c57967f2a6e65342518ce5812f1 Mon Sep 17 00:00:00 2001 From: Matt Hook Date: Wed, 6 Sep 2023 07:53:31 +1200 Subject: [PATCH] non-admins must supply existing passwd when changing passwd (#10248) --- api/http/handler/users/handler.go | 1 + api/http/handler/users/user_update.go | 24 +++++++++++++++---- .../handler/users/user_update_password.go | 2 +- app/portainer/services/api/userService.js | 4 ++-- .../views/users/edit/userController.js | 2 +- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/api/http/handler/users/handler.go b/api/http/handler/users/handler.go index 4491fee72..833c51dbc 100644 --- a/api/http/handler/users/handler.go +++ b/api/http/handler/users/handler.go @@ -22,6 +22,7 @@ var ( errAdminCannotRemoveSelf = errors.New("Cannot remove your own user account. Contact another administrator") errCannotRemoveLastLocalAdmin = errors.New("Cannot remove the last local administrator account") errCryptoHashFailure = errors.New("Unable to hash data") + errWrongPassword = errors.New("Wrong password") ) func hideFields(user *portainer.User) { diff --git a/api/http/handler/users/user_update.go b/api/http/handler/users/user_update.go index 8c58003f6..fdaba7b21 100644 --- a/api/http/handler/users/user_update.go +++ b/api/http/handler/users/user_update.go @@ -21,9 +21,10 @@ type themePayload struct { } type userUpdatePayload struct { - Username string `validate:"required" example:"bob"` - Password string `validate:"required" example:"cg9Wgky3"` - Theme *themePayload + Username string `validate:"required" example:"bob"` + Password string `validate:"required" example:"cg9Wgky3"` + NewPassword string `validate:"required" example:"asfj2emv"` + Theme *themePayload // User role (1 for administrator account and 2 for regular account) Role int `validate:"required" enums:"1,2" example:"2"` @@ -37,6 +38,7 @@ func (payload *userUpdatePayload) Validate(r *http.Request) error { if payload.Role != 0 && payload.Role != 1 && payload.Role != 2 { return errors.New("invalid role value. Value must be one of: 1 (administrator) or 2 (regular user)") } + return nil } @@ -106,8 +108,20 @@ func (handler *Handler) userUpdate(w http.ResponseWriter, r *http.Request) *http user.Username = payload.Username } - if payload.Password != "" { - user.Password, err = handler.CryptoService.Hash(payload.Password) + if payload.NewPassword != "" { + // Non-admins need to supply the previous password + if tokenData.Role != portainer.AdministratorRole { + err := handler.CryptoService.CompareHashAndData(user.Password, payload.Password) + if err != nil { + return httperror.Forbidden("Current password doesn't match. Password left unchanged", errors.New("Current password does not match the password provided. Please try again")) + } + } + + if !handler.passwordStrengthChecker.Check(payload.NewPassword) { + return httperror.BadRequest("Password does not meet the minimum strength requirements", nil) + } + + user.Password, err = handler.CryptoService.Hash(payload.NewPassword) if err != nil { return httperror.InternalServerError("Unable to hash user password", errCryptoHashFailure) } diff --git a/api/http/handler/users/user_update_password.go b/api/http/handler/users/user_update_password.go index 5239f0534..22d9be631 100644 --- a/api/http/handler/users/user_update_password.go +++ b/api/http/handler/users/user_update_password.go @@ -87,7 +87,7 @@ func (handler *Handler) userUpdatePassword(w http.ResponseWriter, r *http.Reques } if !handler.passwordStrengthChecker.Check(payload.NewPassword) { - return httperror.BadRequest("Password does not meet the requirements", nil) + return httperror.BadRequest("Password does not meet the minimum strength requirements", nil) } user.Password, err = handler.CryptoService.Hash(payload.NewPassword) diff --git a/app/portainer/services/api/userService.js b/app/portainer/services/api/userService.js index efa798760..c5e945c5b 100644 --- a/app/portainer/services/api/userService.js +++ b/app/portainer/services/api/userService.js @@ -54,8 +54,8 @@ export function UserService($q, Users, TeamService, TeamMembershipService) { return Users.remove({ id: id }).$promise; }; - service.updateUser = function (id, { password, role, username }) { - return Users.update({ id }, { password, role, username }).$promise; + service.updateUser = function (id, { newPassword, role, username }) { + return Users.update({ id }, { newPassword, role, username }).$promise; }; service.updateUserPassword = function (id, currentPassword, newPassword) { diff --git a/app/portainer/views/users/edit/userController.js b/app/portainer/views/users/edit/userController.js index f334d2a1b..e27667c8d 100644 --- a/app/portainer/views/users/edit/userController.js +++ b/app/portainer/views/users/edit/userController.js @@ -72,7 +72,7 @@ angular.module('portainer.app').controller('UserController', [ if (!confirmed) { return; } - UserService.updateUser($scope.user.Id, { password: $scope.formValues.newPassword }) + UserService.updateUser($scope.user.Id, { newPassword: $scope.formValues.newPassword }) .then(function success() { Notifications.success('Success', 'Password successfully updated');