From 5dc3e830a87de5ef67bcc0e73c7be94db8c2fc02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Wed, 22 Nov 2023 10:20:19 +0000 Subject: [PATCH] feat: add CREATE_TAG_TYPE permission (#5386) https://linear.app/unleash/issue/2-1164/update-tag-type-covers-both-creation-and-update Adds a new `CREATE_TAG_TYPE` permission instead of using `UPDATE_TAG_TYPE` for both actions. --- .../ManageTagsDialog/ManageBulkTagsDialog.tsx | 2 +- .../ManageTagsDialog/ManageTagsDialog.tsx | 2 +- .../providers/AccessProvider/permissions.ts | 3 +- .../tags/CreateTagType/CreateTagType.tsx | 4 +-- .../AddTagTypeButton/AddTagTypeButton.tsx | 6 ++-- .../export-import-permissions.e2e.test.ts | 2 +- .../import-permissions-service.ts | 3 +- src/lib/middleware/rbac-middleware.test.ts | 28 +++++++++++++++++++ src/lib/routes/admin-api/tag-type.ts | 5 ++-- src/lib/types/permissions.ts | 3 +- ...21153304-add-permission-create-tag-type.js | 18 ++++++++++++ 11 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 src/migrations/20231121153304-add-permission-create-tag-type.js diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx index 828d4690de..1a612086ba 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx @@ -224,7 +224,7 @@ export const ManageBulkTagsDialog: VFC = ({ open={open} secondaryButtonText='Cancel' primaryButtonText='Save tags' - title='Update tags to feature toggle' + title='Update feature toggle tags' onClick={() => onSubmit(payload)} disabledPrimaryButton={ payload.addedTags.length === 0 && diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageTagsDialog.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageTagsDialog.tsx index e99b88f070..d86f472c77 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageTagsDialog.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageTagsDialog.tsx @@ -248,7 +248,7 @@ export const ManageTagsDialog = ({ open, setOpen }: IManageTagsProps) => { open={open} secondaryButtonText='Cancel' primaryButtonText='Save tags' - title='Update tags to feature toggle' + title='Update feature toggle tags' onClick={onSubmit} disabledPrimaryButton={loading || differenceCount === 0} onClose={onCancel} diff --git a/frontend/src/component/providers/AccessProvider/permissions.ts b/frontend/src/component/providers/AccessProvider/permissions.ts index 9e64bb71c9..b4db6fd530 100644 --- a/frontend/src/component/providers/AccessProvider/permissions.ts +++ b/frontend/src/component/providers/AccessProvider/permissions.ts @@ -13,8 +13,9 @@ export const DELETE_CONTEXT_FIELD = 'DELETE_CONTEXT_FIELD'; export const CREATE_PROJECT = 'CREATE_PROJECT'; export const UPDATE_PROJECT = 'UPDATE_PROJECT'; export const DELETE_PROJECT = 'DELETE_PROJECT'; -export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE'; +export const CREATE_TAG_TYPE = 'CREATE_TAG_TYPE'; export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE'; +export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE'; export const CREATE_ADDON = 'CREATE_ADDON'; export const UPDATE_ADDON = 'UPDATE_ADDON'; export const DELETE_ADDON = 'DELETE_ADDON'; diff --git a/frontend/src/component/tags/CreateTagType/CreateTagType.tsx b/frontend/src/component/tags/CreateTagType/CreateTagType.tsx index faec6e2782..bd77245048 100644 --- a/frontend/src/component/tags/CreateTagType/CreateTagType.tsx +++ b/frontend/src/component/tags/CreateTagType/CreateTagType.tsx @@ -3,7 +3,7 @@ import useTagTypeForm from '../TagTypeForm/useTagTypeForm'; import TagTypeForm from '../TagTypeForm/TagTypeForm'; import { CreateButton } from 'component/common/CreateButton/CreateButton'; import FormTemplate from 'component/common/FormTemplate/FormTemplate'; -import { UPDATE_TAG_TYPE } from 'component/providers/AccessProvider/permissions'; +import { CREATE_TAG_TYPE } from 'component/providers/AccessProvider/permissions'; import useTagTypesApi from 'hooks/api/actions/useTagTypesApi/useTagTypesApi'; import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; import useToast from 'hooks/useToast'; @@ -78,7 +78,7 @@ const CreateTagType = () => { clearErrors={clearErrors} validateNameUniqueness={validateNameUniqueness} > - + ); diff --git a/frontend/src/component/tags/TagTypeList/AddTagTypeButton/AddTagTypeButton.tsx b/frontend/src/component/tags/TagTypeList/AddTagTypeButton/AddTagTypeButton.tsx index b31bf37c91..c5c14be189 100644 --- a/frontend/src/component/tags/TagTypeList/AddTagTypeButton/AddTagTypeButton.tsx +++ b/frontend/src/component/tags/TagTypeList/AddTagTypeButton/AddTagTypeButton.tsx @@ -1,7 +1,7 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import PermissionButton from 'component/common/PermissionButton/PermissionButton'; import PermissionIconButton from 'component/common/PermissionIconButton/PermissionIconButton'; -import { UPDATE_TAG_TYPE } from 'component/providers/AccessProvider/permissions'; +import { CREATE_TAG_TYPE } from 'component/providers/AccessProvider/permissions'; import useMediaQuery from '@mui/material/useMediaQuery'; import { useNavigate } from 'react-router-dom'; @@ -18,14 +18,14 @@ export const AddTagTypeButton = () => { navigate('/tag-types/create')} size='large' - permission={UPDATE_TAG_TYPE} + permission={CREATE_TAG_TYPE} > } elseShow={ navigate('/tag-types/create')} > New tag type diff --git a/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts b/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts index 127ad70b7e..f33c7d5973 100644 --- a/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts @@ -373,11 +373,11 @@ test('validate import data', async () => { affectedItems: [ 'Create feature toggles', 'Update feature toggles', - 'Update tag types', 'Create context fields', 'Create activation strategies', 'Delete activation strategies', 'Update variants', + 'Create tag types', ], }, ], diff --git a/src/lib/features/export-import-toggles/import-permissions-service.ts b/src/lib/features/export-import-toggles/import-permissions-service.ts index bf6b11fc87..ddf54f9a32 100644 --- a/src/lib/features/export-import-toggles/import-permissions-service.ts +++ b/src/lib/features/export-import-toggles/import-permissions-service.ts @@ -11,6 +11,7 @@ import { UPDATE_FEATURE, UPDATE_FEATURE_ENVIRONMENT_VARIANTS, UPDATE_TAG_TYPE, + CREATE_TAG_TYPE, } from '../../types'; import { PermissionError } from '../../error'; @@ -94,7 +95,7 @@ export class ImportPermissionsService { ]); const permissions = [UPDATE_FEATURE]; if (newTagTypes.length > 0) { - permissions.push(UPDATE_TAG_TYPE); + permissions.push(CREATE_TAG_TYPE); } if (Array.isArray(newContextFields) && newContextFields.length > 0) { permissions.push(CREATE_CONTEXT_FIELD); diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index ad34d8c853..05a9f1501d 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -329,6 +329,34 @@ test('Does not double check permission if not changing project when updating tog ); }); +test('CREATE_TAG_TYPE does not need projectId', async () => { + const accessService = { + hasPermission: jest.fn().mockReturnValue(true), + }; + + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); + const cb = jest.fn(); + const req: any = { + user: new User({ username: 'user', id: 1 }), + params: {}, + body: { name: 'new-tag-type', description: 'New tag type for testing' }, + }; + func(req, undefined, cb); + + await req.checkRbac(perms.CREATE_TAG_TYPE); + expect(accessService.hasPermission).toHaveBeenCalledTimes(1); + expect(accessService.hasPermission).toHaveBeenCalledWith( + req.user, + [perms.CREATE_TAG_TYPE], + undefined, + undefined, + ); +}); + test('UPDATE_TAG_TYPE does not need projectId', async () => { const accessService = { hasPermission: jest.fn().mockReturnValue(true), diff --git a/src/lib/routes/admin-api/tag-type.ts b/src/lib/routes/admin-api/tag-type.ts index 65e9c9c049..ffb2eaa155 100644 --- a/src/lib/routes/admin-api/tag-type.ts +++ b/src/lib/routes/admin-api/tag-type.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import Controller from '../controller'; import { + CREATE_TAG_TYPE, DELETE_TAG_TYPE, NONE, UPDATE_TAG_TYPE, @@ -72,7 +73,7 @@ class TagTypeController extends Controller { method: 'post', path: '', handler: this.createTagType, - permission: UPDATE_TAG_TYPE, + permission: CREATE_TAG_TYPE, middleware: [ openApiService.validPath({ tags: ['Tags'], @@ -91,7 +92,7 @@ class TagTypeController extends Controller { method: 'post', path: '/validate', handler: this.validateTagType, - permission: UPDATE_TAG_TYPE, + permission: NONE, middleware: [ openApiService.validPath({ tags: ['Tags'], diff --git a/src/lib/types/permissions.ts b/src/lib/types/permissions.ts index 146f45a7b2..49db78eb66 100644 --- a/src/lib/types/permissions.ts +++ b/src/lib/types/permissions.ts @@ -37,6 +37,7 @@ export const CREATE_STRATEGY = 'CREATE_STRATEGY'; export const UPDATE_STRATEGY = 'UPDATE_STRATEGY'; export const DELETE_STRATEGY = 'DELETE_STRATEGY'; +export const CREATE_TAG_TYPE = 'CREATE_TAG_TYPE'; export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE'; export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE'; @@ -112,6 +113,6 @@ export const ROOT_PERMISSION_CATEGORIES = [ }, { label: 'Tag type', - permissions: [UPDATE_TAG_TYPE, DELETE_TAG_TYPE], + permissions: [CREATE_TAG_TYPE, UPDATE_TAG_TYPE, DELETE_TAG_TYPE], }, ]; diff --git a/src/migrations/20231121153304-add-permission-create-tag-type.js b/src/migrations/20231121153304-add-permission-create-tag-type.js new file mode 100644 index 0000000000..35f4539d9c --- /dev/null +++ b/src/migrations/20231121153304-add-permission-create-tag-type.js @@ -0,0 +1,18 @@ +exports.up = function (db, cb) { + db.runSql( + ` + INSERT INTO permissions (permission, display_name, type) VALUES ('CREATE_TAG_TYPE', 'Create tag types', 'root'); + SELECT assign_unleash_permission_to_role('CREATE_TAG_TYPE', 'Editor'); + `, + cb + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + DELETE FROM permissions WHERE permission = 'CREATE_TAG_TYPE'; + `, + cb + ); +};