From ff9ee2f5a9037cebfa992c30a14086fa33fde0a5 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Tue, 28 Jan 2025 14:15:00 +0100 Subject: [PATCH] Use "can write or review project content" authorization middleware (#23111) GitOrigin-RevId: c5d1cb955e5833347f7e0c3610c5b8d768026478 --- .../Authorization/AuthorizationManager.js | 32 +++++----- .../Authorization/AuthorizationMiddleware.js | 58 +++++++++---------- .../src/Features/History/HistoryRouter.mjs | 4 +- .../AuthorizationManagerTests.js | 2 +- .../AuthorizationMiddlewareTests.js | 8 +-- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index 6d4f09ed56..2f339de83d 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -202,6 +202,19 @@ async function canUserWriteProjectContent(userId, projectId, token) { ) } +async function canUserWriteOrReviewProjectContent(userId, projectId, token) { + const privilegeLevel = await getPrivilegeLevelForProject( + userId, + projectId, + token + ) + return ( + privilegeLevel === PrivilegeLevels.OWNER || + privilegeLevel === PrivilegeLevels.READ_AND_WRITE || + privilegeLevel === PrivilegeLevels.REVIEW + ) +} + async function canUserWriteProjectSettings(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, @@ -273,23 +286,12 @@ async function canUserDeleteOrResolveThread( return comment.metadata.user_id === userId } -async function canUserSendOrReopenComment(userId, projectId, token) { - const privilegeLevel = await getPrivilegeLevelForProject( - userId, - projectId, - token - ) - return ( - privilegeLevel === PrivilegeLevels.OWNER || - privilegeLevel === PrivilegeLevels.READ_AND_WRITE || - privilegeLevel === PrivilegeLevels.REVIEW - ) -} - module.exports = { canUserReadProject: callbackify(canUserReadProject), canUserWriteProjectContent: callbackify(canUserWriteProjectContent), - canUserSendOrReopenComment: callbackify(canUserSendOrReopenComment), + canUserWriteOrReviewProjectContent: callbackify( + canUserWriteOrReviewProjectContent + ), canUserDeleteOrResolveThread: callbackify(canUserDeleteOrResolveThread), canUserWriteProjectSettings: callbackify(canUserWriteProjectSettings), canUserRenameProject: callbackify(canUserRenameProject), @@ -301,7 +303,7 @@ module.exports = { promises: { canUserReadProject, canUserWriteProjectContent, - canUserSendOrReopenComment, + canUserWriteOrReviewProjectContent, canUserDeleteOrResolveThread, canUserWriteProjectSettings, canUserRenameProject, diff --git a/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js b/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js index c26210572a..02043442c8 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js +++ b/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js @@ -132,32 +132,6 @@ async function ensureUserCanDeleteOrResolveThread(req, res, next) { return HttpErrorHandler.forbidden(req, res) } -async function ensureUserCanSendOrReopenComment(req, res, next) { - const projectId = _getProjectId(req) - const userId = _getUserId(req) - const token = TokenAccessHandler.getRequestToken(req, projectId) - - const canSendOrReopenComment = - await AuthorizationManager.promises.canUserSendOrReopenComment( - userId, - projectId, - token - ) - if (canSendOrReopenComment) { - logger.debug( - { userId, projectId }, - 'allowing user to send or reopen a comment' - ) - return next() - } - - logger.debug( - { userId, projectId }, - 'denying user to send or reopen a comment' - ) - return HttpErrorHandler.forbidden(req, res) -} - async function ensureUserCanWriteProjectContent(req, res, next) { const projectId = _getProjectId(req) const userId = _getUserId(req) @@ -182,6 +156,32 @@ async function ensureUserCanWriteProjectContent(req, res, next) { HttpErrorHandler.forbidden(req, res) } +async function ensureUserCanWriteOrReviewProjectContent(req, res, next) { + const projectId = _getProjectId(req) + const userId = _getUserId(req) + const token = TokenAccessHandler.getRequestToken(req, projectId) + + const canWriteOrReviewProjectContent = + await AuthorizationManager.promises.canUserWriteOrReviewProjectContent( + userId, + projectId, + token + ) + if (canWriteOrReviewProjectContent) { + logger.debug( + { userId, projectId }, + 'allowing user write or review access to project content' + ) + return next() + } + + logger.debug( + { userId, projectId }, + 'denying user write or review access to project content' + ) + return HttpErrorHandler.forbidden(req, res) +} + async function ensureUserCanAdminProject(req, res, next) { const projectId = _getProjectId(req) const userId = _getUserId(req) @@ -277,15 +277,15 @@ module.exports = { ensureUserCanWriteProjectSettings: expressify( ensureUserCanWriteProjectSettings ), - ensureUserCanSendOrReopenComment: expressify( - ensureUserCanSendOrReopenComment - ), ensureUserCanDeleteOrResolveThread: expressify( ensureUserCanDeleteOrResolveThread ), ensureUserCanWriteProjectContent: expressify( ensureUserCanWriteProjectContent ), + ensureUserCanWriteOrReviewProjectContent: expressify( + ensureUserCanWriteOrReviewProjectContent + ), ensureUserCanAdminProject: expressify(ensureUserCanAdminProject), ensureUserIsSiteAdmin: expressify(ensureUserIsSiteAdmin), restricted, diff --git a/services/web/app/src/Features/History/HistoryRouter.mjs b/services/web/app/src/Features/History/HistoryRouter.mjs index 3e532a2cad..5b37236166 100644 --- a/services/web/app/src/Features/History/HistoryRouter.mjs +++ b/services/web/app/src/Features/History/HistoryRouter.mjs @@ -138,12 +138,12 @@ function apply(webRouter, privateApiRouter) { ) webRouter.post( '/project/:Project_id/labels', - AuthorizationMiddleware.ensureUserCanWriteProjectContent, + AuthorizationMiddleware.ensureUserCanWriteOrReviewProjectContent, HistoryController.createLabel ) webRouter.delete( '/project/:Project_id/labels/:label_id', - AuthorizationMiddleware.ensureUserCanWriteProjectContent, + AuthorizationMiddleware.ensureUserCanWriteOrReviewProjectContent, HistoryController.deleteLabel ) diff --git a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js index c0ac9b1087..7463bbdeb7 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js @@ -453,7 +453,7 @@ describe('AuthorizationManager', function () { tokenReadOnly: true, }) - testPermission('canUserSendOrReopenComment', { + testPermission('canUserWriteOrReviewProjectContent', { siteAdmin: true, owner: true, readAndWrite: true, diff --git a/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js b/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js index 851b15d00b..33ff719e32 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js @@ -25,7 +25,7 @@ describe('AuthorizationMiddleware', function () { canUserReadProject: sinon.stub(), canUserWriteProjectSettings: sinon.stub(), canUserWriteProjectContent: sinon.stub(), - canUserSendOrReopenComment: sinon.stub(), + canUserWriteOrReviewProjectContent: sinon.stub(), canUserDeleteOrResolveThread: sinon.stub(), canUserAdminProject: sinon.stub(), canUserRenameProject: sinon.stub(), @@ -86,10 +86,10 @@ describe('AuthorizationMiddleware', function () { ) }) - describe('ensureUserCanSendOrReopenComment', function () { + describe('ensureUserCanWriteOrReviewProjectContent', function () { testMiddleware( - 'ensureUserCanSendOrReopenComment', - 'canUserSendOrReopenComment' + 'ensureUserCanWriteOrReviewProjectContent', + 'canUserWriteOrReviewProjectContent' ) })