fix(server): validate duplicate group ownership before dismissal (#28221)

This commit is contained in:
Timon
2026-05-04 12:51:54 +02:00
committed by GitHub
parent 3decc864b5
commit d4f29ab6ac
7 changed files with 43 additions and 10 deletions
+1 -1
View File
@@ -146,7 +146,7 @@ Class | Method | HTTP request | Description
*DeprecatedApi* | [**runQueueCommandLegacy**](doc//DeprecatedApi.md#runqueuecommandlegacy) | **PUT** /jobs/{name} | Run jobs
*DownloadApi* | [**downloadArchive**](doc//DownloadApi.md#downloadarchive) | **POST** /download/archive | Download asset archive
*DownloadApi* | [**getDownloadInfo**](doc//DownloadApi.md#getdownloadinfo) | **POST** /download/info | Retrieve download information
*DuplicatesApi* | [**deleteDuplicate**](doc//DuplicatesApi.md#deleteduplicate) | **DELETE** /duplicates/{id} | Delete a duplicate
*DuplicatesApi* | [**deleteDuplicate**](doc//DuplicatesApi.md#deleteduplicate) | **DELETE** /duplicates/{id} | Dismiss a duplicate group
*DuplicatesApi* | [**deleteDuplicates**](doc//DuplicatesApi.md#deleteduplicates) | **DELETE** /duplicates | Delete duplicates
*DuplicatesApi* | [**getAssetDuplicates**](doc//DuplicatesApi.md#getassetduplicates) | **GET** /duplicates | Retrieve duplicates
*DuplicatesApi* | [**resolveDuplicates**](doc//DuplicatesApi.md#resolveduplicates) | **POST** /duplicates/resolve | Resolve duplicate groups
+4 -4
View File
@@ -16,9 +16,9 @@ class DuplicatesApi {
final ApiClient apiClient;
/// Delete a duplicate
/// Dismiss a duplicate group
///
/// Delete a single duplicate asset specified by its ID.
/// Dismiss a duplicate group by its ID, unlinking all assets in the group without deleting them.
///
/// Note: This method returns the HTTP [Response].
///
@@ -51,9 +51,9 @@ class DuplicatesApi {
);
}
/// Delete a duplicate
/// Dismiss a duplicate group
///
/// Delete a single duplicate asset specified by its ID.
/// Dismiss a duplicate group by its ID, unlinking all assets in the group without deleting them.
///
/// Parameters:
///
+2 -2
View File
@@ -5172,7 +5172,7 @@
},
"/duplicates/{id}": {
"delete": {
"description": "Delete a single duplicate asset specified by its ID.",
"description": "Dismiss a duplicate group by its ID, unlinking all assets in the group without deleting them.",
"operationId": "deleteDuplicate",
"parameters": [
{
@@ -5202,7 +5202,7 @@
"api_key": []
}
],
"summary": "Delete a duplicate",
"summary": "Dismiss a duplicate group",
"tags": [
"Duplicates"
],
+1 -1
View File
@@ -4480,7 +4480,7 @@ export function resolveDuplicates({ duplicateResolveDto }: {
})));
}
/**
* Delete a duplicate
* Dismiss a duplicate group
*/
export function deleteDuplicate({ id }: {
id: string;
@@ -41,8 +41,8 @@ export class DuplicateController {
@Authenticated({ permission: Permission.DuplicateDelete })
@HttpCode(HttpStatus.NO_CONTENT)
@Endpoint({
summary: 'Delete a duplicate',
description: 'Delete a single duplicate asset specified by its ID.',
summary: 'Dismiss a duplicate group',
description: 'Dismiss a duplicate group by its ID, unlinking all assets in the group without deleting them.',
history: new HistoryBuilder().added('v1').beta('v1').stable('v2'),
})
deleteDuplicate(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<void> {
@@ -1,3 +1,4 @@
import { BadRequestException } from '@nestjs/common';
import { BulkIdErrorReason } from 'src/dtos/asset-ids.response.dto';
import { MapAsset } from 'src/dtos/asset-response.dto';
import { AssetType, AssetVisibility, JobName, JobStatus } from 'src/enum';
@@ -149,6 +150,36 @@ describe(DuplicateService.name, () => {
});
});
describe('delete', () => {
it('should throw for an unknown or unauthorized group id', async () => {
mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set());
await expect(sut.delete(authStub.admin, 'group-1')).rejects.toThrow(BadRequestException);
expect(mocks.duplicateRepository.delete).not.toHaveBeenCalled();
});
it('should dismiss the duplicate group', async () => {
mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));
mocks.duplicateRepository.delete.mockResolvedValue();
await expect(sut.delete(authStub.admin, 'group-1')).resolves.toBeUndefined();
expect(mocks.duplicateRepository.delete).toHaveBeenCalledWith(authStub.admin.user.id, 'group-1');
});
});
describe('deleteAll', () => {
it('should throw if any group id is unknown or unauthorized', async () => {
mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));
await expect(sut.deleteAll(authStub.admin, { ids: ['group-1', 'group-2'] })).rejects.toThrow(BadRequestException);
expect(mocks.duplicateRepository.deleteAll).not.toHaveBeenCalled();
});
it('should dismiss all duplicate groups', async () => {
mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1', 'group-2']));
mocks.duplicateRepository.deleteAll.mockResolvedValue();
await expect(sut.deleteAll(authStub.admin, { ids: ['group-1', 'group-2'] })).resolves.toBeUndefined();
expect(mocks.duplicateRepository.deleteAll).toHaveBeenCalledWith(authStub.admin.user.id, ['group-1', 'group-2']);
});
});
describe('resolve', () => {
it('should handle mixed success and failure', async () => {
const asset = AssetFactory.create();
+2
View File
@@ -82,10 +82,12 @@ export class DuplicateService extends BaseService {
}
async delete(auth: AuthDto, id: string): Promise<void> {
await this.requireAccess({ auth, permission: Permission.DuplicateDelete, ids: [id] });
await this.duplicateRepository.delete(auth.user.id, id);
}
async deleteAll(auth: AuthDto, dto: BulkIdsDto) {
await this.requireAccess({ auth, permission: Permission.DuplicateDelete, ids: dto.ids });
await this.duplicateRepository.deleteAll(auth.user.id, dto.ids);
}