From 5308fd8b46b7aa251b3c759cb726dae1de52ce14 Mon Sep 17 00:00:00 2001 From: advplyr Date: Sat, 17 Aug 2024 17:18:40 -0500 Subject: [PATCH] Update:Create & update API endpoints to create with new data model --- client/components/modals/AccountModal.vue | 2 + client/components/tables/UsersTable.vue | 2 +- server/Auth.js | 8 +- server/Database.js | 13 +- server/controllers/UserController.js | 196 ++++++++++++++++++---- server/models/User.js | 33 +--- 6 files changed, 184 insertions(+), 70 deletions(-) diff --git a/client/components/modals/AccountModal.vue b/client/components/modals/AccountModal.vue index d58ee913..b22ff245 100644 --- a/client/components/modals/AccountModal.vue +++ b/client/components/modals/AccountModal.vue @@ -351,6 +351,7 @@ export default { update: type === 'admin', delete: type === 'admin', upload: type === 'admin', + accessExplicitContent: true, accessAllLibraries: true, accessAllTags: true, selectedTagsNotAccessible: false @@ -385,6 +386,7 @@ export default { upload: false, accessAllLibraries: true, accessAllTags: true, + accessExplicitContent: true, selectedTagsNotAccessible: false }, librariesAccessible: [], diff --git a/client/components/tables/UsersTable.vue b/client/components/tables/UsersTable.vue index 43a84e5d..92fa684e 100644 --- a/client/components/tables/UsersTable.vue +++ b/client/components/tables/UsersTable.vue @@ -44,7 +44,7 @@
-
+
diff --git a/server/Auth.js b/server/Auth.js index 9ce61272..60af2a1e 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -154,7 +154,7 @@ class Auth { * Finds an existing user by OpenID subject identifier, or by email/username based on server settings, * or creates a new user if configured to do so. * - * @returns {import('./models/User')|null} + * @returns {Promise} */ async findOrCreateUser(userinfo) { let user = await Database.userModel.getUserByOpenIDSub(userinfo.sub) @@ -257,7 +257,7 @@ class Auth { /** * Sets the user group based on group claim in userinfo. * - * @param {import('./objects/user/User')} user + * @param {import('./models/User')} user * @param {Object} userinfo */ async setUserGroup(user, userinfo) { @@ -286,7 +286,7 @@ class Auth { if (user.type !== userType) { Logger.info(`[Auth] openid callback: Updating user "${user.username}" type to "${userType}" from "${user.type}"`) user.type = userType - await Database.userModel.updateFromOld(user) + await user.save() } } else { throw new Error(`No valid group found in userinfo: ${JSON.stringify(userinfo[groupClaimName], null, 2)}`) @@ -296,7 +296,7 @@ class Auth { /** * Updates user permissions based on the advanced permissions claim. * - * @param {import('./objects/user/User')} user + * @param {import('./models/User')} user * @param {Object} userinfo */ async updateUserPermissions(user, userinfo) { diff --git a/server/Database.js b/server/Database.js index 4d55c728..25056fdd 100644 --- a/server/Database.js +++ b/server/Database.js @@ -359,7 +359,7 @@ class Database { * @param {string} username * @param {string} pash * @param {Auth} auth - * @returns {boolean} true if created + * @returns {Promise} true if created */ async createRootUser(username, pash, auth) { if (!this.sequelize) return false @@ -379,17 +379,6 @@ class Database { return this.models.setting.updateSettingObj(settings.toJSON()) } - async createUser(oldUser) { - if (!this.sequelize) return false - await this.models.user.createFromOld(oldUser) - return true - } - - updateUser(oldUser) { - if (!this.sequelize) return false - return this.models.user.updateFromOld(oldUser) - } - updateBulkBooks(oldBooks) { if (!this.sequelize) return false return Promise.all(oldBooks.map((oldBook) => this.models.book.saveFromOld(oldBook))) diff --git a/server/controllers/UserController.js b/server/controllers/UserController.js index 37caa61c..f0848a05 100644 --- a/server/controllers/UserController.js +++ b/server/controllers/UserController.js @@ -116,27 +116,79 @@ class UserController { * @param {Response} res */ async create(req, res) { - const account = req.body - const username = account.username + if (!req.body.username || !req.body.password || typeof req.body.username !== 'string' || typeof req.body.password !== 'string') { + return res.status(400).send('Username and password are required') + } + if (req.body.type && !Database.userModel.accountTypes.includes(req.body.type)) { + return res.status(400).send('Invalid account type') + } - const usernameExists = await Database.userModel.checkUserExistsWithUsername(username) + const usernameExists = await Database.userModel.checkUserExistsWithUsername(req.body.username) if (usernameExists) { return res.status(400).send('Username already taken') } - account.id = uuidv4() - account.pash = await this.auth.hashPass(account.password) - delete account.password - account.token = await this.auth.generateAccessToken(account) - account.createdAt = Date.now() - const newUser = new User(account) + const userId = uuidv4() + const pash = await this.auth.hashPass(req.body.password) + const token = await this.auth.generateAccessToken({ id: userId, username: req.body.username }) + const userType = req.body.type || 'user' - // TODO: Create with new User model - const success = await Database.createUser(newUser) - if (success) { - SocketAuthority.adminEmitter('user_added', newUser.toJSONForBrowser()) + // librariesAccessible and itemTagsSelected can be on req.body or req.body.permissions + // Old model stored them outside of permissions, new model stores them inside permissions + let reqLibrariesAccessible = req.body.librariesAccessible || req.body.permissions?.librariesAccessible + if (reqLibrariesAccessible && (!Array.isArray(reqLibrariesAccessible) || reqLibrariesAccessible.some((libId) => typeof libId !== 'string'))) { + Logger.warn(`[UserController] create: Invalid librariesAccessible value: ${reqLibrariesAccessible}`) + reqLibrariesAccessible = null + } + let reqItemTagsSelected = req.body.itemTagsSelected || req.body.permissions?.itemTagsSelected + if (reqItemTagsSelected && (!Array.isArray(reqItemTagsSelected) || reqItemTagsSelected.some((tagId) => typeof tagId !== 'string'))) { + Logger.warn(`[UserController] create: Invalid itemTagsSelected value: ${reqItemTagsSelected}`) + reqItemTagsSelected = null + } + if (req.body.permissions?.itemTagsSelected || req.body.permissions?.librariesAccessible) { + delete req.body.permissions.itemTagsSelected + delete req.body.permissions.librariesAccessible + } + + // Map permissions + const permissions = Database.userModel.getDefaultPermissionsForUserType(userType) + if (req.body.permissions && typeof req.body.permissions === 'object') { + for (const key in req.body.permissions) { + if (permissions[key] !== undefined) { + if (typeof req.body.permissions[key] !== 'boolean') { + Logger.warn(`[UserController] create: Invalid permission value for key ${key}. Should be boolean`) + } else { + permissions[key] = req.body.permissions[key] + } + } else { + Logger.warn(`[UserController] create: Invalid permission key: ${key}`) + } + } + } + + permissions.itemTagsSelected = reqItemTagsSelected || [] + permissions.librariesAccessible = reqLibrariesAccessible || [] + + const newUser = { + id: userId, + type: userType, + username: req.body.username, + email: typeof req.body.email === 'string' ? req.body.email : null, + pash, + token, + isActive: !!req.body.isActive, + permissions, + bookmarks: [], + extraData: { + seriesHideFromContinueListening: [] + } + } + + const user = await Database.userModel.create(newUser) + if (user) { + SocketAuthority.adminEmitter('user_added', user.toOldJSONForBrowser()) res.json({ - user: newUser.toJSONForBrowser() + user: user.toOldJSONForBrowser() }) } else { return res.status(500).send('Failed to save new user') @@ -161,37 +213,125 @@ class UserController { } const updatePayload = req.body - let shouldUpdateToken = false + // Validate payload + const keysThatCannotBeUpdated = ['id', 'pash', 'token', 'extraData', 'bookmarks'] + for (const key of keysThatCannotBeUpdated) { + if (updatePayload[key] !== undefined) { + return res.status(400).send(`Key "${key}" cannot be updated`) + } + } + if (updatePayload.email && typeof updatePayload.email !== 'string') { + return res.status(400).send('Invalid email') + } + if (updatePayload.username && typeof updatePayload.username !== 'string') { + return res.status(400).send('Invalid username') + } + if (updatePayload.type && !Database.userModel.accountTypes.includes(updatePayload.type)) { + return res.status(400).send('Invalid account type') + } + if (updatePayload.permissions && typeof updatePayload.permissions !== 'object') { + return res.status(400).send('Invalid permissions') + } + + let hasUpdates = false + let shouldUpdateToken = false // When changing username create a new API token - if (updatePayload.username !== undefined && updatePayload.username !== user.username) { + if (updatePayload.username && updatePayload.username !== user.username) { const usernameExists = await Database.userModel.checkUserExistsWithUsername(updatePayload.username) if (usernameExists) { - return res.status(500).send('Username already taken') + return res.status(400).send('Username already taken') } + user.username = updatePayload.username shouldUpdateToken = true + hasUpdates = true } // Updating password if (updatePayload.password) { - updatePayload.pash = await this.auth.hashPass(updatePayload.password) - delete updatePayload.password + user.pash = await this.auth.hashPass(updatePayload.password) + hasUpdates = true } - // TODO: Update with new User model - const oldUser = Database.userModel.getOldUser(user) - if (oldUser.update(updatePayload)) { - if (shouldUpdateToken) { - oldUser.token = await this.auth.generateAccessToken(oldUser) - Logger.info(`[UserController] User ${oldUser.username} has generated a new api token`) + let hasPermissionsUpdates = false + let updateLibrariesAccessible = updatePayload.librariesAccessible || updatePayload.permissions?.librariesAccessible + if (updateLibrariesAccessible && (!Array.isArray(updateLibrariesAccessible) || updateLibrariesAccessible.some((libId) => typeof libId !== 'string'))) { + Logger.warn(`[UserController] update: Invalid librariesAccessible value: ${updateLibrariesAccessible}`) + updateLibrariesAccessible = null + } + let updateItemTagsSelected = updatePayload.itemTagsSelected || updatePayload.permissions?.itemTagsSelected + if (updateItemTagsSelected && (!Array.isArray(updateItemTagsSelected) || updateItemTagsSelected.some((tagId) => typeof tagId !== 'string'))) { + Logger.warn(`[UserController] update: Invalid itemTagsSelected value: ${updateItemTagsSelected}`) + updateItemTagsSelected = null + } + if (updatePayload.permissions?.itemTagsSelected || updatePayload.permissions?.librariesAccessible) { + delete updatePayload.permissions.itemTagsSelected + delete updatePayload.permissions.librariesAccessible + } + if (updatePayload.permissions && typeof updatePayload.permissions === 'object') { + const permissions = { + ...user.permissions } - await Database.updateUser(oldUser) - SocketAuthority.clientEmitter(req.user.id, 'user_updated', oldUser.toJSONForBrowser()) + for (const key in updatePayload.permissions) { + if (permissions[key] !== undefined) { + if (typeof updatePayload.permissions[key] !== 'boolean') { + Logger.warn(`[UserController] update: Invalid permission value for key ${key}. Should be boolean`) + } else if (permissions[key] !== updatePayload.permissions[key]) { + permissions[key] = updatePayload.permissions[key] + hasPermissionsUpdates = true + } + } else { + Logger.warn(`[UserController] update: Invalid permission key: ${key}`) + } + } + + if (updateItemTagsSelected && updateItemTagsSelected.join(',') !== user.permissions.itemTagsSelected.join(',')) { + permissions.itemTagsSelected = updateItemTagsSelected + hasPermissionsUpdates = true + } + if (updateLibrariesAccessible && updateLibrariesAccessible.join(',') !== user.permissions.librariesAccessible.join(',')) { + permissions.librariesAccessible = updateLibrariesAccessible + hasPermissionsUpdates = true + } + updatePayload.permissions = permissions + } + + // Permissions were updated + if (hasPermissionsUpdates) { + user.permissions = updatePayload.permissions + user.changed('permissions', true) + hasUpdates = true + } + + if (updatePayload.email && updatePayload.email !== user.email) { + user.email = updatePayload.email + hasUpdates = true + } + if (updatePayload.type && updatePayload.type !== user.type) { + user.type = updatePayload.type + hasUpdates = true + } + if (updatePayload.isActive !== undefined && !!updatePayload.isActive !== user.isActive) { + user.isActive = updatePayload.isActive + hasUpdates = true + } + if (updatePayload.lastSeen && typeof updatePayload.lastSeen === 'number') { + user.lastSeen = updatePayload.lastSeen + hasUpdates = true + } + + if (hasUpdates) { + if (shouldUpdateToken) { + user.token = await this.auth.generateAccessToken(user) + Logger.info(`[UserController] User ${user.username} has generated a new api token`) + } + await user.save() + SocketAuthority.clientEmitter(req.user.id, 'user_updated', user.toOldJSONForBrowser()) } res.json({ success: true, - user: oldUser.toJSONForBrowser() + user: user.toOldJSONForBrowser() }) } diff --git a/server/models/User.js b/server/models/User.js index 04f04e2b..f4b3da7a 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -51,6 +51,9 @@ class User extends Model { this.mediaProgresses } + // Excludes "root" since their can only be 1 root user + static accountTypes = ['admin', 'user', 'guest'] + /** * List of expected permission properties from the client * Only used for OpenID @@ -112,7 +115,9 @@ class User extends Model { } /** + * @deprecated * Get old user model from new + * TODO: Currently only used in dbMigration, replace * * @param {User} userExpanded * @returns {oldUser} @@ -151,26 +156,16 @@ class User extends Model { } /** - * - * @param {oldUser} oldUser - * @returns {Promise} - */ - static createFromOld(oldUser) { - const user = this.getFromOld(oldUser) - return this.create(user) - } - - /** + * @deprecated * Update User from old user model + * TODO: Currently only used in dbMigration, replace * * @param {oldUser} oldUser - * @param {boolean} [hooks=true] Run before / after bulk update hooks? * @returns {Promise} */ - static updateFromOld(oldUser, hooks = true) { + static updateFromOld(oldUser) { const user = this.getFromOld(oldUser) return this.update(user, { - hooks: !!hooks, where: { id: user.id } @@ -346,18 +341,6 @@ class User extends Model { }) } - /** - * @deprecated - * Get old user by id - * @param {string} userId - * @returns {Promise} returns null if not found - */ - static async getOldUserById(userId) { - const user = await this.getUserById(userId) - if (!user) return null - return this.getOldUser(user) - } - /** * Get user by openid sub * @param {string} sub