fix(server): prevent cross-tenant attachment access (IDOR)
Add tenant-scoped document lookup with throwIfNotFound() before S3 operations in GetAttachment, DeleteAttachment, and GetAttachmentPresignedUrl services. This prevents users from reading, deleting, or generating presigned URLs for attachments belonging to other tenants. Also adds RequirePermission decorators to the three attachment endpoints and introduces Attachment ability subject with View and Delete actions. GHSA-rc4v-wq22-v6cf Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -31,6 +31,9 @@ import { AttachmentUploadPipeline } from './S3UploadPipeline';
|
||||
import { FileInterceptor } from '@/common/interceptors/file.interceptor';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { ApiCommonHeaders } from '@/common/decorators/ApiCommonHeaders';
|
||||
import { RequirePermission } from '@/modules/Roles/RequirePermission.decorator';
|
||||
import { AbilitySubject } from '@/modules/Roles/Roles.types';
|
||||
import { AttachmentAction } from './Attachments.types';
|
||||
|
||||
@ApiTags('Attachments')
|
||||
@Controller('/attachments')
|
||||
@@ -86,6 +89,7 @@ export class AttachmentsController {
|
||||
@ApiOperation({ summary: 'Get attachment by ID' })
|
||||
@ApiParam({ name: 'id', description: 'Attachment ID' })
|
||||
@ApiResponse({ status: 200, description: 'Returns the attachment file' })
|
||||
@RequirePermission(AttachmentAction.View, AbilitySubject.Attachment)
|
||||
async getAttachment(
|
||||
@Res() res: Response,
|
||||
@Param('id') documentId: string,
|
||||
@@ -112,6 +116,7 @@ export class AttachmentsController {
|
||||
status: 200,
|
||||
description: 'The document has been deleted successfully',
|
||||
})
|
||||
@RequirePermission(AttachmentAction.Delete, AbilitySubject.Attachment)
|
||||
async deleteAttachment(@Param('id') documentId: string) {
|
||||
await this.attachmentsApplication.delete(documentId);
|
||||
|
||||
@@ -185,6 +190,7 @@ export class AttachmentsController {
|
||||
status: 200,
|
||||
description: 'Returns the presigned URL for the attachment',
|
||||
})
|
||||
@RequirePermission(AttachmentAction.View, AbilitySubject.Attachment)
|
||||
async getAttachmentPresignedUrl(@Param('id') documentKey: string) {
|
||||
const presignedUrl =
|
||||
await this.attachmentsApplication.getPresignedUrl(documentKey);
|
||||
|
||||
@@ -1,3 +1,8 @@
|
||||
export interface AttachmentLinkDTO {
|
||||
key: string;
|
||||
}
|
||||
|
||||
export enum AttachmentAction {
|
||||
View = 'View',
|
||||
Delete = 'Delete',
|
||||
}
|
||||
|
||||
@@ -31,17 +31,17 @@ export class DeleteAttachment {
|
||||
* @param {string} filekey
|
||||
*/
|
||||
async delete(filekey: string): Promise<void> {
|
||||
const foundDocument = await this.documentModel()
|
||||
.query()
|
||||
.findOne('key', filekey)
|
||||
.throwIfNotFound();
|
||||
|
||||
const params = {
|
||||
Bucket: this.configService.get('s3.bucket'),
|
||||
Key: filekey,
|
||||
};
|
||||
await this.s3Client.send(new DeleteObjectCommand(params));
|
||||
|
||||
const foundDocument = await this.documentModel()
|
||||
.query()
|
||||
.findOne('key', filekey)
|
||||
.throwIfNotFound();
|
||||
|
||||
await this.uow.withTransaction(async (trx: Knex.Transaction) => {
|
||||
// Delete all document links
|
||||
await this.documentLinkModel()
|
||||
|
||||
@@ -2,6 +2,8 @@ import { GetObjectCommand, S3Client } from '@aws-sdk/client-s3';
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { S3_CLIENT } from '../S3/S3.module';
|
||||
import { TenantModelProxy } from '../System/models/TenantBaseModel';
|
||||
import { DocumentModel } from './models/Document.model';
|
||||
|
||||
@Injectable()
|
||||
export class GetAttachment {
|
||||
@@ -10,13 +12,21 @@ export class GetAttachment {
|
||||
|
||||
@Inject(S3_CLIENT)
|
||||
private readonly s3: S3Client,
|
||||
|
||||
@Inject(DocumentModel.name)
|
||||
private readonly documentModel: TenantModelProxy<typeof DocumentModel>,
|
||||
) {}
|
||||
|
||||
|
||||
/**
|
||||
* Retrieves data of the given document key.
|
||||
* @param {string} filekey
|
||||
*/
|
||||
async getAttachment(filekey: string) {
|
||||
await this.documentModel()
|
||||
.query()
|
||||
.findOne('key', filekey)
|
||||
.throwIfNotFound();
|
||||
|
||||
const params = {
|
||||
Bucket: this.configService.get('s3.bucket'),
|
||||
Key: filekey,
|
||||
|
||||
@@ -24,7 +24,10 @@ export class GetAttachmentPresignedUrl {
|
||||
* @returns {string}
|
||||
*/
|
||||
async getPresignedUrl(key: string) {
|
||||
const foundDocument = await this.documentModel().query().findOne({ key });
|
||||
const foundDocument = await this.documentModel()
|
||||
.query()
|
||||
.findOne({ key })
|
||||
.throwIfNotFound();
|
||||
const config = this.configService.get('s3');
|
||||
|
||||
let ResponseContentDisposition = 'attachment';
|
||||
|
||||
@@ -16,6 +16,7 @@ import { BillAction } from "../Bills/Bills.types";
|
||||
import { AbilitySubject, ISubjectAbilitiesSchema, ISubjectAbilitySchema } from "./Roles.types";
|
||||
import { PaymentReceiveAction } from "../PaymentReceived/types/PaymentReceived.types";
|
||||
import { PreferencesAction } from "../Settings/Settings.types";
|
||||
import { AttachmentAction } from "../Attachments/Attachments.types";
|
||||
|
||||
export const AbilitySchema: ISubjectAbilitiesSchema[] = [
|
||||
{
|
||||
@@ -305,6 +306,14 @@ export const AbilitySchema: ISubjectAbilitiesSchema[] = [
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
subject: AbilitySubject.Attachment,
|
||||
subjectLabel: 'ability.attachments',
|
||||
abilities: [
|
||||
{ key: AttachmentAction.View, label: 'ability.view', default: true },
|
||||
{ key: AttachmentAction.Delete, label: 'ability.delete', default: true },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
|
||||
@@ -60,7 +60,8 @@ export enum AbilitySubject {
|
||||
CreditNote = 'CreditNode',
|
||||
VendorCredit = 'VendorCredit',
|
||||
Project = 'Project',
|
||||
TaxRate = 'TaxRate'
|
||||
TaxRate = 'TaxRate',
|
||||
Attachment = 'Attachment',
|
||||
}
|
||||
|
||||
export interface IRoleCreatedPayload {
|
||||
|
||||
Reference in New Issue
Block a user