fix: disalow attachment delete

This commit is contained in:
Mohamed Marrouchi 2025-05-12 18:03:58 +01:00
parent 75d96650aa
commit 78b217c2f7
3 changed files with 37 additions and 95 deletions

View File

@ -9,7 +9,10 @@
import fs from 'fs';
import { CACHE_MANAGER } from '@nestjs/cache-manager';
import { BadRequestException } from '@nestjs/common/exceptions';
import {
BadRequestException,
MethodNotAllowedException,
} from '@nestjs/common/exceptions';
import { NotFoundException } from '@nestjs/common/exceptions/not-found.exception';
import { MongooseModule } from '@nestjs/mongoose';
import { Request } from 'express';
@ -215,29 +218,10 @@ describe('AttachmentController', () => {
});
describe('deleteOne', () => {
it('should delete an attachment by id', async () => {
jest.spyOn(attachmentService, 'deleteOne');
const result = await attachmentController.deleteOne(
attachmentToDelete.id,
);
expect(attachmentService.deleteOne).toHaveBeenCalledWith(
attachmentToDelete.id,
);
expect(result).toEqual({
acknowledged: true,
deletedCount: 1,
});
});
it('should throw a NotFoundException when attempting to delete an attachment by id', async () => {
it('should throw a MethodNotAllowedException when attempting to delete an attachment by id', async () => {
await expect(
attachmentController.deleteOne(attachmentToDelete.id),
).rejects.toThrow(
new NotFoundException(
`Attachment with ID ${attachmentToDelete.id} not found`,
),
);
).rejects.toThrow(MethodNotAllowedException);
});
});
});

View File

@ -13,6 +13,7 @@ import {
ForbiddenException,
Get,
HttpCode,
MethodNotAllowedException,
NotFoundException,
Param,
Post,
@ -32,7 +33,6 @@ import { config } from '@/config';
import { CsrfInterceptor } from '@/interceptors/csrf.interceptor';
import { Roles } from '@/utils/decorators/roles.decorator';
import { BaseController } from '@/utils/generics/base-controller';
import { DeleteResult } from '@/utils/generics/base-repository';
import { PageQueryDto } from '@/utils/pagination/pagination-query.dto';
import { PageQueryPipe } from '@/utils/pagination/pagination-query.pipe';
import { SearchFilterPipe } from '@/utils/pipes/search-filter.pipe';
@ -185,20 +185,21 @@ export class AttachmentController extends BaseController<Attachment> {
}
/**
* Deletes an attachment with the specified ID.
* Deletion of attachments is disallowed to prevent database inconsistencies.
* Attachments may be referenced by blocks, messages, or content elements,
* and deleting them directly could lead to orphaned references or broken UI.
*
* @param id - The ID of the attachment to delete.
* @returns A promise that resolves to the result of the deletion operation.
* @param id - The ID of the attachment (not used since deletion is not allowed).
* @throws MethodNotAllowedException - Always thrown to indicate deletion is not permitted.
*/
@CsrfCheck(true)
@Delete(':id')
@HttpCode(204)
async deleteOne(@Param('id') id: string): Promise<DeleteResult> {
const result = await this.attachmentService.deleteOne(id);
if (result.deletedCount === 0) {
this.logger.warn(`Unable to delete attachment by id ${id}`);
throw new NotFoundException(`Attachment with ID ${id} not found`);
}
return result;
@HttpCode(405)
async deleteOne(@Param('id') id: string): Promise<void> {
// Deletion is explicitly disallowed due to potential reference issues.
this.logger.warn(`Attempted deletion of attachment ${id} is not allowed.`);
throw new MethodNotAllowedException(
'Deletion of attachments is not permitted to avoid data inconsistency.',
);
}
}

View File

@ -1,13 +1,12 @@
/*
* Copyright © 2024 Hexastack. All rights reserved.
* Copyright © 2025 Hexastack. All rights reserved.
*
* Licensed under the GNU Affero General Public License v3.0 (AGPLv3) with the following additional terms:
* 1. The name "Hexabot" is a trademark of Hexastack. You may not use this name in derivative works without express written permission.
* 2. All derivative works must include clear attribution to the original creator and software, Hexastack and Hexabot, in a prominent location (e.g., in the software's "About" section, documentation, and README file).
*/
import CancelOutlinedIcon from "@mui/icons-material/CancelOutlined";
import DeleteOutlineOutlinedIcon from "@mui/icons-material/DeleteOutlineOutlined";
import ClearIcon from "@mui/icons-material/Clear";
import FileOpenIcon from "@mui/icons-material/FileOpen";
import MusicNoteIcon from "@mui/icons-material/MusicNote";
import VideoCameraBackOutlinedIcon from "@mui/icons-material/VideoCameraBackOutlined";
@ -21,18 +20,11 @@ import {
} from "@mui/material";
import { FC } from "react";
import { useDelete } from "@/hooks/crud/useDelete";
import { useGet } from "@/hooks/crud/useGet";
import { useDialogs } from "@/hooks/useDialogs";
import useFormattedFileSize from "@/hooks/useFormattedFileSize";
import { useHasPermission } from "@/hooks/useHasPermission";
import { useToast } from "@/hooks/useToast";
import { useTranslate } from "@/hooks/useTranslate";
import { EntityType } from "@/services/types";
import { IAttachment } from "@/types/attachment.types";
import { PermissionAction } from "@/types/permission.types";
import { ConfirmDialogBody } from "../dialogs";
const AttachmentPreview = ({
attachment,
@ -79,22 +71,10 @@ const AttachmentThumbnail: FC<AttachmentThumbnailProps> = ({
onChange,
}) => {
const formatSize = useFormattedFileSize();
const hasPermission = useHasPermission();
const { data: attachment } = useGet(id, {
entity: EntityType.ATTACHMENT,
});
const { toast } = useToast();
const { t } = useTranslate();
const dialogs = useDialogs();
const { mutate: deleteAttachment } = useDelete(EntityType.ATTACHMENT, {
onError: () => {
toast.error(t("message.internal_server_error"));
},
onSuccess: () => {
toast.success(t("message.success_save"));
onChange && onChange(null);
},
});
if (!attachment) {
return t("message.attachment_not_found") + id;
@ -117,45 +97,22 @@ const AttachmentThumbnail: FC<AttachmentThumbnailProps> = ({
</Typography>
</CardContent>
{format === "full" &&
hasPermission(EntityType.ATTACHMENT, PermissionAction.DELETE) &&
onChange ? (
<>
<CardActions sx={{ justifyContent: "center", flex: "1 1 50%" }}>
<Button
color="primary"
variant="contained"
startIcon={<CancelOutlinedIcon />}
onClick={(e) => {
onChange && onChange(null);
e.preventDefault();
e.stopPropagation();
}}
size="small"
>
{t("button.unselect")}
</Button>
<Button
color="secondary"
variant="contained"
startIcon={<DeleteOutlineOutlinedIcon />}
onClick={async (e) => {
const isConfirmed = await dialogs.confirm(
ConfirmDialogBody,
);
if (isConfirmed) {
deleteAttachment(attachment.id);
}
e.preventDefault();
e.stopPropagation();
}}
size="small"
>
{t("button.remove")}
</Button>
</CardActions>
</>
{format === "full" && onChange ? (
<CardActions sx={{ justifyContent: "center", flex: "1 1 50%" }}>
<Button
color="inherit"
variant="contained"
startIcon={<ClearIcon />}
onClick={async (e) => {
onChange && onChange(null);
e.preventDefault();
e.stopPropagation();
}}
size="small"
>
{t("button.delete")}
</Button>
</CardActions>
) : null}
</>
) : null}