From c79dc9f70f512dbec0e3460ee78dbc9feccfbbb1 Mon Sep 17 00:00:00 2001 From: TheCatLady <52870424+TheCatLady@users.noreply.github.com> Date: Sun, 31 Oct 2021 11:56:59 -0400 Subject: [PATCH] fix: add missing route guards to issues pages (#2235) * fix: users should always be able to view their own issues * fix: apply route guards to issues pages instead * fix(api): only allow users w/ issue perms to edit comments / delete issues --- server/routes/issue.ts | 58 +++++++++++++++------------- server/routes/request.ts | 19 ++++++++- src/pages/issues/[issueId]/index.tsx | 12 ++++++ src/pages/issues/index.tsx | 12 ++++++ 4 files changed, 74 insertions(+), 27 deletions(-) diff --git a/server/routes/issue.ts b/server/routes/issue.ts index 06f8ef9d9..b7faa2399 100644 --- a/server/routes/issue.ts +++ b/server/routes/issue.ts @@ -68,7 +68,7 @@ issueRoutes.get, IssueResultsResponse>( return next({ status: 403, message: - 'You do not have permission to view issues created by other users', + 'You do not have permission to view issues reported by other users', }); } query = query.andWhere('createdBy.id = :id', { id: req.user?.id }); @@ -291,35 +291,41 @@ issueRoutes.post<{ issueId: string; status: string }, Issue>( } ); -issueRoutes.delete('/:issueId', async (req, res, next) => { - const issueRepository = getRepository(Issue); - - try { - const issue = await issueRepository.findOneOrFail({ - where: { id: Number(req.params.issueId) }, - relations: ['createdBy'], - }); +issueRoutes.delete( + '/:issueId', + isAuthenticated([Permission.MANAGE_ISSUES, Permission.CREATE_ISSUES], { + type: 'or', + }), + async (req, res, next) => { + const issueRepository = getRepository(Issue); - if ( - !req.user?.hasPermission(Permission.MANAGE_ISSUES) && - (issue.createdBy.id !== req.user?.id || issue.comments.length > 1) - ) { - return next({ - status: 401, - message: 'You do not have permission to delete this issue.', + try { + const issue = await issueRepository.findOneOrFail({ + where: { id: Number(req.params.issueId) }, + relations: ['createdBy'], }); - } - await issueRepository.remove(issue); + if ( + !req.user?.hasPermission(Permission.MANAGE_ISSUES) && + (issue.createdBy.id !== req.user?.id || issue.comments.length > 1) + ) { + return next({ + status: 401, + message: 'You do not have permission to delete this issue.', + }); + } - return res.status(204).send(); - } catch (e) { - logger.error('Something went wrong deleting an issue.', { - label: 'API', - errorMessage: e.message, - }); - next({ status: 404, message: 'Issue not found.' }); + await issueRepository.remove(issue); + + return res.status(204).send(); + } catch (e) { + logger.error('Something went wrong deleting an issue.', { + label: 'API', + errorMessage: e.message, + }); + next({ status: 404, message: 'Issue not found.' }); + } } -}); +); export default issueRoutes; diff --git a/server/routes/request.ts b/server/routes/request.ts index 2eac90f00..d0a0e0233 100644 --- a/server/routes/request.ts +++ b/server/routes/request.ts @@ -500,9 +500,26 @@ requestRoutes.get('/:requestId', async (req, res, next) => { relations: ['requestedBy', 'modifiedBy'], }); + if ( + request.requestedBy.id !== req.user?.id && + !req.user?.hasPermission( + [Permission.MANAGE_REQUESTS, Permission.REQUEST_VIEW], + { type: 'or' } + ) + ) { + return next({ + status: 403, + message: 'You do not have permission to view this request.', + }); + } + return res.status(200).json(request); } catch (e) { - next({ status: 404, message: 'Request not found' }); + logger.debug('Failed to retrieve request.', { + label: 'API', + errorMessage: e.message, + }); + next({ status: 404, message: 'Request not found.' }); } }); diff --git a/src/pages/issues/[issueId]/index.tsx b/src/pages/issues/[issueId]/index.tsx index 730bee6e7..48aadebf7 100644 --- a/src/pages/issues/[issueId]/index.tsx +++ b/src/pages/issues/[issueId]/index.tsx @@ -1,8 +1,20 @@ import { NextPage } from 'next'; import React from 'react'; import IssueDetails from '../../../components/IssueDetails'; +import useRouteGuard from '../../../hooks/useRouteGuard'; +import { Permission } from '../../../hooks/useUser'; const IssuePage: NextPage = () => { + useRouteGuard( + [ + Permission.MANAGE_ISSUES, + Permission.CREATE_ISSUES, + Permission.VIEW_ISSUES, + ], + { + type: 'or', + } + ); return ; }; diff --git a/src/pages/issues/index.tsx b/src/pages/issues/index.tsx index 0168d888f..f352b89b0 100644 --- a/src/pages/issues/index.tsx +++ b/src/pages/issues/index.tsx @@ -1,8 +1,20 @@ import { NextPage } from 'next'; import React from 'react'; import IssueList from '../../components/IssueList'; +import useRouteGuard from '../../hooks/useRouteGuard'; +import { Permission } from '../../hooks/useUser'; const IssuePage: NextPage = () => { + useRouteGuard( + [ + Permission.MANAGE_ISSUES, + Permission.CREATE_ISSUES, + Permission.VIEW_ISSUES, + ], + { + type: 'or', + } + ); return ; };