Skip to content

Commit 6c8eb6a

Browse files
committed
Merge PR #2293 into staging
# Conflicts: # backend/src/services/file-lifecycle.service.ts
2 parents cd837b9 + d945cf2 commit 6c8eb6a

3 files changed

Lines changed: 197 additions & 0 deletions

File tree

backend/src/endpoints/file/file.controller.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ export class FileController {
181181
dto,
182182
auth.user,
183183
auth.apiKey?.action,
184+
auth.apiKey,
184185
);
185186
return plainToInstance(FileDto, file, {
186187
excludeExtraneousValues: true,

backend/src/services/file-lifecycle.service.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import { MissionGuardService } from '@/endpoints/auth/mission-guard.service';
12
import { TriggerService } from '@/services/trigger.service';
23
import { TemporaryFileAccessesDto, UpdateFile } from '@kleinkram/api-dto';
4+
import { ApiKeyEntity } from '@kleinkram/backend-common';
35
import { FileAuditService } from '@kleinkram/backend-common/audit/file-audit.service';
46
import { redis } from '@kleinkram/backend-common/consts';
57
import { ActionEntity } from '@kleinkram/backend-common/entities/action/action.entity';
@@ -27,6 +29,7 @@ import {
2729
import {
2830
BadRequestException,
2931
ConflictException,
32+
ForbiddenException,
3033
HttpException,
3134
HttpStatus,
3235
Inject,
@@ -75,6 +78,7 @@ export class FileLifecycleService implements OnModuleInit {
7578
private readonly dataSource: DataSource,
7679
private readonly auditService: FileAuditService,
7780
private readonly triggerService: TriggerService,
81+
private readonly missionGuardService: MissionGuardService,
7882
) {}
7983

8084
onModuleInit(): void {
@@ -88,6 +92,7 @@ export class FileLifecycleService implements OnModuleInit {
8892
file: UpdateFile,
8993
actor?: UserEntity,
9094
action?: ActionEntity,
95+
apiKey?: ApiKeyEntity,
9196
): Promise<FileEntity | null> {
9297
logger.debug(`Updating file with uuid: ${uuid}`);
9398

@@ -127,6 +132,8 @@ export class FileLifecycleService implements OnModuleInit {
127132
file.missionUuid &&
128133
file.missionUuid !== databaseFile.mission.uuid
129134
) {
135+
await this.checkMovePermission(file.missionUuid, actor, apiKey);
136+
130137
oldMissionUuid = databaseFile.mission.uuid;
131138
const newMission = await this.missionRepository.findOneOrFail({
132139
where: { uuid: file.missionUuid },
@@ -708,4 +715,37 @@ export class FileLifecycleService implements OnModuleInit {
708715

709716
return filesToFix.length;
710717
}
718+
719+
private async checkMovePermission(
720+
newMissionUuid: string,
721+
actor?: UserEntity,
722+
apiKey?: ApiKeyEntity,
723+
): Promise<void> {
724+
if (apiKey) {
725+
if (newMissionUuid !== apiKey.mission.uuid) {
726+
throw new ForbiddenException(
727+
'API keys cannot move files to a different mission.',
728+
);
729+
}
730+
if (apiKey.rights < AccessGroupRights.CREATE) {
731+
throw new ForbiddenException(
732+
'API Key does not have CREATE permission on the destination mission.',
733+
);
734+
}
735+
} else if (actor) {
736+
const hasCreateAccess =
737+
await this.missionGuardService.canAccessMission(
738+
actor,
739+
newMissionUuid,
740+
AccessGroupRights.CREATE,
741+
);
742+
if (!hasCreateAccess) {
743+
throw new ForbiddenException(
744+
'User does not have CREATE permission on the destination mission.',
745+
);
746+
}
747+
} else {
748+
throw new ForbiddenException('Unauthorized action.');
749+
}
750+
}
711751
}

backend/tests/actions/api-key-scope.test.ts

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
ActionEntity,
88
ActionTemplateEntity,
99
ApiKeyEntity,
10+
FileEntity,
1011
MissionEntity,
1112
ProjectEntity,
1213
UserEntity,
@@ -18,6 +19,7 @@ import {
1819
createMissionUsingPost,
1920
createProjectUsingPost,
2021
HeaderCreator,
22+
uploadFile,
2123
} from '../utils/api-calls';
2224
import { clearAllData, database } from '../utils/database-utilities';
2325

@@ -295,4 +297,158 @@ describe('Verify Action API Key Scope', () => {
295297

296298
expect(projectResponse.status).toBe(403); // Forbidden
297299
});
300+
301+
test('if an action API key CANNOT move a file to another mission (even in the same project)', async () => {
302+
// 1. Submit Action in the original mission
303+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
304+
const headers = new HeaderCreator(globalThis.creator);
305+
headers.addHeader('Content-Type', 'application/json');
306+
const submitResponse = await fetch(`${DEFAULT_URL}/actions`, {
307+
method: 'POST',
308+
headers: headers.getHeaders(),
309+
body: JSON.stringify({
310+
missionUUID: globalThis.missionUuid,
311+
templateUUID: globalThis.templateUuid,
312+
} as SubmitActionDto),
313+
});
314+
expect(submitResponse.status).toBe(201);
315+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
316+
const { actionUUID: uuid } = await submitResponse.json();
317+
318+
// 2. Get API Key
319+
const actionRepo = database.getRepository(ActionEntity);
320+
const action = await actionRepo.findOneOrFail({
321+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
322+
where: { uuid },
323+
});
324+
325+
const apiKeyRepo = database.getRepository(ApiKeyEntity);
326+
const apiKeyEntity = apiKeyRepo.create({
327+
// eslint-disable-next-line @typescript-eslint/naming-convention
328+
key_type: KeyTypes.ACTION,
329+
mission: { uuid: globalThis.missionUuid },
330+
action: action,
331+
rights: AccessGroupRights.WRITE,
332+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
333+
user: globalThis.creator,
334+
});
335+
await apiKeyRepo.save(apiKeyEntity);
336+
const apiKey = apiKeyEntity.apikey;
337+
338+
// 3. Create another mission in the same project
339+
const otherMissionUuid = await createMissionUsingPost(
340+
{
341+
name: 'other_mission_same_project',
342+
projectUUID: globalThis.projectUuid,
343+
tags: {},
344+
ignoreTags: true,
345+
},
346+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
347+
globalThis.creator,
348+
);
349+
// 4. Upload a file in the original mission using the helper (so it exists on S3)
350+
await uploadFile(
351+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
352+
globalThis.creator,
353+
'test.bag',
354+
globalThis.missionUuid,
355+
);
356+
const fileRepo = database.getRepository(FileEntity);
357+
const file = await fileRepo.findOneByOrFail({ filename: 'test.bag' });
358+
359+
// 5. Try to move file to the other mission using PUT /files/:uuid
360+
const moveResponse = await fetch(`${DEFAULT_URL}/files/${file.uuid}`, {
361+
method: 'PUT',
362+
headers: {
363+
// eslint-disable-next-line @typescript-eslint/naming-convention
364+
'Content-Type': 'application/json',
365+
// eslint-disable-next-line @typescript-eslint/naming-convention
366+
'x-api-key': apiKey,
367+
// eslint-disable-next-line @typescript-eslint/naming-convention
368+
'kleinkram-client-version': appVersion,
369+
},
370+
body: JSON.stringify({
371+
uuid: file.uuid,
372+
filename: 'test.bag',
373+
date: file.date.toISOString(),
374+
missionUuid: otherMissionUuid,
375+
}),
376+
});
377+
378+
expect(moveResponse.status).toBe(403); // Forbidden
379+
});
380+
381+
test('if an action API key CAN update a file filename within its own mission', async () => {
382+
// 1. Submit Action in the original mission
383+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
384+
const headers = new HeaderCreator(globalThis.creator);
385+
headers.addHeader('Content-Type', 'application/json');
386+
const submitResponse = await fetch(`${DEFAULT_URL}/actions`, {
387+
method: 'POST',
388+
headers: headers.getHeaders(),
389+
body: JSON.stringify({
390+
missionUUID: globalThis.missionUuid,
391+
templateUUID: globalThis.templateUuid,
392+
} as SubmitActionDto),
393+
});
394+
expect(submitResponse.status).toBe(201);
395+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
396+
const { actionUUID: uuid } = await submitResponse.json();
397+
398+
// 2. Get API Key
399+
const actionRepo = database.getRepository(ActionEntity);
400+
const action = await actionRepo.findOneOrFail({
401+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
402+
where: { uuid },
403+
});
404+
405+
const apiKeyRepo = database.getRepository(ApiKeyEntity);
406+
const apiKeyEntity = apiKeyRepo.create({
407+
// eslint-disable-next-line @typescript-eslint/naming-convention
408+
key_type: KeyTypes.ACTION,
409+
mission: { uuid: globalThis.missionUuid },
410+
action: action,
411+
rights: AccessGroupRights.WRITE,
412+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
413+
user: globalThis.creator,
414+
});
415+
await apiKeyRepo.save(apiKeyEntity);
416+
const apiKey = apiKeyEntity.apikey;
417+
418+
// 3. Upload a file in the original mission using the helper (so it exists on S3)
419+
await uploadFile(
420+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
421+
globalThis.creator,
422+
'test.bag',
423+
globalThis.missionUuid,
424+
);
425+
const fileRepo = database.getRepository(FileEntity);
426+
const file = await fileRepo.findOneByOrFail({ filename: 'test.bag' });
427+
428+
// 4. Update file filename within the same mission using PUT /files/:uuid
429+
const updateResponse = await fetch(
430+
`${DEFAULT_URL}/files/${file.uuid}`,
431+
{
432+
method: 'PUT',
433+
headers: {
434+
// eslint-disable-next-line @typescript-eslint/naming-convention
435+
'Content-Type': 'application/json',
436+
// eslint-disable-next-line @typescript-eslint/naming-convention
437+
'x-api-key': apiKey,
438+
// eslint-disable-next-line @typescript-eslint/naming-convention
439+
'kleinkram-client-version': appVersion,
440+
},
441+
body: JSON.stringify({
442+
uuid: file.uuid,
443+
filename: 'updated.bag',
444+
date: file.date.toISOString(),
445+
missionUuid: globalThis.missionUuid,
446+
}),
447+
},
448+
);
449+
450+
expect(updateResponse.status).toBe(200);
451+
const updatedFile = await fileRepo.findOneByOrFail({ uuid: file.uuid });
452+
expect(updatedFile.filename).toBe('updated.bag');
453+
});
298454
});

0 commit comments

Comments
 (0)