Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-05-31 01:16:01 +02:00
Code Issues Projects Releases Wiki Activity

fix: feature not found in project yields 404 (#3958)

Browse Source 
## About the changes
When a feature is not found in a project we should fail with a NotFound
error. If the feature belongs to a different project, it should not be a
permission issue, because the user might not be aware (lack of
permissions/visibility) of that other project, so even in this case the
error should be NotFound (this also works if we ever allow the same
feature name in different projects)

Fixes #3726

---------

Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
This commit is contained in:
Gastón Fournier 2023-06-12 15:07:18 +02:00 committed by GitHub
parent 7003351b35
commit 818b8e7813
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 21 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/lib/services/feature-toggle-service.ts
View File

@ -206,8 +206,12 @@ class FeatureToggleService {
const id = await this.featureToggleStore.getProjectId(featureName);
if (id !== projectId) {
throw new InvalidOperationError(
`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}".`,
throw new NotFoundError(
`There's no feature named "${featureName}" in project "${projectId}"${
id === undefined
? '.'
: `, but there's a feature with that name in project "${id}"`
}`,
);
}
}

2
src/lib/types/stores/feature-toggle-store.ts
View File

@ -11,7 +11,7 @@ export interface IFeatureToggleQuery {
export interface IFeatureToggleStore extends Store<FeatureToggle, string> {
count(query?: Partial<IFeatureToggleQuery>): Promise<number>;
setLastSeen(toggleNames: string[]): Promise<void>;
getProjectId(name: string): Promise<string>;
getProjectId(name: string): Promise<string | undefined>;
create(project: string, data: FeatureToggleDTO): Promise<FeatureToggle>;
update(project: string, data: FeatureToggleDTO): Promise<FeatureToggle>;
archive(featureName: string): Promise<FeatureToggle>;

28
src/test/e2e/api/admin/project/features.e2e.test.ts
View File

@ -575,7 +575,7 @@ test('Trying to toggle environment that does not exist yields 404', async () =>
test('Getting feature that does not exist should yield 404', async () => {
await app.request
.get('/api/admin/projects/default/features/non.existing.feature')
.expect(403);
.expect(404);
});
describe('Interacting with features using project IDs that belong to other projects', () => {
@ -617,34 +617,34 @@ describe('Interacting with features using project IDs that belong to other proje
await db.stores.userStore.deleteAll();
});
test("Getting a feature yields 403 if the provided project id doesn't match the feature's project", async () => {
test("Getting a feature yields 404 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);
.expect(404);
});
test("Getting a feature yields 403 if the provided project doesn't exist", async () => {
test("Getting a feature yields 404 if the provided project doesn't exist", async () => {
await app.request
.get(
`/api/admin/projects/${nonExistingProject}/features/${featureName}`,
)
.expect(403);
.expect(404);
});
test("Archiving a feature yields 403 if the provided project id doesn't match the feature's project", async () => {
test("Archiving a feature yields 404 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);
.expect(404);
});
test("Archiving a feature yields 403 if the provided project doesn't exist", async () => {
test("Archiving a feature yields 404 if the provided project doesn't exist", async () => {
await app.request
.delete(
`/api/admin/projects/${nonExistingProject}/features/${featureName}`,
)
.expect(403);
.expect(404);
});
test("Trying to archive a feature that doesn't exist should yield a 404, regardless of whether the project exists or not.", async () => {
@ -1055,7 +1055,7 @@ test('add strategy cannot use wrong projectId', async () => {
userIds: '',
},
})
.expect(403);
.expect(404);
});
test('update strategy on feature toggle cannot use wrong projectId', async () => {
@ -2215,9 +2215,9 @@ test('should validate context when calling update with PUT', async () => {
.put(`/api/admin/projects/another-project/features/${name}`)
.send({ name, description: 'updated', type: 'kill-switch' })
.expect((res) => {
expect(res.body.name).toBe('InvalidOperationError');
expect(res.body.name).toBe('NotFoundError');
})
.expect(403);
.expect(404);
});
test('should validate context when calling update with PATCH', async () => {
@ -2231,9 +2231,9 @@ test('should validate context when calling update with PATCH', async () => {
.patch(`/api/admin/projects/another-project/features/${name}`)
.send([])
.expect((res) => {
expect(res.body.name).toBe('InvalidOperationError');
expect(res.body.name).toBe('NotFoundError');
})
.expect(403);
.expect(404);
});
test('should not update project with PUT', async () => {