Merge pull request #5370 from mikiher/fix/root-user-delete-protection

Prevent admin users from deleting the root account

server/controllers/UserController.js

@@ -363,15 +363,16 @@ class UserController {
    * @param {Response} res
    */
   async delete(req, res) {
-    if (req.params.id === 'root') {
-      Logger.error('[UserController] Attempt to delete root user. Root user cannot be deleted')
-      return res.sendStatus(400)
-    }
+    const user = req.reqUser
+
     if (req.user.id === req.params.id) {
       Logger.error(`[UserController] User ${req.user.username} is attempting to delete self`)
       return res.sendStatus(400)
     }
-    const user = req.reqUser
+    if (user.isRoot) {
+      Logger.error(`[UserController] Admin user "${req.user.username}" attempted to delete root user`)
+      return res.sendStatus(403)
+    }
 
     // Todo: check if user is logged in and cancel streams
 

server/models/User.js

@@ -530,7 +530,14 @@ class User extends Model {
       },
       {
         sequelize,
-        modelName: 'user'
+        modelName: 'user',
+        hooks: {
+          beforeDestroy(user) {
+            if (user.type === 'root') {
+              throw new Error('Root user cannot be deleted')
+            }
+          }
+        }
       }
     )
   }

test/server/controllers/UserController.test.js

@@ -0,0 +1,26 @@
+const { expect } = require('chai')
+const sinon = require('sinon')
+
+const UserController = require('../../../server/controllers/UserController')
+const Logger = require('../../../server/Logger')
+
+describe('UserController - delete', () => {
+  beforeEach(() => {
+    sinon.stub(Logger, 'error')
+  })
+
+  afterEach(() => {
+    sinon.restore()
+  })
+
+  it('rejects deleting the root user by UUID', async () => {
+    const rootUser = { id: 'root-uuid', isRoot: true, username: 'root' }
+    const adminUser = { id: 'admin-uuid', isRoot: false, username: 'admin' }
+    const fakeRes = { sendStatus: sinon.spy(), json: sinon.spy() }
+
+    await UserController.delete({ user: adminUser, reqUser: rootUser, params: { id: rootUser.id } }, fakeRes)
+
+    expect(fakeRes.sendStatus.calledWith(403)).to.be.true
+    expect(fakeRes.json.called).to.be.false
+  })
+})