diff --git a/frontend/src/component/segments/CreateSegment/CreateSegment.tsx b/frontend/src/component/segments/CreateSegment/CreateSegment.tsx index c72af26d5f..86007eab5a 100644 --- a/frontend/src/component/segments/CreateSegment/CreateSegment.tsx +++ b/frontend/src/component/segments/CreateSegment/CreateSegment.tsx @@ -1,7 +1,10 @@ import React, { useContext } from 'react'; import { CreateButton } from 'component/common/CreateButton/CreateButton'; import FormTemplate from 'component/common/FormTemplate/FormTemplate'; -import { CREATE_SEGMENT } from 'component/providers/AccessProvider/permissions'; +import { + CREATE_SEGMENT, + UPDATE_PROJECT_SEGMENT, +} from 'component/providers/AccessProvider/permissions'; import { useSegmentsApi } from 'hooks/api/actions/useSegmentsApi/useSegmentsApi'; import { useConstraintsValidation } from 'hooks/api/getters/useConstraintsValidation/useConstraintsValidation'; import { useSegments } from 'hooks/api/getters/useSegments/useSegments'; @@ -112,7 +115,8 @@ export const CreateSegment = ({ modal }: ICreateSegmentProps) => { > diff --git a/frontend/src/component/segments/EditSegment/EditSegment.tsx b/frontend/src/component/segments/EditSegment/EditSegment.tsx index 107991208c..912da57cee 100644 --- a/frontend/src/component/segments/EditSegment/EditSegment.tsx +++ b/frontend/src/component/segments/EditSegment/EditSegment.tsx @@ -1,5 +1,8 @@ import FormTemplate from 'component/common/FormTemplate/FormTemplate'; -import { UPDATE_SEGMENT } from 'component/providers/AccessProvider/permissions'; +import { + UPDATE_PROJECT_SEGMENT, + UPDATE_SEGMENT, +} from 'component/providers/AccessProvider/permissions'; import { useSegmentsApi } from 'hooks/api/actions/useSegmentsApi/useSegmentsApi'; import { useConstraintsValidation } from 'hooks/api/getters/useConstraintsValidation/useConstraintsValidation'; import { useSegment } from 'hooks/api/getters/useSegment/useSegment'; @@ -135,7 +138,8 @@ export const EditSegment = ({ modal }: IEditSegmentProps) => { mode='edit' > diff --git a/frontend/src/hooks/api/actions/useApi/useApi.ts b/frontend/src/hooks/api/actions/useApi/useApi.ts index b8ff9c6628..1bc0627870 100644 --- a/frontend/src/hooks/api/actions/useApi/useApi.ts +++ b/frontend/src/hooks/api/actions/useApi/useApi.ts @@ -194,14 +194,14 @@ const useAPI = ({ loadingOn: boolean = true, ) => { const start = timeApiCallStart( - requestId || `Uknown request happening on ${apiCaller}`, + requestId || `Unknown request happening on ${apiCaller}`, ); const res = await requestFunction(apiCaller, requestId, loadingOn); timeApiCallEnd( start, - requestId || `Uknown request happening on ${apiCaller}`, + requestId || `Unknown request happening on ${apiCaller}`, ); return res; diff --git a/src/lib/error/permission-error.ts b/src/lib/error/permission-error.ts index 7b17064c1b..8b5fb0094e 100644 --- a/src/lib/error/permission-error.ts +++ b/src/lib/error/permission-error.ts @@ -15,7 +15,7 @@ class PermissionError extends UnleashError { const permissionsMessage = permissions.length === 1 ? `the "${permissions[0]}" permission` - : `all of the following permissions: ${permissions + : `one of the following permissions: ${permissions .map((perm) => `"${perm}"`) .join(', ')}`; diff --git a/src/lib/features/segment/segment-controller.ts b/src/lib/features/segment/segment-controller.ts index 345ef66439..c79728ca10 100644 --- a/src/lib/features/segment/segment-controller.ts +++ b/src/lib/features/segment/segment-controller.ts @@ -30,6 +30,7 @@ import { IFlagResolver, NONE, UPDATE_FEATURE_STRATEGY, + UPDATE_PROJECT_SEGMENT, UPDATE_SEGMENT, serializeDates, } from '../../types'; @@ -165,7 +166,7 @@ export class SegmentsController extends Controller { method: 'delete', path: '/:id', handler: this.removeSegment, - permission: DELETE_SEGMENT, + permission: [DELETE_SEGMENT, UPDATE_PROJECT_SEGMENT], acceptAnyContentType: true, middleware: [ openApiService.validPath({ @@ -186,7 +187,7 @@ export class SegmentsController extends Controller { method: 'put', path: '/:id', handler: this.updateSegment, - permission: UPDATE_SEGMENT, + permission: [UPDATE_SEGMENT, UPDATE_PROJECT_SEGMENT], middleware: [ openApiService.validPath({ summary: 'Update segment by id', @@ -225,7 +226,7 @@ export class SegmentsController extends Controller { method: 'post', path: '', handler: this.createSegment, - permission: CREATE_SEGMENT, + permission: [CREATE_SEGMENT, UPDATE_PROJECT_SEGMENT], middleware: [ openApiService.validPath({ summary: 'Create a new segment', diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index 53dfbe08a4..ad34d8c853 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -7,12 +7,16 @@ import ApiUser from '../types/api-user'; import { IFeatureToggleStore } from '../features/feature-toggle/types/feature-toggle-store-type'; import FakeFeatureToggleStore from '../features/feature-toggle/fakes/fake-feature-toggle-store'; import { ApiTokenType } from '../types/models/api-token'; +import { ISegmentStore } from '../types'; +import FakeSegmentStore from '../../test/fixtures/fake-segment-store'; let config: IUnleashConfig; let featureToggleStore: IFeatureToggleStore; +let segmentStore: ISegmentStore; beforeEach(() => { featureToggleStore = new FakeFeatureToggleStore(); + segmentStore = new FakeSegmentStore(); config = createTestConfig(); }); @@ -21,7 +25,11 @@ test('should add checkRbac to request', () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); @@ -40,7 +48,11 @@ test('should give api-user ADMIN permission', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -66,7 +78,11 @@ test('should not give api-user ADMIN permission', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -94,7 +110,11 @@ test('should not allow user to miss userId', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -116,7 +136,11 @@ test('should return false for missing user', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = {}; @@ -134,7 +158,11 @@ test('should verify permission for root resource', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -163,7 +191,11 @@ test('should lookup projectId from params', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -198,7 +230,11 @@ test('should lookup projectId from feature toggle', async () => { featureToggleStore.getProjectId = jest.fn().mockReturnValue(projectId); - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -231,7 +267,11 @@ test('should lookup projectId from data', async () => { hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { @@ -266,7 +306,11 @@ test('Does not double check permission if not changing project when updating tog }; featureToggleStore.getProjectId = jest.fn().mockReturnValue(oldProjectId); - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { user: new User({ username: 'user', id: 1 }), @@ -290,7 +334,11 @@ test('UPDATE_TAG_TYPE does not need projectId', async () => { hasPermission: jest.fn().mockReturnValue(true), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { user: new User({ username: 'user', id: 1 }), @@ -314,7 +362,11 @@ test('DELETE_TAG_TYPE does not need projectId', async () => { hasPermission: jest.fn().mockReturnValue(true), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { user: new User({ username: 'user', id: 1 }), @@ -340,7 +392,11 @@ test('should not expect featureName for UPDATE_FEATURE when projectId specified' hasPermission: jest.fn(), }; - const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const func = rbacMiddleware( + config, + { featureToggleStore, segmentStore }, + accessService, + ); const cb = jest.fn(); const req: any = { diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index 78adb57b38..ff6a2daf45 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -3,6 +3,8 @@ import { DELETE_FEATURE, ADMIN, UPDATE_FEATURE, + DELETE_SEGMENT, + UPDATE_PROJECT_SEGMENT, } from '../types/permissions'; import { IUnleashConfig } from '../types/option'; import { IUnleashStores } from '../types/stores'; @@ -32,7 +34,10 @@ export function findParam( const rbacMiddleware = ( config: Pick, - { featureToggleStore }: Pick, + { + featureToggleStore, + segmentStore, + }: Pick, accessService: PermissionChecker, ): any => { const logger = config.getLogger('/middleware/rbac-middleware.ts'); @@ -87,6 +92,17 @@ const rbacMiddleware = ( projectId = 'default'; } + // DELETE segment does not include information about the segment's project + // This is needed to check if the user has the right permissions on a project level + if ( + !projectId && + permissionsArray.includes(UPDATE_PROJECT_SEGMENT) + ) { + const { id } = params; + const { project } = await segmentStore.get(id); + projectId = project; + } + return accessService.hasPermission( user, permissionsArray,