diff --git a/src/rest-server/src/controllers/v2/group.js b/src/rest-server/src/controllers/v2/group.js index 81acfe7f71d..7b46bcbce4b 100644 --- a/src/rest-server/src/controllers/v2/group.js +++ b/src/rest-server/src/controllers/v2/group.js @@ -51,15 +51,6 @@ const getAllGroup = async (req, res, next) => { const getGroupUserList = async (req, res, next) => { try { - if (!req.user.admin) { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } const groupname = req.params.groupname; const allUserInfoList = await userModel.getAllUser(); const userlist = []; @@ -79,15 +70,6 @@ const getGroupUserList = async (req, res, next) => { const createGroup = async (req, res, next) => { try { - if (!req.user.admin) { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } const groupname = req.body.groupname; const groupValue = { groupname: req.body.groupname, @@ -107,42 +89,30 @@ const createGroup = async (req, res, next) => { const updateGroup = async (req, res, next) => { const groupname = req.body.data.groupname; try { - if (req.user.admin) { - const groupInfo = await groupModel.getGroup(groupname); - if (req.body.patch) { - if ('description' in req.body.data) { - groupInfo.description = req.body.data.description; - } - if ('externalName' in req.body.data) { - groupInfo.externalName = req.body.data.externalName; - } - if ('extension' in req.body.data) { - if (Object.keys(req.body.data.extension).length > 0) { - for (const [key, value] of Object.entries( - req.body.data.extension, - )) { - groupInfo.extension[key] = value; - } - } - } - } else { + const groupInfo = await groupModel.getGroup(groupname); + if (req.body.patch) { + if ('description' in req.body.data) { groupInfo.description = req.body.data.description; + } + if ('externalName' in req.body.data) { groupInfo.externalName = req.body.data.externalName; - groupInfo.extension = req.body.data.extension; } - await groupModel.updateGroup(groupname, groupInfo); - return res.status(201).json({ - message: `update group ${groupname} successfully.`, - }); + if ('extension' in req.body.data) { + if (Object.keys(req.body.data.extension).length > 0) { + for (const [key, value] of Object.entries(req.body.data.extension)) { + groupInfo.extension[key] = value; + } + } + } } else { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); + groupInfo.description = req.body.data.description; + groupInfo.externalName = req.body.data.externalName; + groupInfo.extension = req.body.data.extension; } + await groupModel.updateGroup(groupname, groupInfo); + return res.status(201).json({ + message: `update group ${groupname} successfully.`, + }); } catch (error) { if (error.status === 404) { return next( @@ -160,20 +130,10 @@ const updateGroup = async (req, res, next) => { const deleteGroup = async (req, res, next) => { try { const groupname = req.params.groupname; - if (req.user.admin) { - await groupModel.deleteGroup(groupname); - return res.status(200).json({ - message: 'group is removed successfully', - }); - } else { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + await groupModel.deleteGroup(groupname); + return res.status(200).json({ + message: 'group is removed successfully', + }); } catch (error) { return next(createError.unknown(error)); } @@ -184,24 +144,14 @@ const updateGroupExtension = async (req, res, next) => { try { const groupname = req.params.groupname; const extensionData = req.body.extension; - if (req.user.admin) { - const groupInfo = await groupModel.getGroup(groupname); - for (const [key, value] of Object.entries(extensionData)) { - groupInfo.extension[key] = value; - } - await groupModel.updateGroup(groupname, groupInfo); - return res.status(201).json({ - message: 'update group extension data successfully.', - }); - } else { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); + const groupInfo = await groupModel.getGroup(groupname); + for (const [key, value] of Object.entries(extensionData)) { + groupInfo.extension[key] = value; } + await groupModel.updateGroup(groupname, groupInfo); + return res.status(201).json({ + message: 'update group extension data successfully.', + }); } catch (error) { return next(createError.unknown(error)); } @@ -213,26 +163,16 @@ const updateGroupExtensionAttr = async (req, res, next) => { const groupname = req.params.groupname; const attrs = req.params[0].split('/'); const updateData = req.body.data; - if (req.user.admin) { - const groupInfo = await groupModel.getGroup(groupname); - groupInfo.extension = common.assignValueByKeyarray( - groupInfo.extension, - attrs, - updateData, - ); - await groupModel.updateGroup(groupname, groupInfo); - return res.status(201).json({ - message: 'Update group extension data successfully.', - }); - } else { - return next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + const groupInfo = await groupModel.getGroup(groupname); + groupInfo.extension = common.assignValueByKeyarray( + groupInfo.extension, + attrs, + updateData, + ); + await groupModel.updateGroup(groupname, groupInfo); + return res.status(201).json({ + message: 'Update group extension data successfully.', + }); } catch (error) { if (error.status === 404) { return next( @@ -252,22 +192,12 @@ const updateGroupDescription = async (req, res, next) => { try { const groupname = req.params.groupname; const descriptionData = req.body.description; - if (req.user.admin) { - const groupInfo = await groupModel.getGroup(groupname); - groupInfo.description = descriptionData; - await groupModel.updateGroup(groupname, groupInfo); - return res.status(201).json({ - message: 'update group description data successfully.', - }); - } else { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + const groupInfo = await groupModel.getGroup(groupname); + groupInfo.description = descriptionData; + await groupModel.updateGroup(groupname, groupInfo); + return res.status(201).json({ + message: 'update group description data successfully.', + }); } catch (error) { return next(createError.unknown(error)); } @@ -278,22 +208,12 @@ const updateGroupExternalName = async (req, res, next) => { try { const groupname = req.params.groupname; const externalNameData = req.body.externalName; - if (req.user.admin) { - const groupInfo = await groupModel.getGroup(groupname); - groupInfo.externalName = externalNameData; - await groupModel.updateGroup(groupname, groupInfo); - return res.status(201).json({ - message: 'update group externalNameData data successfully.', - }); - } else { - next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + const groupInfo = await groupModel.getGroup(groupname); + groupInfo.externalName = externalNameData; + await groupModel.updateGroup(groupname, groupInfo); + return res.status(201).json({ + message: 'update group externalNameData data successfully.', + }); } catch (error) { return next(createError.unknown(error)); } diff --git a/src/rest-server/src/controllers/v2/job.js b/src/rest-server/src/controllers/v2/job.js index 2af246cf561..7accc3b8c96 100644 --- a/src/rest-server/src/controllers/v2/job.js +++ b/src/rest-server/src/controllers/v2/job.js @@ -222,14 +222,6 @@ const getSshInfo = asyncHandler(async (req, res) => { }); const addTag = asyncHandler(async (req, res) => { - // only admin users can add tags - if (!req.user.admin) { - throw createError( - 'Unauthorized', - 'UnauthorizedUserError', - 'Only admin users are allowed to do this operation.', - ); - } await job.addTag(req.params.frameworkName, req.body.value); res.status(status('OK')).json({ status: status('OK'), @@ -238,14 +230,6 @@ const addTag = asyncHandler(async (req, res) => { }); const deleteTag = asyncHandler(async (req, res) => { - // only admin users can delete tags - if (!req.user.admin) { - throw createError( - 'Unauthorized', - 'UnauthorizedUserError', - 'Only admin users are allowed to do this operation.', - ); - } await job.deleteTag(req.params.frameworkName, req.body.value); res.status(status('OK')).json({ status: status('OK'), diff --git a/src/rest-server/src/controllers/v2/user.js b/src/rest-server/src/controllers/v2/user.js index 48d4ad8bcb0..9d6ad6ec044 100644 --- a/src/rest-server/src/controllers/v2/user.js +++ b/src/rest-server/src/controllers/v2/user.js @@ -293,48 +293,38 @@ const updateVirtualClusterInternal = async (newVc) => { const updateUserVirtualCluster = async (req, res, next) => { try { const username = req.params.username; - if (req.user.admin) { - const newGroupList = await updateVirtualClusterInternal( - req.body.virtualCluster, - ); - let userInfo; - try { - userInfo = await userModel.getUser(username); - } catch (error) { - if (error.status === 404) { - return next( - createError( - 'Not Found', - 'NoUserError', - `User ${req.params.username} not found.`, - ), - ); - } - return next(createError.unknown(error)); - } - if (await userModel.checkAdmin(username)) { + const newGroupList = await updateVirtualClusterInternal( + req.body.virtualCluster, + ); + let userInfo; + try { + userInfo = await userModel.getUser(username); + } catch (error) { + if (error.status === 404) { return next( createError( - 'Forbidden', - 'ForbiddenUserError', - "Admin's virtual clusters cannot be updated.", + 'Not Found', + 'NoUserError', + `User ${req.params.username} not found.`, ), ); } - userInfo.grouplist = newGroupList; - await userModel.updateUser(username, userInfo); - return res.status(201).json({ - message: 'Update user virtualCluster data successfully.', - }); - } else { + return next(createError.unknown(error)); + } + if (await userModel.checkAdmin(username)) { return next( createError( 'Forbidden', 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, + "Admin's virtual clusters cannot be updated.", ), ); } + userInfo.grouplist = newGroupList; + await userModel.updateUser(username, userInfo); + return res.status(201).json({ + message: 'Update user virtualCluster data successfully.', + }); } catch (error) { if (error.code === 'NoVirtualClusterError') { return next(error); @@ -757,29 +747,19 @@ const oidcUserUpdate = async (req, res, next) => { const deleteUser = async (req, res, next) => { try { const username = req.params.username; - if (req.user.admin) { - if (await userModel.checkAdmin(username)) { - return next( - createError( - 'Forbidden', - 'RemoveAdminError', - `Admin ${username} is not allowed to remove.`, - ), - ); - } - await userModel.deleteUser(username); - return res.status(200).json({ - message: 'user is removed successfully', - }); - } else { - next( + if (await userModel.checkAdmin(username)) { + return next( createError( 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, + 'RemoveAdminError', + `Admin ${username} is not allowed to remove.`, ), ); } + await userModel.deleteUser(username); + return res.status(200).json({ + message: 'user is removed successfully', + }); } catch (error) { if (error.status === 404) { return next( diff --git a/src/rest-server/src/controllers/v2/virtual-cluster.js b/src/rest-server/src/controllers/v2/virtual-cluster.js index 216c57264b9..c96d70b98cf 100644 --- a/src/rest-server/src/controllers/v2/virtual-cluster.js +++ b/src/rest-server/src/controllers/v2/virtual-cluster.js @@ -58,13 +58,6 @@ const get = asyncHandler(async (req, res) => { const update = asyncHandler(async (req, res) => { const virtualClusterName = req.params.virtualClusterName; - if (!req.user.admin) { - throw createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allowed to do this operation.`, - ); - } if (virtualClusterName === authConfig.groupConfig.defaultGroup.groupname) { throw createError( 'Forbidden', @@ -130,44 +123,29 @@ const update = asyncHandler(async (req, res) => { const updateStatus = asyncHandler(async (req, res) => { const virtualClusterName = req.params.virtualClusterName; const vcStatus = req.body.vcStatus; - if (req.user.admin) { - if (vcStatus === 'stopped') { - await virtualCluster.stop(virtualClusterName); - res.status(status('Created')).json({ - status: status('Created'), - message: `Stop virtual cluster ${virtualClusterName} successfully.`, - }); - } else if (vcStatus === 'running') { - await virtualCluster.activate(virtualClusterName); - res.status(status('Created')).json({ - status: status('Created'), - message: `Activate virtual cluster ${virtualClusterName} successfully.`, - }); - } else { - throw createError( - 'Bad Request', - 'BadConfigurationError', - `Unknown vc status: ${vcStatus}`, - ); - } + if (vcStatus === 'stopped') { + await virtualCluster.stop(virtualClusterName); + res.status(status('Created')).json({ + status: status('Created'), + message: `Stop virtual cluster ${virtualClusterName} successfully.`, + }); + } else if (vcStatus === 'running') { + await virtualCluster.activate(virtualClusterName); + res.status(status('Created')).json({ + status: status('Created'), + message: `Activate virtual cluster ${virtualClusterName} successfully.`, + }); } else { throw createError( - 'Forbidden', - 'ForbiddenUserError', - 'Non-admin is not allowed to do this operation.', + 'Bad Request', + 'BadConfigurationError', + `Unknown vc status: ${vcStatus}`, ); } }); const remove = asyncHandler(async (req, res) => { const virtualClusterName = req.params.virtualClusterName; - if (!req.user.admin) { - throw createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allowed to do this operation.`, - ); - } if (virtualClusterName === authConfig.groupConfig.adminGroup.groupname) { throw createError( 'Forbidden', diff --git a/src/rest-server/src/middlewares/token.js b/src/rest-server/src/middlewares/token.js index b5f490389d3..ccbca2f3ddd 100644 --- a/src/rest-server/src/middlewares/token.js +++ b/src/rest-server/src/middlewares/token.js @@ -83,6 +83,7 @@ const notApplication = async (req, _, next) => { } }; +// this middleware should be used after `check` to ensure `req.user` contains `admin` as a key const checkAdmin = async (req, _, next) => { if (!req.user.admin) { return next( diff --git a/src/rest-server/src/routes/kubernetes.js b/src/rest-server/src/routes/kubernetes.js index d9686ece674..147839a6d0a 100644 --- a/src/rest-server/src/routes/kubernetes.js +++ b/src/rest-server/src/routes/kubernetes.js @@ -4,23 +4,13 @@ const express = require('express'); const token = require('@pai/middlewares/token'); const kubernetes = require('@pai/models/kubernetes/kubernetes'); -const createError = require('@pai/utils/error'); const router = new express.Router(); router .route('/nodes') /** GET /api/v1/kubernetes/nodes - Return k8s nodes info */ - .get(token.check, async (req, res, next) => { - if (!req.user.admin) { - return next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + .get(token.check, token.checkAdmin, async (req, res, next) => { try { const nodes = await kubernetes.getNodes(); res.status(200).json(nodes); @@ -32,16 +22,7 @@ router router .route('/pods') /** GET /api/v1/kubernetes/pods - Return k8s pods info */ - .get(token.check, async (req, res, next) => { - if (!req.user.admin) { - return next( - createError( - 'Forbidden', - 'ForbiddenUserError', - `Non-admin is not allow to do this operation.`, - ), - ); - } + .get(token.check, token.checkAdmin, async (req, res, next) => { try { const pods = await kubernetes.getPods(req.query); res.status(200).json(pods); diff --git a/src/rest-server/src/routes/v2/group.js b/src/rest-server/src/routes/v2/group.js index 817ce20aa99..dc508f00e2b 100644 --- a/src/rest-server/src/routes/v2/group.js +++ b/src/rest-server/src/routes/v2/group.js @@ -37,7 +37,11 @@ router router .route('/:groupname') /** Post /api/v2/group/:groupname */ - .delete(token.checkNotApplication, groupController.deleteGroup); + .delete( + token.checkNotApplication, + token.checkAdmin, + groupController.deleteGroup, + ); router .route('/') @@ -45,6 +49,7 @@ router .post( token.checkNotApplication, param.validate(groupInputSchema.groupCreateInputSchema), + token.checkAdmin, groupController.createGroup, ); @@ -54,6 +59,7 @@ router .put( token.checkNotApplication, param.validate(groupInputSchema.groupUpdateInputSchema), + token.checkAdmin, groupController.updateGroup, ); @@ -63,13 +69,14 @@ router .put( token.checkNotApplication, param.validate(groupInputSchema.groupExtensionAttrUpdateInputSchema), + token.checkAdmin, groupController.updateGroupExtensionAttr, ); router .route('/:groupname/userlist') /** get /api/v2/group/:groupname/userlist */ - .get(token.check, groupController.getGroupUserList); + .get(token.check, token.checkAdmin, groupController.getGroupUserList); /** Legacy API and will be deprecated in the future. Please use put /api/v2/group */ router @@ -77,6 +84,7 @@ router .put( token.checkNotApplication, param.validate(groupInputSchema.groupExtensionUpdateInputSchema), + token.checkAdmin, groupController.updateGroupExtension, ); @@ -86,6 +94,7 @@ router .put( token.checkNotApplication, param.validate(groupInputSchema.groupDescriptionUpdateInputSchema), + token.checkAdmin, groupController.updateGroupDescription, ); @@ -95,6 +104,7 @@ router .put( token.checkNotApplication, param.validate(groupInputSchema.groupExternalNameUpdateInputSchema), + token.checkAdmin, groupController.updateGroupExternalName, ); diff --git a/src/rest-server/src/routes/v2/job.js b/src/rest-server/src/routes/v2/job.js index 9498449ad9f..d08b988acbf 100644 --- a/src/rest-server/src/routes/v2/job.js +++ b/src/rest-server/src/routes/v2/job.js @@ -80,6 +80,7 @@ router .put( token.check, param.validate(tagInputSchema.tagInputSchema), + token.checkAdmin, controller.addTag, ); @@ -89,6 +90,7 @@ router .delete( token.check, param.validate(tagInputSchema.tagInputSchema), + token.checkAdmin, controller.deleteTag, ); diff --git a/src/rest-server/src/routes/v2/user.js b/src/rest-server/src/routes/v2/user.js index d560d1a7ff9..6aa2e298d54 100644 --- a/src/rest-server/src/routes/v2/user.js +++ b/src/rest-server/src/routes/v2/user.js @@ -49,7 +49,11 @@ if (authnConfig.authnMethod === 'basic') { router .route('/:username') /** Delete /api/v2/users/:username */ - .delete(token.checkNotApplication, userController.deleteUser); + .delete( + token.checkNotApplication, + token.checkAdmin, + userController.deleteUser, + ); router .route('/') @@ -118,6 +122,7 @@ if (authnConfig.authnMethod === 'basic') { .put( token.checkNotApplication, param.validate(userInputSchema.userVirtualClusterUpdateInputSchema), + token.checkAdmin, userController.updateUserVirtualCluster, ); diff --git a/src/rest-server/src/routes/v2/virtual-cluster.js b/src/rest-server/src/routes/v2/virtual-cluster.js index 56745dee9b2..d9e0947ee4b 100644 --- a/src/rest-server/src/routes/v2/virtual-cluster.js +++ b/src/rest-server/src/routes/v2/virtual-cluster.js @@ -37,10 +37,11 @@ router .put( token.checkNotApplication, param.validate(vcConfig.vcCreateInputSchema), + token.checkAdmin, controller.update, ) /** DELETE /api/v2/virtual-clusters/:virtualClusterName - Remove a virtual cluster */ - .delete(token.checkNotApplication, controller.remove); + .delete(token.checkNotApplication, token.checkAdmin, controller.remove); router .route('/:virtualClusterName/status') @@ -48,6 +49,7 @@ router .put( token.checkNotApplication, param.validate(vcConfig.vcStatusPutInputSchema), + token.checkAdmin, controller.updateStatus, );