diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index 1f63d26339..5615efed5c 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -252,40 +252,6 @@ test('should lookup projectId from data', async () => { ); }); -test('Need access to UPDATE_FEATURE on the project you change to', async () => { - const oldProjectId = 'some-project-34'; - const newProjectId = 'some-project-35'; - const featureName = 'some-feature-toggle'; - const accessService = { - hasPermission: jest.fn().mockReturnValue(true), - }; - featureToggleStore.getProjectId = jest.fn().mockReturnValue(oldProjectId); - - const func = rbacMiddleware(config, { featureToggleStore }, accessService); - const cb = jest.fn(); - const req: any = { - user: new User({ username: 'user', id: 1 }), - params: { featureName }, - body: { featureName, project: newProjectId }, - }; - func(req, undefined, cb); - - await req.checkRbac(perms.UPDATE_FEATURE); - expect(accessService.hasPermission).toHaveBeenCalledTimes(2); - expect(accessService.hasPermission).toHaveBeenNthCalledWith( - 1, - req.user, - perms.UPDATE_FEATURE, - oldProjectId, - ); - expect(accessService.hasPermission).toHaveBeenNthCalledWith( - 2, - req.user, - perms.UPDATE_FEATURE, - newProjectId, - ); -}); - test('Does not double check permission if not changing project when updating toggle', async () => { const oldProjectId = 'some-project-34'; const featureName = 'some-feature-toggle'; diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index c5da8c6f07..9d74647b7d 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -1,9 +1,9 @@ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ import { CREATE_FEATURE, - UPDATE_FEATURE, DELETE_FEATURE, ADMIN, + UPDATE_FEATURE, } from '../types/permissions'; import { IUnleashConfig } from '../types/option'; import { IUnleashStores } from '../types/stores'; @@ -47,33 +47,10 @@ const rbacMiddleware = ( let { projectId } = params; // Temporary workaround to figure out projectId for feature toggle updates. - if (permission === DELETE_FEATURE) { + // will be removed in Unleash v5.0 + if ([DELETE_FEATURE, UPDATE_FEATURE].includes(permission)) { const { featureName } = params; projectId = await featureToggleStore.getProjectId(featureName); - } else if (permission === UPDATE_FEATURE) { - // if projectId of feature is different from project in body - // need to check that we have UPDATE_FEATURE access on both old and new project - // TODO: Look at this to make it smoother once we get around to looking at project - // Changing project of a toggle should most likely be a separate endpoint - const { featureName } = params; - projectId = await featureToggleStore.getProjectId(featureName); - const newProjectId = req.body - ? req.body.project || projectId - : projectId; - if (newProjectId !== projectId) { - return ( - accessService.hasPermission( - user, - permission, - projectId, - ) && - accessService.hasPermission( - user, - permission, - newProjectId, - ) - ); - } } else if (permission === CREATE_FEATURE) { projectId = req.body.project || 'default'; } @@ -84,5 +61,4 @@ const rbacMiddleware = ( }; }; -module.exports = rbacMiddleware; export default rbacMiddleware; diff --git a/src/lib/routes/admin-api/archive.ts b/src/lib/routes/admin-api/archive.ts index 1b90495fda..96d2cb1d72 100644 --- a/src/lib/routes/admin-api/archive.ts +++ b/src/lib/routes/admin-api/archive.ts @@ -7,7 +7,7 @@ import Controller from '../controller'; import { extractUsername } from '../../util/extract-user'; import { DELETE_FEATURE, UPDATE_FEATURE } from '../../types/permissions'; -import FeatureToggleServiceV2 from '../../services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../services/feature-toggle-service'; import { IAuthRequest } from '../unleash-types'; export default class ArchiveController extends Controller { diff --git a/src/lib/routes/admin-api/feature.ts b/src/lib/routes/admin-api/feature.ts index 9a99ab5e1e..a91d767966 100644 --- a/src/lib/routes/admin-api/feature.ts +++ b/src/lib/routes/admin-api/feature.ts @@ -11,7 +11,7 @@ import { } from '../../types/permissions'; import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types/services'; -import FeatureToggleServiceV2 from '../../services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../services/feature-toggle-service'; import { featureSchema, querySchema } from '../../schema/feature-schema'; import { IFeatureToggleQuery } from '../../types/model'; import FeatureTagService from '../../services/feature-tag-service'; @@ -150,8 +150,11 @@ class FeatureController extends Controller { toggle.strategies.map(async (s) => this.service.createStrategy( s, - createdFeature.project, - createdFeature.name, + { + projectId: createdFeature.project, + featureName: createdFeature.name, + environment: DEFAULT_ENV, + }, userName, ), ), @@ -190,8 +193,7 @@ class FeatureController extends Controller { updatedFeature.strategies.map(async (s) => this.service.createStrategy( s, - projectId, - featureName, + { projectId, featureName, environment: DEFAULT_ENV }, userName, ), ), diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index a12079ae1a..ce24f4f3de 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -3,7 +3,7 @@ import { applyPatch, Operation } from 'fast-json-patch'; import Controller from '../../controller'; import { IUnleashConfig } from '../../../types/option'; import { IUnleashServices } from '../../../types/services'; -import FeatureToggleServiceV2 from '../../../services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../../services/feature-toggle-service'; import { Logger } from '../../../logger'; import { CREATE_FEATURE, UPDATE_FEATURE } from '../../../types/permissions'; import { @@ -64,18 +64,20 @@ export default class ProjectFeaturesController extends Controller { this.featureService = featureToggleServiceV2; this.logger = config.getLogger('/admin-api/project/features.ts'); + // Environments this.get(`${PATH_ENV}`, this.getEnvironment); this.post(`${PATH_ENV}/on`, this.toggleEnvironmentOn, UPDATE_FEATURE); this.post(`${PATH_ENV}/off`, this.toggleEnvironmentOff, UPDATE_FEATURE); + // activation strategies this.get(`${PATH_STRATEGIES}`, this.getStrategies); this.post(`${PATH_STRATEGIES}`, this.addStrategy, UPDATE_FEATURE); - this.get(`${PATH_STRATEGY}`, this.getStrategy); this.put(`${PATH_STRATEGY}`, this.updateStrategy, UPDATE_FEATURE); this.patch(`${PATH_STRATEGY}`, this.patchStrategy, UPDATE_FEATURE); this.delete(`${PATH_STRATEGY}`, this.deleteStrategy, UPDATE_FEATURE); + // feature toggles this.get(PATH, this.getFeatures); this.post(PATH, this.createFeature, CREATE_FEATURE); @@ -244,10 +246,8 @@ export default class ProjectFeaturesController extends Controller { const userName = extractUsername(req); const featureStrategy = await this.featureService.createStrategy( req.body, - projectId, - featureName, + { environment, projectId, featureName }, userName, - environment, ); res.status(200).json(featureStrategy); } @@ -270,14 +270,13 @@ export default class ProjectFeaturesController extends Controller { req: IAuthRequest, res: Response, ): Promise { - const { strategyId, environment, projectId } = req.params; + const { strategyId, environment, projectId, featureName } = req.params; const userName = extractUsername(req); const updatedStrategy = await this.featureService.updateStrategy( strategyId, - environment, - projectId, - userName, req.body, + { environment, projectId, featureName }, + userName, ); res.status(200).json(updatedStrategy); } @@ -286,17 +285,16 @@ export default class ProjectFeaturesController extends Controller { req: IAuthRequest, res: Response, ): Promise { - const { strategyId, projectId, environment } = req.params; + const { strategyId, projectId, environment, featureName } = req.params; const userName = extractUsername(req); const patch = req.body; const strategy = await this.featureService.getStrategy(strategyId); const { newDocument } = applyPatch(strategy, patch); const updatedStrategy = await this.featureService.updateStrategy( strategyId, - environment, - projectId, - userName, newDocument, + { environment, projectId, featureName }, + userName, ); res.status(200).json(updatedStrategy); } @@ -323,10 +321,8 @@ export default class ProjectFeaturesController extends Controller { this.logger.info(strategyId); const strategy = await this.featureService.deleteStrategy( strategyId, - featureName, + { environment, projectId, featureName }, userName, - projectId, - environment, ); res.status(200).json(strategy); } @@ -340,7 +336,7 @@ export default class ProjectFeaturesController extends Controller { >, res: Response, ): Promise { - const { strategyId, environment, projectId } = req.params; + const { strategyId, environment, projectId, featureName } = req.params; const userName = extractUsername(req); const { name, value } = req.body; @@ -349,9 +345,8 @@ export default class ProjectFeaturesController extends Controller { strategyId, name, value, + { environment, projectId, featureName }, userName, - projectId, - environment, ); res.status(200).json(updatedStrategy); } diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index e4e363a6a3..91ddc1066a 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -3,7 +3,7 @@ import { Response } from 'express'; import Controller from '../controller'; import { IUnleashServices } from '../../types/services'; import { IUnleashConfig } from '../../types/option'; -import FeatureToggleServiceV2 from '../../services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../services/feature-toggle-service'; import { Logger } from '../../logger'; import { querySchema } from '../../schema/feature-schema'; import { IFeatureToggleQuery } from '../../types/model'; diff --git a/src/lib/services/feature-toggle-service-v2.ts b/src/lib/services/feature-toggle-service.ts similarity index 91% rename from src/lib/services/feature-toggle-service-v2.ts rename to src/lib/services/feature-toggle-service.ts index 9aaeb3df24..fe647070c4 100644 --- a/src/lib/services/feature-toggle-service-v2.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -48,6 +48,15 @@ import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client import { DEFAULT_ENV } from '../util/constants'; import { applyPatch, deepClone, Operation } from 'fast-json-patch'; +interface IFeatureContext { + featureName: string; + projectId: string; +} + +interface IFeatureStrategyContext extends IFeatureContext { + environment: string; +} + class FeatureToggleServiceV2 { private logger: Logger; @@ -96,6 +105,35 @@ class FeatureToggleServiceV2 { this.featureEnvironmentStore = featureEnvironmentStore; } + async validateFeatureContext({ + featureName, + projectId, + }: IFeatureContext): Promise { + const id = await this.featureToggleStore.getProjectId(featureName); + if (id !== projectId) { + throw new InvalidOperationError( + 'You can not change the projectId for an activation strategy.', + ); + } + } + + validateFeatureStrategyContext( + strategy: IFeatureStrategy, + { featureName, projectId }: IFeatureStrategyContext, + ): void { + if (strategy.projectId !== projectId) { + throw new InvalidOperationError( + 'You can not change the projectId for an activation strategy.', + ); + } + + if (strategy.featureName !== featureName) { + throw new InvalidOperationError( + 'You can not change the featureName for an activation strategy.', + ); + } + } + async patchFeature( projectId: string, featureName: string, @@ -126,11 +164,11 @@ class FeatureToggleServiceV2 { async createStrategy( strategyConfig: Omit, - projectId: string, - featureName: string, + context: IFeatureStrategyContext, userName: string, - environment: string = DEFAULT_ENV, ): Promise { + const { featureName, projectId, environment } = context; + await this.validateFeatureContext(context); try { const newFeatureStrategy = await this.featureStrategiesStore.createStrategyFeatureEnv({ @@ -176,17 +214,20 @@ class FeatureToggleServiceV2 { * } * @param id * @param updates + * @param context - Which context does this strategy live in (projectId, featureName, environment) + * @param userName - Human readable id of the user performing the update */ - // TODO: verify projectId is not changed from URL! async updateStrategy( id: string, - environment: string, - project: string, - userName: string, updates: Partial, + context: IFeatureStrategyContext, + userName: string, ): Promise { + const { projectId, environment } = context; const existingStrategy = await this.featureStrategiesStore.get(id); + this.validateFeatureStrategyContext(existingStrategy, context); + if (existingStrategy.id === id) { const strategy = await this.featureStrategiesStore.updateStrategy( id, @@ -201,7 +242,7 @@ class FeatureToggleServiceV2 { }; await this.eventStore.store({ type: FEATURE_STRATEGY_UPDATE, - project, + project: projectId, environment, createdBy: userName, data, @@ -211,16 +252,18 @@ class FeatureToggleServiceV2 { throw new NotFoundError(`Could not find strategy with id ${id}`); } - // TODO: verify projectId is not changed from URL! async updateStrategyParameter( id: string, name: string, value: string | number, + context: IFeatureStrategyContext, userName: string, - project: string, - environment: string, ): Promise { + const { projectId, environment } = context; + const existingStrategy = await this.featureStrategiesStore.get(id); + this.validateFeatureStrategyContext(existingStrategy, context); + if (existingStrategy.id === id) { existingStrategy.parameters[name] = value; const strategy = await this.featureStrategiesStore.updateStrategy( @@ -235,7 +278,7 @@ class FeatureToggleServiceV2 { }; await this.eventStore.store({ type: FEATURE_STRATEGY_UPDATE, - project, + project: projectId, environment, createdBy: userName, data, @@ -251,22 +294,22 @@ class FeatureToggleServiceV2 { * * } * @param id - strategy id - * @param featureName - Name of the feature the strategy belongs to - * @param userName - Who's doing the change - * @param project - Which project does this feature toggle belong to + * @param context - Which context does this strategy live in (projectId, featureName, environment) * @param environment - Which environment does this strategy belong to */ async deleteStrategy( id: string, - featureName: string, + context: IFeatureStrategyContext, userName: string, - project: string = 'default', - environment: string = DEFAULT_ENV, ): Promise { + const existingStrategy = await this.featureStrategiesStore.get(id); + const { featureName, projectId, environment } = context; + this.validateFeatureStrategyContext(existingStrategy, context); + await this.featureStrategiesStore.delete(id); await this.eventStore.store({ type: FEATURE_STRATEGY_REMOVE, - project, + project: projectId, environment, createdBy: userName, data: { @@ -274,6 +317,7 @@ class FeatureToggleServiceV2 { name: featureName, }, }); + // If there are no strategies left for environment disable it await this.featureEnvironmentStore.disableEnvironmentIfNoStrategies( featureName, @@ -444,10 +488,12 @@ class FeatureToggleServiceV2 { createStrategies.push( this.createStrategy( s, - projectId, - newFeatureName, + { + projectId, + featureName: newFeatureName, + environment: e.name, + }, userName, - e.name, ), ); }), diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index bc713d7c2e..8e39e913cb 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -23,7 +23,7 @@ import ResetTokenService from './reset-token-service'; import SettingService from './setting-service'; import SessionService from './session-service'; import UserFeedbackService from './user-feedback-service'; -import FeatureToggleServiceV2 from './feature-toggle-service-v2'; +import FeatureToggleService from './feature-toggle-service'; import EnvironmentService from './environment-service'; import FeatureTagService from './feature-tag-service'; import ProjectHealthService from './project-health-service'; @@ -58,7 +58,7 @@ export const createServices = ( const versionService = new VersionService(stores, config); const healthService = new HealthService(stores, config); const userFeedbackService = new UserFeedbackService(stores, config); - const featureToggleServiceV2 = new FeatureToggleServiceV2(stores, config); + const featureToggleServiceV2 = new FeatureToggleService(stores, config); const environmentService = new EnvironmentService(stores, config); const featureTagService = new FeatureTagService(stores, config); const projectHealthService = new ProjectHealthService( @@ -76,6 +76,7 @@ export const createServices = ( return { accessService, addonService, + featureToggleService: featureToggleServiceV2, featureToggleServiceV2, featureTypeService, healthService, diff --git a/src/lib/services/project-health-service.ts b/src/lib/services/project-health-service.ts index ec2639b608..eddacba478 100644 --- a/src/lib/services/project-health-service.ts +++ b/src/lib/services/project-health-service.ts @@ -11,7 +11,7 @@ import { import { IFeatureToggleStore } from '../types/stores/feature-toggle-store'; import { IFeatureTypeStore } from '../types/stores/feature-type-store'; import { IProjectStore } from '../types/stores/project-store'; -import FeatureToggleServiceV2 from './feature-toggle-service-v2'; +import FeatureToggleServiceV2 from './feature-toggle-service'; import { hoursToMilliseconds } from 'date-fns'; import Timer = NodeJS.Timer; diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index bce88e7837..ba5b29c079 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -27,7 +27,7 @@ import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-st import { IProjectQuery, IProjectStore } from '../types/stores/project-store'; import { IRole } from '../types/stores/access-store'; import { IEventStore } from '../types/stores/event-store'; -import FeatureToggleServiceV2 from './feature-toggle-service-v2'; +import FeatureToggleServiceV2 from './feature-toggle-service'; import { CREATE_FEATURE, UPDATE_FEATURE } from '../types/permissions'; import NoAccessError from '../error/no-access-error'; import IncompatibleProjectError from '../error/incompatible-project-error'; diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index 3ae887a687..b2528df9f8 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -18,7 +18,7 @@ import HealthService from '../services/health-service'; import SettingService from '../services/setting-service'; import SessionService from '../services/session-service'; import UserFeedbackService from '../services/user-feedback-service'; -import FeatureToggleServiceV2 from '../services/feature-toggle-service-v2'; +import FeatureToggleService from '../services/feature-toggle-service'; import EnvironmentService from '../services/environment-service'; import FeatureTagService from '../services/feature-tag-service'; import ProjectHealthService from '../services/project-health-service'; @@ -35,7 +35,8 @@ export interface IUnleashServices { environmentService: EnvironmentService; eventService: EventService; featureTagService: FeatureTagService; - featureToggleServiceV2: FeatureToggleServiceV2; + featureToggleService: FeatureToggleService; + featureToggleServiceV2: FeatureToggleService; // deprecated featureTypeService: FeatureTypeService; healthService: HealthService; projectHealthService: ProjectHealthService; diff --git a/src/test/e2e/api/admin/feature-archive.e2e.test.ts b/src/test/e2e/api/admin/feature-archive.e2e.test.ts index bbbaf29b0a..0d3dc6ffef 100644 --- a/src/test/e2e/api/admin/feature-archive.e2e.test.ts +++ b/src/test/e2e/api/admin/feature-archive.e2e.test.ts @@ -98,8 +98,7 @@ test('returns three archived toggles', async () => { }); }); -test.skip('revives a feature by name', async () => { - expect.assertions(0); +test('revives a feature by name', async () => { return app.request .post('/api/admin/archive/revive/featureArchivedX') .set('Content-Type', 'application/json') diff --git a/src/test/e2e/api/admin/feature.e2e.test.ts b/src/test/e2e/api/admin/feature.e2e.test.ts index f9c72bc350..f1ad2f9392 100644 --- a/src/test/e2e/api/admin/feature.e2e.test.ts +++ b/src/test/e2e/api/admin/feature.e2e.test.ts @@ -3,6 +3,7 @@ import { FeatureToggleDTO, IStrategyConfig } from 'lib/types/model'; import dbInit, { ITestDb } from '../../helpers/database-init'; import { IUnleashTest, setupApp } from '../../helpers/test-helper'; import getLogger from '../../../fixtures/no-logger'; +import { DEFAULT_ENV } from '../../../../lib/util/constants'; let app: IUnleashTest; let db: ITestDb; @@ -30,8 +31,7 @@ beforeAll(async () => { ); await app.services.featureToggleServiceV2.createStrategy( strategy, - projectId, - toggle.name, + { projectId, featureName: toggle.name, environment: DEFAULT_ENV }, username, ); }; @@ -263,6 +263,24 @@ test('can change status of feature toggle that does exist', async () => { .expect(200); }); +test('cannot change project for feature toggle', async () => { + await app.request + .put('/api/admin/features/featureY') + .send({ + name: 'featureY', + enabled: true, + project: 'random', //will be ignored + strategies: [{ name: 'default' }], + }) + .set('Content-Type', 'application/json') + .expect(200); + const { body } = await app.request + .get('/api/admin/features/featureY') + .expect(200); + + expect(body.project).toBe('default'); +}); + test('can not toggle of feature that does not exist', async () => { expect.assertions(0); return app.request @@ -285,8 +303,7 @@ test('can toggle a feature that does exist', async () => { ); await app.services.featureToggleServiceV2.createStrategy( defaultStrategy, - 'default', - featureName, + { projectId: 'default', featureName, environment: DEFAULT_ENV }, username, ); return app.request diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 1477134873..7ddc479d8f 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -643,6 +643,145 @@ test('Can add strategy to feature toggle to a "some-env-2"', async () => { }); }); +test('Can update strategy on feature toggle', async () => { + const envName = 'default'; + const featureName = 'feature.strategy.update.strat'; + + const projectPath = '/api/admin/projects/default'; + const featurePath = `${projectPath}/features/${featureName}`; + + // create feature toggle + await app.request + .post(`${projectPath}/features`) + .send({ name: featureName }) + .expect(201); + + // add strategy + const { body: strategy } = await app.request + .post(`${featurePath}/environments/${envName}/strategies`) + .send({ + name: 'default', + parameters: { + userIds: '', + }, + }) + .expect(200); + + // update strategy + await app.request + .put(`${featurePath}/environments/${envName}/strategies/${strategy.id}`) + .send({ + name: 'default', + parameters: { + userIds: '1234', + }, + }) + .expect(200); + + const { body } = await app.request.get(`${featurePath}`); + const defaultEnv = body.environments[0]; + expect(body.environments).toHaveLength(1); + expect(defaultEnv.name).toBe(envName); + expect(defaultEnv.strategies).toHaveLength(1); + expect(defaultEnv.strategies[0].parameters).toStrictEqual({ + userIds: '1234', + }); +}); + +test('Can NOT delete strategy with wrong projectId', async () => { + const envName = 'default'; + const featureName = 'feature.strategy.delete.strat.error'; + + const projectPath = '/api/admin/projects/default'; + const featurePath = `${projectPath}/features/${featureName}`; + + // create feature toggle + await app.request + .post(`${projectPath}/features`) + .send({ name: featureName }) + .expect(201); + + // add strategy + const { body: strategy } = await app.request + .post(`${featurePath}/environments/${envName}/strategies`) + .send({ + name: 'default', + parameters: { + userIds: '', + }, + }) + .expect(200); + + // delete strategy + await app.request + .delete( + `/api/admin/projects/wrongId/features/${featureName}/environments/${envName}/strategies/${strategy.id}`, + ) + .expect(403); +}); + +test('add strategy cannot use wrong projectId', async () => { + const envName = 'default'; + const featureName = 'feature.strategy.add.strat.wrong.projectId'; + + // create feature toggle + await app.request + .post('/api/admin/projects/default/features') + .send({ name: featureName }) + .expect(201); + + // add strategy + await app.request + .post( + `/api/admin/projects/invalidId/features/${featureName}/environments/${envName}/strategies`, + ) + .send({ + name: 'default', + parameters: { + userIds: '', + }, + }) + .expect(403); +}); + +test('update strategy on feature toggle cannot use wrong projectId', async () => { + const envName = 'default'; + const featureName = 'feature.strategy.update.strat.wrong.projectId'; + + const projectPath = '/api/admin/projects/default'; + const featurePath = `${projectPath}/features/${featureName}`; + + // create feature toggle + await app.request + .post(`${projectPath}/features`) + .send({ name: featureName }) + .expect(201); + + // add strategy + const { body: strategy } = await app.request + .post(`${featurePath}/environments/${envName}/strategies`) + .send({ + name: 'default', + parameters: { + userIds: '', + }, + }) + .expect(200); + + // update strategy + await app.request + .put( + `/api/admin/projects/invalidId/features/${featureName}/environments/${envName}/strategies/${strategy.id}`, + ) + .send({ + name: 'default', + parameters: { + userIds: '1234', + }, + }) + .expect(403); +}); + test('Environments are returned in sortOrder', async () => { const sortedSecond = 'sortedSecond'; const sortedLast = 'sortedLast'; diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index 6d9e7d49fa..b922f903c7 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -136,11 +136,11 @@ test('import for 4.1.2 enterprise format fixed works', async () => { test('Can roundtrip. I.e. export and then import', async () => { const projectId = 'export-project'; - const environmentId = 'export-environment'; + const environment = 'export-environment'; const userName = 'export-user'; const featureName = 'export.feature'; await db.stores.environmentStore.create({ - name: environmentId, + name: environment, type: 'test', }); await db.stores.projectStore.create({ @@ -149,7 +149,7 @@ test('Can roundtrip. I.e. export and then import', async () => { description: 'Project for export', }); await app.services.environmentService.addEnvironmentToProject( - environmentId, + environment, projectId, ); await app.services.featureToggleServiceV2.createFeatureToggle( @@ -169,9 +169,8 @@ test('Can roundtrip. I.e. export and then import', async () => { ], parameters: {}, }, - projectId, - featureName, - environmentId, + { projectId, featureName, environment }, + userName, ); const data = await app.services.stateService.export({}); await app.services.stateService.import({ @@ -184,11 +183,11 @@ test('Can roundtrip. I.e. export and then import', async () => { test('Roundtrip with tags works', async () => { const projectId = 'tags-project'; - const environmentId = 'tags-environment'; + const environment = 'tags-environment'; const userName = 'tags-user'; const featureName = 'tags.feature'; await db.stores.environmentStore.create({ - name: environmentId, + name: environment, type: 'test', }); await db.stores.projectStore.create({ @@ -197,7 +196,7 @@ test('Roundtrip with tags works', async () => { description: 'Project for export', }); await app.services.environmentService.addEnvironmentToProject( - environmentId, + environment, projectId, ); await app.services.featureToggleServiceV2.createFeatureToggle( @@ -217,9 +216,12 @@ test('Roundtrip with tags works', async () => { ], parameters: {}, }, - projectId, - featureName, - environmentId, + { + projectId, + featureName, + environment, + }, + userName, ); await app.services.featureTagService.addTag( featureName, @@ -245,11 +247,11 @@ test('Roundtrip with tags works', async () => { test('Roundtrip with strategies in multiple environments works', async () => { const projectId = 'multiple-environment-project'; - const environmentId = 'multiple-environment-environment'; + const environment = 'multiple-environment-environment'; const userName = 'multiple-environment-user'; const featureName = 'multiple-environment.feature'; await db.stores.environmentStore.create({ - name: environmentId, + name: environment, type: 'test', }); await db.stores.projectStore.create({ @@ -258,7 +260,7 @@ test('Roundtrip with strategies in multiple environments works', async () => { description: 'Project for export', }); await app.services.environmentService.addEnvironmentToProject( - environmentId, + environment, projectId, ); await app.services.environmentService.addEnvironmentToProject( @@ -282,9 +284,8 @@ test('Roundtrip with strategies in multiple environments works', async () => { ], parameters: {}, }, - projectId, - featureName, - environmentId, + { projectId, featureName, environment }, + userName, ); await app.services.featureToggleServiceV2.createStrategy( { @@ -294,9 +295,8 @@ test('Roundtrip with strategies in multiple environments works', async () => { ], parameters: {}, }, - projectId, - featureName, - DEFAULT_ENV, + { projectId, featureName, environment: DEFAULT_ENV }, + userName, ); const data = await app.services.stateService.export({}); await app.services.stateService.import({ diff --git a/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts index 2985559f19..0eeac5eb03 100644 --- a/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts +++ b/src/test/e2e/api/client/feature.env.disabled.e2e.test.ts @@ -1,30 +1,32 @@ import { IUnleashTest, setupApp } from '../../helpers/test-helper'; import dbInit, { ITestDb } from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; +import { DEFAULT_ENV } from '../../../../lib/util/constants'; let app: IUnleashTest; let db: ITestDb; const featureName = 'feature.default.1'; +const username = 'test'; +const projectId = 'default'; beforeAll(async () => { db = await dbInit('feature_env_api_client', getLogger); app = await setupApp(db.stores); await app.services.featureToggleServiceV2.createFeatureToggle( - 'default', + projectId, { name: featureName, description: 'the #1 feature', }, - 'test', + username, ); await app.services.featureToggleServiceV2.createStrategy( { name: 'default', constraints: [], parameters: {} }, - 'default', - featureName, - 'test', + { projectId, featureName, environment: DEFAULT_ENV }, + username, ); }); diff --git a/src/test/e2e/api/client/feature.token.access.e2e.test.ts b/src/test/e2e/api/client/feature.token.access.e2e.test.ts index 8e45a957b1..29097cc92c 100644 --- a/src/test/e2e/api/client/feature.token.access.e2e.test.ts +++ b/src/test/e2e/api/client/feature.token.access.e2e.test.ts @@ -55,8 +55,7 @@ beforeAll(async () => { constraints: [], parameters: {}, }, - project, - feature1, + { projectId: project, featureName: feature1, environment: DEFAULT_ENV }, username, ); await featureToggleServiceV2.createStrategy( @@ -65,10 +64,8 @@ beforeAll(async () => { constraints: [], parameters: {}, }, - project, - feature1, + { projectId: project, featureName: feature1, environment }, username, - environment, ); // create feature 2 @@ -85,10 +82,8 @@ beforeAll(async () => { constraints: [], parameters: {}, }, - project, - feature2, + { projectId: project, featureName: feature2, environment }, username, - environment, ); // create feature 3 @@ -105,10 +100,8 @@ beforeAll(async () => { constraints: [], parameters: {}, }, - project2, - feature3, + { projectId: project2, featureName: feature3, environment }, username, - environment, ); }); diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index 8c20fb224b..121a4936ba 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts @@ -1,4 +1,4 @@ -import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service'; import { IStrategyConfig } from '../../../lib/types/model'; import { createTestConfig } from '../../config/test-config'; import dbInit from '../helpers/database-init'; @@ -41,8 +41,7 @@ test('Should create feature toggle strategy configuration', async () => { const createdConfig = await service.createStrategy( config, - projectId, - 'Demo', + { projectId, featureName: 'Demo', environment: DEFAULT_ENV }, username, ); @@ -53,6 +52,7 @@ test('Should create feature toggle strategy configuration', async () => { test('Should be able to update existing strategy configuration', async () => { const projectId = 'default'; const username = 'existing-strategy'; + const featureName = 'update-existing-strategy'; const config: Omit = { name: 'default', constraints: [], @@ -62,32 +62,33 @@ test('Should be able to update existing strategy configuration', async () => { await service.createFeatureToggle( projectId, { - name: 'update-existing-strategy', + name: featureName, }, 'test', ); const createdConfig = await service.createStrategy( config, - 'default', - 'update-existing-strategy', + { projectId, featureName, environment: DEFAULT_ENV }, username, ); expect(createdConfig.name).toEqual('default'); const updatedConfig = await service.updateStrategy( createdConfig.id, - DEFAULT_ENV, - projectId, - username, { parameters: { b2b: true }, }, + { projectId, featureName, environment: DEFAULT_ENV }, + username, ); expect(createdConfig.id).toEqual(updatedConfig.id); expect(updatedConfig.parameters).toEqual({ b2b: true }); }); test('Should be able to get strategy by id', async () => { + const featureName = 'get-strategy-by-id'; + const projectId = 'default'; + const userName = 'strategy'; const config: Omit = { name: 'default', @@ -95,19 +96,17 @@ test('Should be able to get strategy by id', async () => { parameters: {}, }; await service.createFeatureToggle( - 'default', + projectId, { - name: 'get-strategy-by-id', + name: featureName, }, - 'test', + userName, ); const createdConfig = await service.createStrategy( config, - 'default', - 'Demo', + { projectId, featureName, environment: DEFAULT_ENV }, userName, - DEFAULT_ENV, ); const fetchedConfig = await service.getStrategy(createdConfig.id); expect(fetchedConfig).toEqual(createdConfig); diff --git a/src/test/e2e/services/project-health-service.e2e.test.ts b/src/test/e2e/services/project-health-service.e2e.test.ts index af1e53e3b9..81ba8ca8ac 100644 --- a/src/test/e2e/services/project-health-service.e2e.test.ts +++ b/src/test/e2e/services/project-health-service.e2e.test.ts @@ -1,6 +1,6 @@ import dbInit, { ITestDb } from '../helpers/database-init'; import getLogger from '../../fixtures/no-logger'; -import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service'; import { AccessService } from '../../../lib/services/access-service'; import ProjectService from '../../../lib/services/project-service'; import ProjectHealthService from '../../../lib/services/project-health-service'; diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index e313f2ebcb..2c9fdcc3a1 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -1,6 +1,6 @@ import dbInit, { ITestDb } from '../helpers/database-init'; import getLogger from '../../fixtures/no-logger'; -import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service-v2'; +import FeatureToggleServiceV2 from '../../../lib/services/feature-toggle-service'; import ProjectService from '../../../lib/services/project-service'; import { AccessService } from '../../../lib/services/access-service'; import { diff --git a/src/test/e2e/stores/feature-toggle-store.e2e.test.ts b/src/test/e2e/stores/feature-toggle-store.e2e.test.ts index 12371c502d..f74c008823 100644 --- a/src/test/e2e/stores/feature-toggle-store.e2e.test.ts +++ b/src/test/e2e/stores/feature-toggle-store.e2e.test.ts @@ -6,6 +6,7 @@ let db; let featureToggleStore; beforeAll(async () => { + getLogger.setMuteError(true); db = await dbInit('feature_toggle_store_serial', getLogger); stores = db.stores; featureToggleStore = stores.featureToggleStore; @@ -23,7 +24,6 @@ test('should not crash for unknown toggle', async () => { }); test('should not crash for undefined toggle name', async () => { - getLogger.setMuteError(true); const project = await featureToggleStore.getProjectId(undefined); expect(project).toBe(undefined); });