diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index 2959122659..ef1ee12929 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -38,7 +38,10 @@ import { createResponseSchema } from '../../../openapi/util/create-response-sche import { FeatureEnvironmentSchema } from '../../../openapi/spec/feature-environment-schema'; import { SetStrategySortOrderSchema } from '../../../openapi/spec/set-strategy-sort-order-schema'; -import { emptyResponse } from '../../../openapi/util/standard-responses'; +import { + emptyResponse, + getStandardResponses, +} from '../../../openapi/util/standard-responses'; import { SegmentService } from '../../../services/segment-service'; interface FeatureStrategyParams { @@ -319,7 +322,17 @@ export default class ProjectFeaturesController extends Controller { openApiService.validPath({ operationId: 'getFeature', tags: ['Features'], - responses: { 200: createResponseSchema('featureSchema') }, + description: + 'This endpoint returns the information about the requested feature if the feature belongs to the specified project.', + summary: 'Get a feature.', + responses: { + 200: createResponseSchema('featureSchema'), + 403: { + description: + 'You either do not have the required permissions or used an invalid URL.', + }, + ...getStandardResponses(401, 404), + }, }), ], }); @@ -364,7 +377,17 @@ export default class ProjectFeaturesController extends Controller { openApiService.validPath({ tags: ['Features'], operationId: 'archiveFeature', - responses: { 200: emptyResponse }, + description: + 'This endpoint archives the specified feature if the feature belongs to the specified project.', + summary: 'Archive a feature.', + responses: { + 200: emptyResponse, + 403: { + description: + 'You either do not have the required permissions or used an invalid URL.', + }, + ...getStandardResponses(401, 404), + }, }), ], }); @@ -438,8 +461,12 @@ export default class ProjectFeaturesController extends Controller { req: Request, res: Response, ): Promise { - const { featureName } = req.params; - const feature = await this.featureService.getFeature(featureName); + const { featureName, projectId } = req.params; + const feature = await this.featureService.getFeature( + featureName, + false, + projectId, + ); res.status(200).json(feature); } @@ -493,7 +520,6 @@ export default class ProjectFeaturesController extends Controller { ); } - // TODO: validate projectId async archiveFeature( req: IAuthRequest< { projectId: string; featureName: string }, @@ -503,9 +529,13 @@ export default class ProjectFeaturesController extends Controller { >, res: Response, ): Promise { - const { featureName } = req.params; + const { featureName, projectId } = req.params; const userName = extractUsername(req); - await this.featureService.archiveToggle(featureName, userName); + await this.featureService.archiveToggle( + featureName, + userName, + projectId, + ); res.status(202).send(); } diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 9eb72be567..34fa48b9f0 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -159,7 +159,7 @@ class FeatureToggleService { const id = await this.featureToggleStore.getProjectId(featureName); if (id !== projectId) { throw new InvalidOperationError( - 'Project id does not match the project that the feature belongs to', + `The operation could not be completed. The feature exists, but the provided project id ("${projectId}") does not match the project that the feature belongs to ("${id}"). Try using "${id}" in the request URL instead of "${projectId}".`, ); } } @@ -548,15 +548,22 @@ class FeatureToggleService { * GET /api/admin/projects/:project/features/:featureName * @param featureName * @param archived - return archived or non archived toggles + * @param projectId - provide if you're requesting the feature in the context of a specific project. */ async getFeature( featureName: string, archived: boolean = false, + projectId?: string, ): Promise { - return this.featureStrategiesStore.getFeatureToggleWithEnvs( - featureName, - archived, - ); + const feature = + await this.featureStrategiesStore.getFeatureToggleWithEnvs( + featureName, + archived, + ); + if (projectId) { + await this.validateFeatureContext({ featureName, projectId }); + } + return feature; } /** @@ -865,9 +872,17 @@ class FeatureToggleService { return feature; } - // todo: add projectId - async archiveToggle(featureName: string, createdBy: string): Promise { + async archiveToggle( + featureName: string, + createdBy: string, + projectId?: string, + ): Promise { const feature = await this.featureToggleStore.get(featureName); + + if (projectId) { + await this.validateFeatureContext({ featureName, projectId }); + } + await this.featureToggleStore.archive(featureName); const tags = await this.tagStore.getAllTagsForFeature(featureName); await this.eventStore.store( 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 d40d257747..b5b8613654 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -13,7 +13,7 @@ import { import ApiUser from '../../../../../lib/types/api-user'; import { ApiTokenType } from '../../../../../lib/types/models/api-token'; import IncompatibleProjectError from '../../../../../lib/error/incompatible-project-error'; -import { IVariant, WeightType } from '../../../../../lib/types/model'; +import { IVariant, RoleName, WeightType } from '../../../../../lib/types/model'; import { v4 as uuidv4 } from 'uuid'; import supertest from 'supertest'; import { randomId } from '../../../../../lib/util/random-id'; @@ -459,6 +459,89 @@ test('Getting feature that does not exist should yield 404', async () => { .expect(404); }); +describe('Interacting with features using project IDs that belong to other projects', () => { + const otherProject = 'project2'; + const featureName = 'new-toggle'; + const nonExistingProject = 'this-is-not-a-project'; + + beforeAll(async () => { + const dummyAdmin = await app.services.userService.createUser({ + name: 'Some Name', + email: 'test@getunleash.io', + rootRole: RoleName.ADMIN, + }); + await app.services.projectService.createProject( + { name: otherProject, id: otherProject }, + dummyAdmin, + ); + + // ensure the new project has been created + await app.request + .get(`/api/admin/projects/${otherProject}`) + .expect(200); + + // create toggle in default project + await app.request + .post('/api/admin/projects/default/features') + .send({ name: featureName }) + .expect(201); + }); + + afterAll(async () => { + await db.stores.projectStore.delete(otherProject); + await db.stores.featureToggleStore.delete(featureName); + await db.stores.userStore.deleteAll(); + }); + + test("Getting a feature yields 403 if the provided project id doesn't match the feature's project", async () => { + await app.request + .get(`/api/admin/projects/${otherProject}/features/${featureName}`) + .expect(403); + }); + + test("Getting a feature yields 403 if the provided project doesn't exist", async () => { + await app.request + .get( + `/api/admin/projects/${nonExistingProject}/features/${featureName}`, + ) + .expect(403); + }); + + test("Archiving a feature yields 403 if the provided project id doesn't match the feature's project", async () => { + await app.request + .delete( + `/api/admin/projects/${otherProject}/features/${featureName}`, + ) + .expect(403); + }); + + test("Archiving a feature yields 403 if the provided project doesn't exist", async () => { + await app.request + .delete( + `/api/admin/projects/${nonExistingProject}/features/${featureName}`, + ) + .expect(403); + }); + + test("Trying to archive a feature that doesn't exist should yield a 404, regardless of whether the project exists or not.", async () => { + await app.request + .delete( + `/api/admin/projects/${nonExistingProject}/features/${ + featureName + featureName + }`, + ) + .expect(404); + + await app.request + .delete( + `/api/admin/projects/${otherProject}/features/${ + featureName + featureName + }`, + ) + .expect(404); + }); +}); + test('Should update feature toggle', async () => { const url = '/api/admin/projects/default/features'; const name = 'new.toggle.update'; diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 44598061c8..56ed7e7911 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -4991,6 +4991,7 @@ If the provided project does not exist, the list of events will be empty.", }, "/api/admin/projects/{projectId}/features/{featureName}": { "delete": { + "description": "This endpoint archives the specified feature if the feature belongs to the specified project.", "operationId": "archiveFeature", "parameters": [ { @@ -5014,12 +5015,23 @@ If the provided project does not exist, the list of events will be empty.", "200": { "description": "This response has no body.", }, + "401": { + "description": "Authorization information is missing or invalid. Provide a valid API token as the \`authorization\` header, e.g. \`authorization:*.*.my-admin-token\`.", + }, + "403": { + "description": "You either do not have the required permissions or used an invalid URL.", + }, + "404": { + "description": "The requested resource was not found.", + }, }, + "summary": "Archive a feature.", "tags": [ "Features", ], }, "get": { + "description": "This endpoint returns the information about the requested feature if the feature belongs to the specified project.", "operationId": "getFeature", "parameters": [ { @@ -5050,7 +5062,17 @@ If the provided project does not exist, the list of events will be empty.", }, "description": "featureSchema", }, + "401": { + "description": "Authorization information is missing or invalid. Provide a valid API token as the \`authorization\` header, e.g. \`authorization:*.*.my-admin-token\`.", + }, + "403": { + "description": "You either do not have the required permissions or used an invalid URL.", + }, + "404": { + "description": "The requested resource was not found.", + }, }, + "summary": "Get a feature.", "tags": [ "Features", ],