Fix IDOR Takeover in PUT /api/v1/user (#5986)

Co-authored-by: christopherholland-workday <christopher.holland+evisort@workday.com>
Co-authored-by: yau-wd <yau.ong@workday.com>

Author: 3 people

packages/server/src/enterprise/controllers/user.controller.ts

@@ -56,11 +56,11 @@ export class UserController {
             if (!currentUser) {
                 throw new InternalFlowiseError(StatusCodes.UNAUTHORIZED, UserErrorMessage.USER_NOT_FOUND)
             }
-            const { id } = req.body
+            const { id, name, oldPassword, newPassword, confirmPassword } = req.body
             if (currentUser.id !== id) {
                 throw new InternalFlowiseError(StatusCodes.FORBIDDEN, UserErrorMessage.USER_NOT_FOUND)
             }
-            const user = await userService.updateUser(req.body)
+            const user = await userService.updateUser({ id, name, updatedBy: currentUser.id, oldPassword, newPassword, confirmPassword })
             return res.status(StatusCodes.OK).json(user)
         } catch (error) {
             next(error)

packages/server/src/enterprise/services/user.service.ts

@@ -150,16 +150,10 @@ export class UserService {
                 if (!updateUserData) throw new InternalFlowiseError(StatusCodes.NOT_FOUND, UserErrorMessage.USER_NOT_FOUND)
             }
 
-            newUserData.createdBy = oldUserData.createdBy
-
             if (newUserData.name) {
                 this.validateUserName(newUserData.name)
             }
 
-            if (newUserData.status) {
-                this.validateUserStatus(newUserData.status)
-            }
-
             if (newUserData.oldPassword && newUserData.newPassword && newUserData.confirmPassword) {
                 if (!oldUserData.credential) {
                     throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, UserErrorMessage.INVALID_USER_CREDENTIAL)
@@ -176,7 +170,23 @@ export class UserService {
                 newUserData.tokenExpiry = undefined
             }
 
-            updatedUser = queryRunner.manager.merge(User, oldUserData, newUserData)
+            const safePatch: Partial<User> = {
+                createdBy: oldUserData.createdBy // always preserve from DB
+            }
+
+            if (newUserData.name) {
+                safePatch.name = newUserData.name
+            }
+
+            safePatch.updatedBy = newUserData.updatedBy // always set (controller forces req.user.id)
+            if (newUserData.oldPassword && newUserData.newPassword && newUserData.confirmPassword) {
+                // credential/tempToken/tokenExpiry were set by the validated workflow above
+                safePatch.credential = newUserData.credential
+                safePatch.tempToken = newUserData.tempToken
+                safePatch.tokenExpiry = newUserData.tokenExpiry
+            }
+
+            updatedUser = queryRunner.manager.merge(User, oldUserData, safePatch)
             await queryRunner.startTransaction()
             await this.saveUser(updatedUser, queryRunner)
             await queryRunner.commitTransaction()