From a1ad775a7a72f181dd44d2c73935e9c9c21e39fe Mon Sep 17 00:00:00 2001 From: Takeshi Kimata <117462761+kimatata@users.noreply.github.com> Date: Sat, 20 Jul 2024 08:23:31 +0900 Subject: [PATCH] feat: even project manager should not edit/delete project (#14) * fix: remove unused import * feat: even project manager should not edit/delete project * docs: even project manager should not edit/delete project --- backend/routes/projects/delete.js | 4 +- backend/routes/projects/edit.js | 4 +- docs/docs/usage/roles.md | 45 ++++++++----------- .../src/app/[locale]/PaneMainFeatures.tsx | 1 - frontend/src/app/[locale]/page.tsx | 2 +- .../[projectId]/settings/SettingsPage.tsx | 4 +- frontend/types/user.ts | 5 ++- frontend/utils/TokenProvider.tsx | 13 ++++++ frontend/utils/token.ts | 23 +++++++++- 9 files changed, 64 insertions(+), 37 deletions(-) diff --git a/backend/routes/projects/delete.js b/backend/routes/projects/delete.js index 806f610..47dadc6 100644 --- a/backend/routes/projects/delete.js +++ b/backend/routes/projects/delete.js @@ -7,12 +7,12 @@ const { DataTypes } = require('sequelize'); module.exports = function (sequelize) { const { verifySignedIn } = require('../../middleware/auth')(sequelize); - const { verifyProjectManagerFromProjectId } = require('../../middleware/verifyEditable')(sequelize); + const { verifyProjectOwner } = require('../../middleware/verifyEditable')(sequelize); const Project = defineProject(sequelize, DataTypes); const Folder = defineFolder(sequelize, DataTypes); const Run = defineRun(sequelize, DataTypes); - router.delete('/:projectId', verifySignedIn, verifyProjectManagerFromProjectId, async (req, res) => { + router.delete('/:projectId', verifySignedIn, verifyProjectOwner, async (req, res) => { const projectId = req.params.projectId; const t = await sequelize.transaction(); diff --git a/backend/routes/projects/edit.js b/backend/routes/projects/edit.js index 778f205..a90c65b 100644 --- a/backend/routes/projects/edit.js +++ b/backend/routes/projects/edit.js @@ -5,10 +5,10 @@ const { DataTypes } = require('sequelize'); module.exports = function (sequelize) { const { verifySignedIn } = require('../../middleware/auth')(sequelize); - const { verifyProjectManagerFromProjectId } = require('../../middleware/verifyEditable')(sequelize); + const { verifyProjectOwner } = require('../../middleware/verifyEditable')(sequelize); const Project = defineProject(sequelize, DataTypes); - router.put('/:projectId', verifySignedIn, verifyProjectManagerFromProjectId, async (req, res) => { + router.put('/:projectId', verifySignedIn, verifyProjectOwner, async (req, res) => { const projectId = req.params.projectId; const { name, detail, isPublic } = req.body; try { diff --git a/docs/docs/usage/roles.md b/docs/docs/usage/roles.md index 383ef6e..cf8b63a 100644 --- a/docs/docs/usage/roles.md +++ b/docs/docs/usage/roles.md @@ -29,39 +29,32 @@ There are three types of roles: #### Project -| Action | Manager/Owner | Developer | Reporter | Not member | -| ------ | ------------- | --------- | -------- | ---------- | -| Delete | ✅ | ❌ | ❌ | ❌ | -| Update | ✅ | ❌ | ❌ | ❌ | -| Read | ✅ | ✅ | ✅ | 🌓 | +| Action | Owner[^1] | Manager | Developer | Reporter | Not member[^2] | +| ------ | --------- | ------- | --------- | -------- | -------------- | +| Write | ✅ | ❌ | ❌ | ❌ | ❌ | +| Read | ✅ | ✅ | ✅ | ✅ | 🌓[^3] | #### Project Members -| Action | Manager/Owner | Developer | Reporter | Not member | -| ----------- | ------------- | --------- | -------- | ---------- | -| Add | ✅ | ❌ | ❌ | ❌ | -| Delete | ✅ | ❌ | ❌ | ❌ | -| Change role | ✅ | ❌ | ❌ | ❌ | -| Read | ✅ | ✅ | ✅ | 🌓 | +| Action | Owner | Manager | Developer | Reporter | Not member | +| ------ | ----- | ------- | --------- | -------- | ---------- | +| Write | ✅ | ✅ | ❌ | ❌ | ❌ | +| Read | ✅ | ✅ | ✅ | ✅ | 🌓 | #### Folders and Test cases -| Action | Manager/Owner | Developer | Reporter | Not member | -| ------ | ------------- | --------- | -------- | ---------- | -| Create | ✅ | ✅ | ❌ | ❌ | -| Delete | ✅ | ✅ | ❌ | ❌ | -| Update | ✅ | ✅ | ❌ | ❌ | -| Read | ✅ | ✅ | ✅ | 🌓 | +| Action | Owner | Owner | Developer | Reporter | Not member | +| ------ | ----- | ----- | --------- | -------- | ---------- | +| Write | ✅ | ✅ | ✅ | ❌ | ❌ | +| Read | ✅ | ✅ | ✅ | ✅ | 🌓 | #### Test runs -| Action | Manager/Owner | Developer | Reporter | Not member | -| ------ | ------------- | --------- | -------- | ---------- | -| Create | ✅ | ✅ | ✅ | ❌ | -| Delete | ✅ | ✅ | ✅ | ❌ | -| Update | ✅ | ✅ | ✅ | ❌ | -| Read | ✅ | ✅ | ✅ | 🌓 | +| Action | Owner | Manager | Developer | Reporter | Not member | +| ------ | ----- | ------- | --------- | -------- | ---------- | +| Write | ✅ | ✅ | ✅ | ✅ | ❌ | +| Read | ✅ | ✅ | ✅ | ✅ | 🌓 | -1. "Owner" and "Not member" are not role. "Owner" is the user who created project. - "Not member" means a user who is not a project member -1. 🌓 means that read permission is only allowed if the project is set to public. +[^1]: "Owner" is not role. "Owner" is the user who created project. +[^2]: "Not member" is not role. "Not member" means a user who is not a project member +[^3]: 🌓 means that read permission is only allowed if the project is set to public. diff --git a/frontend/src/app/[locale]/PaneMainFeatures.tsx b/frontend/src/app/[locale]/PaneMainFeatures.tsx index 569dae4..a703bec 100644 --- a/frontend/src/app/[locale]/PaneMainFeatures.tsx +++ b/frontend/src/app/[locale]/PaneMainFeatures.tsx @@ -1,4 +1,3 @@ -import { title, subtitle } from '@/components/primitives'; import { Card, CardHeader, CardBody, Avatar } from '@nextui-org/react'; import { Scale, Folder, Check, Globe } from 'lucide-react'; import { useTranslations } from 'next-intl'; diff --git a/frontend/src/app/[locale]/page.tsx b/frontend/src/app/[locale]/page.tsx index 98dfb2b..fde6e26 100644 --- a/frontend/src/app/[locale]/page.tsx +++ b/frontend/src/app/[locale]/page.tsx @@ -1,5 +1,5 @@ import { useTranslations } from 'next-intl'; -import { Image, Divider } from '@nextui-org/react'; +import { Divider } from '@nextui-org/react'; import { title, subtitle } from '@/components/primitives'; import PaneMainTitle from './PaneMainTitle'; import PaneMainFeatures from './PaneMainFeatures'; diff --git a/frontend/src/app/[locale]/projects/[projectId]/settings/SettingsPage.tsx b/frontend/src/app/[locale]/projects/[projectId]/settings/SettingsPage.tsx index 960bbe2..1d05d50 100644 --- a/frontend/src/app/[locale]/projects/[projectId]/settings/SettingsPage.tsx +++ b/frontend/src/app/[locale]/projects/[projectId]/settings/SettingsPage.tsx @@ -92,7 +92,7 @@ export default function SettingsPage({ projectId, messages, locale }: Props) { startContent={} size="sm" color="danger" - isDisabled={!context.isProjectManager(Number(projectId))} + isDisabled={!context.isProjectOwner(Number(projectId))} onClick={() => setIsDeleteConfirmDialogOpen(true)} > {messages.deleteProject} @@ -101,7 +101,7 @@ export default function SettingsPage({ projectId, messages, locale }: Props) { startContent={} size="sm" color="primary" - isDisabled={!context.isProjectManager(Number(projectId))} + isDisabled={!context.isProjectOwner(Number(projectId))} onClick={() => setIsProjectDialogOpen(true)} className="ms-2" > diff --git a/frontend/types/user.ts b/frontend/types/user.ts index 0484bcb..1e29a7f 100644 --- a/frontend/types/user.ts +++ b/frontend/types/user.ts @@ -18,16 +18,17 @@ export type TokenProps = { export type TokenType = { access_token: string; expires_at: number; - user: UserType; + user: UserType | null; }; export type TokenContextType = { token: { access_token: string; - user: UserType; + user: UserType | null; }; isSignedIn: () => boolean; isAdmin: () => boolean; + isProjectOwner: (projectId: number) => boolean; isProjectManager: (projectId: number) => boolean; isProjectDeveloper: (projectId: number) => boolean; isProjectReporter: (projectId: number) => boolean; diff --git a/frontend/utils/TokenProvider.tsx b/frontend/utils/TokenProvider.tsx index 27cdbcf..b66e20e 100644 --- a/frontend/utils/TokenProvider.tsx +++ b/frontend/utils/TokenProvider.tsx @@ -6,6 +6,7 @@ import { useRouter, usePathname } from '@/src/navigation'; import { isSignedIn as tokenIsSinedIn, isAdmin as tokenIsAdmin, + isProjectOnwer as tokenIsProjectOnwer, isProjectManager as tokenIsProjectManager, isProjectDeveloper as tokenIsProjectDeveloper, isProjectReporter as tokenIsProjectReporter, @@ -30,12 +31,19 @@ const defaultContext = { }, isSignedIn: () => false, isAdmin: () => false, + isProjectOwner: (projectId: number) => { + return false; + }, isProjectManager: (projectId: number) => { return false; }, isProjectDeveloper: (projectId: number) => { return false; }, + isProjectReporter: (projectId: number) => { + return false; + }, + refreshProjectRoles: () => {}, setToken: (token: TokenType) => {}, storeTokenToLocalStorage, removeTokenFromLocalStorage, @@ -63,6 +71,10 @@ const TokenProvider = ({ toastMessages, locale, children }: TokenProps) => { return tokenIsAdmin(token); }; + const isProjectOwner = (projectId: number) => { + return tokenIsProjectOnwer(projectRoles, projectId); + }; + const isProjectManager = (projectId: number) => { return tokenIsProjectManager(projectRoles, projectId); }; @@ -93,6 +105,7 @@ const TokenProvider = ({ toastMessages, locale, children }: TokenProps) => { projectRoles, isSignedIn, isAdmin, + isProjectOwner, isProjectManager, isProjectDeveloper, isProjectReporter, diff --git a/frontend/utils/token.ts b/frontend/utils/token.ts index f49ab59..fe29c15 100644 --- a/frontend/utils/token.ts +++ b/frontend/utils/token.ts @@ -30,7 +30,7 @@ function isSignedIn(token: TokenType): boolean { function isAdmin(token: TokenType) { if (tokenExists(token) && isTokenValid(token)) { const adminRoleIndex = roles.findIndex((entry) => entry.uid === 'administrator'); - if (token.user.role === adminRoleIndex) { + if (token.user && token.user.role === adminRoleIndex) { return true; } } @@ -61,6 +61,26 @@ async function fetchMyRoles(jwt: string) { } } +function isProjectOnwer(projectRoles: ProjectRoleType[], projectId: number) { + if (!projectRoles) { + return false; + } + + const found = projectRoles.find((role) => { + return role.projectId === projectId; + }); + + if (!found) { + return false; + } + + if (found.isOwner === true) { + return true; + } + + return false; +} + function isProjectManager(projectRoles: ProjectRoleType[], projectId: number) { if (!projectRoles) { return false; @@ -172,6 +192,7 @@ function checkSignInPage(token: TokenType, pathname: string) { export { isSignedIn, isAdmin, + isProjectOnwer, isProjectManager, isProjectDeveloper, isProjectReporter,