From 3c016314e6e4752b825d9c3e3fb04d4148ce3c71 Mon Sep 17 00:00:00 2001 From: mikiher Date: Wed, 15 Jul 2026 20:34:02 +0300 Subject: [PATCH] fix(auth): prevent admin users from deleting the root account --- server/controllers/UserController.js | 9 +- server/models/User.js | 9 +- .../server/controllers/UserController.test.js | 177 ++++++++++++++++++ 3 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 test/server/controllers/UserController.test.js diff --git a/server/controllers/UserController.js b/server/controllers/UserController.js index 3ec10539e..c0cbfc747 100644 --- a/server/controllers/UserController.js +++ b/server/controllers/UserController.js @@ -363,7 +363,13 @@ class UserController { * @param {Response} res */ async delete(req, res) { - if (req.params.id === 'root') { + const user = req.reqUser + + if (user.isRoot && !req.user.isRoot) { + Logger.error(`[UserController] Admin user "${req.user.username}" attempted to delete root user`) + return res.sendStatus(403) + } + if (user.isRoot) { Logger.error('[UserController] Attempt to delete root user. Root user cannot be deleted') return res.sendStatus(400) } @@ -371,7 +377,6 @@ class UserController { Logger.error(`[UserController] User ${req.user.username} is attempting to delete self`) return res.sendStatus(400) } - const user = req.reqUser // Todo: check if user is logged in and cancel streams diff --git a/server/models/User.js b/server/models/User.js index f380f8e4f..0b9d49438 100644 --- a/server/models/User.js +++ b/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') + } + } + } } ) } diff --git a/test/server/controllers/UserController.test.js b/test/server/controllers/UserController.test.js new file mode 100644 index 000000000..629788d84 --- /dev/null +++ b/test/server/controllers/UserController.test.js @@ -0,0 +1,177 @@ +const { expect } = require('chai') +const { Sequelize } = require('sequelize') +const sinon = require('sinon') + +const Database = require('../../../server/Database') +const UserController = require('../../../server/controllers/UserController') +const Logger = require('../../../server/Logger') +const SocketAuthority = require('../../../server/SocketAuthority') + +function createFakeRes() { + return { + sendStatus: sinon.spy(), + status: sinon.stub().returnsThis(), + send: sinon.spy(), + json: sinon.spy() + } +} + +describe('UserController - delete root protection', () => { + let rootUser + let adminUser + let regularUser + + beforeEach(async () => { + global.ServerSettings = {} + Database.sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + Database.sequelize.uppercaseFirst = (str) => (str ? `${str[0].toUpperCase()}${str.substr(1)}` : '') + await Database.buildModels() + + sinon.stub(Logger, 'info') + sinon.stub(Logger, 'error') + sinon.stub(SocketAuthority, 'adminEmitter') + + rootUser = await Database.userModel.create({ + username: 'root', + pash: 'hashed_password_root', + type: 'root', + isActive: true + }) + + adminUser = await Database.userModel.create({ + username: 'admin', + pash: 'hashed_password_admin', + type: 'admin', + isActive: true + }) + + regularUser = await Database.userModel.create({ + username: 'regular', + pash: 'hashed_password_regular', + type: 'user', + isActive: true + }) + }) + + afterEach(async () => { + sinon.restore() + await Database.sequelize.sync({ force: true }) + }) + + it('should prevent admin from deleting root user by UUID (403)', async () => { + const fakeReq = { + user: adminUser, + reqUser: rootUser, + params: { id: rootUser.id } + } + const fakeRes = createFakeRes() + + await UserController.delete(fakeReq, fakeRes) + + expect(fakeRes.sendStatus.calledWith(403)).to.be.true + expect(fakeRes.json.called).to.be.false + + const existingRoot = await Database.userModel.findByPk(rootUser.id) + expect(existingRoot).to.not.be.null + expect(existingRoot.type).to.equal('root') + }) + + it('should prevent root from deleting root user (400)', async () => { + const fakeReq = { + user: rootUser, + reqUser: rootUser, + params: { id: rootUser.id } + } + const fakeRes = createFakeRes() + + await UserController.delete(fakeReq, fakeRes) + + expect(fakeRes.sendStatus.calledWith(400)).to.be.true + expect(fakeRes.json.called).to.be.false + + const existingRoot = await Database.userModel.findByPk(rootUser.id) + expect(existingRoot).to.not.be.null + }) + + it('should not block deletion when URL param is literal "root" but target is a different user', async () => { + const fakeReq = { + user: adminUser, + reqUser: regularUser, + params: { id: 'root' } + } + const fakeRes = createFakeRes() + + await UserController.delete(fakeReq, fakeRes) + + expect(fakeRes.json.calledWith({ success: true })).to.be.true + + const deletedUser = await Database.userModel.findByPk(regularUser.id) + expect(deletedUser).to.be.null + }) + + it('should allow admin to delete a regular user (200)', async () => { + const fakeReq = { + user: adminUser, + reqUser: regularUser, + params: { id: regularUser.id } + } + const fakeRes = createFakeRes() + + await UserController.delete(fakeReq, fakeRes) + + expect(fakeRes.json.calledWith({ success: true })).to.be.true + expect(SocketAuthority.adminEmitter.calledWith('user_removed')).to.be.true + + const deletedUser = await Database.userModel.findByPk(regularUser.id) + expect(deletedUser).to.be.null + }) + + it('should prevent admin from deleting self (400)', async () => { + const fakeReq = { + user: adminUser, + reqUser: adminUser, + params: { id: adminUser.id } + } + const fakeRes = createFakeRes() + + await UserController.delete(fakeReq, fakeRes) + + expect(fakeRes.sendStatus.calledWith(400)).to.be.true + expect(fakeRes.json.called).to.be.false + + const existingAdmin = await Database.userModel.findByPk(adminUser.id) + expect(existingAdmin).to.not.be.null + }) +}) + +describe('User model - beforeDestroy root protection', () => { + beforeEach(async () => { + global.ServerSettings = {} + Database.sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + Database.sequelize.uppercaseFirst = (str) => (str ? `${str[0].toUpperCase()}${str.substr(1)}` : '') + await Database.buildModels() + }) + + afterEach(async () => { + await Database.sequelize.sync({ force: true }) + }) + + it('should reject direct destroy of root user', async () => { + const rootUser = await Database.userModel.create({ + username: 'root', + pash: 'hashed_password_root', + type: 'root', + isActive: true + }) + + try { + await rootUser.destroy() + expect.fail('Expected destroy to throw') + } catch (error) { + expect(error.message).to.equal('Root user cannot be deleted') + } + + const existingRoot = await Database.userModel.findByPk(rootUser.id) + expect(existingRoot).to.not.be.null + }) +})