feat: remove all root bypasses

initial migration to usergroup-system for root user is governed by
superadminAccount and superadminForcePromote fields in config file

those will have default values expected for non-fork installs, but
existing fork installs will not have them and thus will never trigger
superadmin force-promotion from migration script
This commit is contained in:
Bobby 2023-02-25 18:16:11 +07:00
parent 807f153324
commit 729883a4f5
No known key found for this signature in database
GPG Key ID: 941839794CBF5A09
5 changed files with 50 additions and 37 deletions

View File

@ -20,6 +20,16 @@ module.exports = {
*/
enableUserAccounts: true,
/*
Be advised that this section is only relevant to the migration script.
This serves as a compatibility layer to non-fork installs,
as it will try to force promote the chosen user to superadmin usergroup,
since base lolisafe v3 do not have usergroups system.
It is recommended to set "superadminForcePromote" to any falsy value for existing installs.
*/
superadminAccount: 'root',
superadminForcePromote: true,
/*
Here you can decide if you want lolisafe to serve the files or if you prefer doing so via nginx.
The main difference between the two is the ease of use and the chance of analytics in the future.

View File

@ -168,9 +168,9 @@ self.register = async (req, res) => {
throw new ClientError(`Username must have ${self.user.min}-${self.user.max} characters.`)
}
// Please be advised that root user is hard-coded to always have superadmin permission
// However, you may choose to delete the root user via direct database query,
// so it is also hard-coded to always prevent it from being re-created via the API
// "root" user is not required if you have other users with superadmin permission,
// so removing them via direct database query is possible.
// However, protect it from being re-created via the API endpoints anyway.
if (username === 'root') {
throw new ClientError('Username is reserved.')
}
@ -236,9 +236,7 @@ self.changePassword = async (req, res) => {
}
self.assertPermission = (user, target) => {
if (target.username === 'root') {
throw new ClientError('User "root" may not be tampered with.', { statusCode: 403 })
} else if (!perms.higher(user, target)) {
if (!perms.higher(user, target)) {
throw new ClientError('The user is in the same or higher group as you.', { statusCode: 403 })
}
}
@ -256,6 +254,7 @@ self.createUser = async (req, res) => {
throw new ClientError(`Username must have ${self.user.min}-${self.user.max} characters.`)
}
// Please consult notes in self.register() function.
if (username === 'root') {
throw new ClientError('Username is reserved.')
}

View File

@ -14,10 +14,6 @@ self.permissions = Object.freeze({
self.keys = Object.freeze(Object.keys(self.permissions))
self.group = user => {
// root bypass
if (user.username === 'root') {
return 'superadmin'
}
for (const key of self.keys) {
if (user.permission === self.permissions[key]) {
return key
@ -28,10 +24,6 @@ self.group = user => {
// returns true if user is in the group OR higher
self.is = (user, group) => {
// root bypass
if (user.username === 'root') {
return true
}
if (typeof group !== 'string' || !group) {
return false
}

View File

@ -567,10 +567,11 @@ self.unlinkFile = async filename => {
}
}
self.bulkDeleteFromDb = async (field, values, user) => {
// Always return an empty array on failure
if (!user || !['id', 'name'].includes(field) || !values.length) {
return []
self.bulkDeleteFromDb = async (field, values = [], user, permissionBypass = false) => {
// NOTE: permissionBypass should not be set unless used by lolisafe's automated service.
if ((!user && !permissionBypass) || !['id', 'name'].includes(field) || !values.length) {
return values
}
// SQLITE_LIMIT_VARIABLE_NUMBER, which defaults to 999
@ -582,7 +583,7 @@ self.bulkDeleteFromDb = async (field, values, user) => {
}
const failed = []
const ismoderator = perms.is(user, 'moderator')
const ismoderator = permissionBypass || perms.is(user, 'moderator')
try {
const unlinkeds = []
@ -717,7 +718,6 @@ self.bulkDeleteExpired = async (dryrun, verbose) => {
const timestamp = Date.now() / 1000
const fields = ['id']
if (verbose) fields.push('name')
const sudo = { username: 'root' }
const result = {}
result.expired = await self.db.table('files')
@ -728,7 +728,8 @@ self.bulkDeleteExpired = async (dryrun, verbose) => {
// Make a shallow copy
const field = fields[0]
const values = result.expired.slice().map(row => row[field])
result.failed = await self.bulkDeleteFromDb(field, values, sudo)
// NOTE: 4th parameter set to true to bypass permission check
result.failed = await self.bulkDeleteFromDb(field, values, null, true)
if (verbose && result.failed.length) {
result.failed = result.failed
.map(failed => result.expired.find(file => file[fields[0]] === failed))

View File

@ -46,23 +46,34 @@ const map = {
}
}
const root = await db.table('users')
.where('username', 'root')
.select('permission')
.first()
if (root && root.permission !== perms.permissions.superadmin) {
await db.table('users')
.where('username', 'root')
if (config.superadminForcePromote) {
if (!config.superadminAccount) {
console.log('Attempting to promote superadmin user, but "superadminAccount" field is missing from config.')
process.exit(0)
}
const superadminAccount = await db.table('users')
.where('username', config.superadminAccount)
.select('permission')
.first()
.update({
permission: perms.permissions.superadmin
})
.then(result => {
// NOTE: permissionController.js actually has a hard-coded check for "root" account so that
// it will always have "superadmin" permission regardless of its permission value in database
console.log(`Updated root's permission to ${perms.permissions.superadmin} (superadmin).`)
done++
})
if (!superadminAccount) {
console.log(`Superadmin account "${config.superadminAccount}" is not found in database.`)
process.exit(0)
}
if (superadminAccount.permission !== perms.permissions.superadmin) {
await db.table('users')
.where('username', config.superadminAccount)
.first()
.update({
permission: perms.permissions.superadmin
})
.then(result => {
console.log(`Updated "${config.superadminAccount}"'s permission to ${perms.permissions.superadmin} (superadmin).`)
done++
})
}
}
const filesOutdatedSize = await db.table('files')