CCM-9872: Copy file to download bucket (#457)

chris-elliott-nhsd · web-flow · commit dba4debf9ba8 · 2025-05-13T09:53:04.000+01:00
diff --git a/infrastructure/terraform/components/sandbox/README.md b/infrastructure/terraform/components/sandbox/README.md
@@ -31,6 +31,7 @@ No requirements.
 | <a name="output_api_base_url"></a> [api\_base\_url](#output\_api\_base\_url) | n/a |
 | <a name="output_cognito_user_pool_client_id"></a> [cognito\_user\_pool\_client\_id](#output\_cognito\_user\_pool\_client\_id) | n/a |
 | <a name="output_cognito_user_pool_id"></a> [cognito\_user\_pool\_id](#output\_cognito\_user\_pool\_id) | n/a |
+| <a name="output_download_bucket_name"></a> [download\_bucket\_name](#output\_download\_bucket\_name) | n/a |
 | <a name="output_internal_bucket_name"></a> [internal\_bucket\_name](#output\_internal\_bucket\_name) | n/a |
 | <a name="output_quarantine_bucket_name"></a> [quarantine\_bucket\_name](#output\_quarantine\_bucket\_name) | n/a |
 | <a name="output_send_proof_queue_url"></a> [send\_proof\_queue\_url](#output\_send\_proof\_queue\_url) | n/a |
diff --git a/infrastructure/terraform/components/sandbox/outputs.tf b/infrastructure/terraform/components/sandbox/outputs.tf
@@ -34,6 +34,10 @@ output "quarantine_bucket_name" {
   value = module.backend_api.quarantine_bucket_name
 }
 
+output "download_bucket_name" {
+  value = module.backend_api.download_bucket_name
+}
+
 output "sftp_poll_lambda_name" {
   value = module.backend_api.sftp_poll_lambda_name
 }
diff --git a/infrastructure/terraform/modules/backend-api/README.md b/infrastructure/terraform/modules/backend-api/README.md
@@ -64,6 +64,7 @@ No requirements.
 | Name | Description |
 |------|-------------|
 | <a name="output_api_base_url"></a> [api\_base\_url](#output\_api\_base\_url) | n/a |
+| <a name="output_download_bucket_name"></a> [download\_bucket\_name](#output\_download\_bucket\_name) | n/a |
 | <a name="output_internal_bucket_name"></a> [internal\_bucket\_name](#output\_internal\_bucket\_name) | n/a |
 | <a name="output_quarantine_bucket_name"></a> [quarantine\_bucket\_name](#output\_quarantine\_bucket\_name) | n/a |
 | <a name="output_send_proof_queue_url"></a> [send\_proof\_queue\_url](#output\_send\_proof\_queue\_url) | n/a |
diff --git a/infrastructure/terraform/modules/backend-api/locals.tf b/infrastructure/terraform/modules/backend-api/locals.tf
@@ -47,6 +47,7 @@ locals {
     TEMPLATES_EVENT_SOURCE           = local.event_source
     TEMPLATES_INTERNAL_BUCKET_NAME   = module.s3bucket_internal.id
     TEMPLATES_QUARANTINE_BUCKET_NAME = module.s3bucket_quarantine.id
+    TEMPLATES_DOWNLOAD_BUCKET_NAME   = module.s3bucket_download.id
     TEMPLATES_TABLE_NAME             = aws_dynamodb_table.templates.name
     ENABLE_PROOFING                  = var.enable_proofing
   }
diff --git a/infrastructure/terraform/modules/backend-api/module_lambda_process_proof.tf b/infrastructure/terraform/modules/backend-api/module_lambda_process_proof.tf
@@ -114,4 +114,18 @@ data "aws_iam_policy_document" "process_proof" {
 
     resources = ["${module.s3bucket_internal.arn}/*"]
   }
+
+  statement {
+    sid    = "AllowS3DownloadWrite"
+    effect = "Allow"
+
+    actions = [
+      "s3:PutObject",
+      "s3:PutObjectVersion",
+      "s3:PutObjectTagging",
+      "s3:PutObjectVersionTagging",
+    ]
+
+    resources = ["${module.s3bucket_download.arn}/*"]
+  }
 }
diff --git a/infrastructure/terraform/modules/backend-api/outputs.tf b/infrastructure/terraform/modules/backend-api/outputs.tf
@@ -26,6 +26,10 @@ output "quarantine_bucket_name" {
   value = module.s3bucket_quarantine.id
 }
 
+output "download_bucket_name" {
+  value = module.s3bucket_download.id
+}
+
 output "sftp_poll_lambda_name" {
   value = module.lambda_sftp_poll.function_name
 }
diff --git a/lambdas/backend-api/src/__tests__/templates/api/process-proof.test.ts b/lambdas/backend-api/src/__tests__/templates/api/process-proof.test.ts
@@ -8,22 +8,34 @@ test.each([
   [
     'NO_THREATS_FOUND',
     'PASSED',
-    (letterFileRepository: LetterFileRepository) =>
+    (letterFileRepository: LetterFileRepository) => {
       expect(
         letterFileRepository.copyFromQuarantineToInternal
       ).toHaveBeenCalledWith(
         'proofs/template-id/proof.pdf',
         'version-id',
         'proofs/template-owner/template-id/proof.pdf'
-      ),
+      );
+      expect(
+        letterFileRepository.copyFromQuarantineToDownload
+      ).toHaveBeenCalledWith(
+        'proofs/template-id/proof.pdf',
+        'version-id',
+        'template-owner/proofs/template-id/proof.pdf'
+      );
+    },
   ],
   [
     'THREATS_FOUND',
     'FAILED',
-    (letterFileRepository) =>
+    (letterFileRepository) => {
       expect(
         letterFileRepository.copyFromQuarantineToInternal
-      ).not.toHaveBeenCalled(),
+      ).not.toHaveBeenCalled();
+      expect(
+        letterFileRepository.copyFromQuarantineToDownload
+      ).not.toHaveBeenCalled();
+    },
   ],
 ])(
   'calls dependencies as expected for a %s virus scan',
diff --git a/lambdas/backend-api/src/__tests__/templates/infra/letter-file-repository.test.ts b/lambdas/backend-api/src/__tests__/templates/infra/letter-file-repository.test.ts
@@ -12,7 +12,8 @@ const setup = () => {
 
   const letterFileRepository = new LetterFileRepository(
     'quarantine-bucket',
-    'internal-bucket'
+    'internal-bucket',
+    'download-bucket'
   );
 
   return { letterFileRepository, mocks: { s3Client } };
@@ -39,6 +40,27 @@ describe('LetterUploadRepository', () => {
     });
   });
 
+  describe('copyFromQuarantineToDownload', () => {
+    it('copies pdf template files from quarantine to download', async () => {
+      const { letterFileRepository, mocks } = setup();
+
+      await letterFileRepository.copyFromQuarantineToDownload(
+        's3/object/key',
+        's3-object-version',
+        's3/download/key'
+      );
+
+      expect(mocks.s3Client).toHaveReceivedCommandWith(CopyObjectCommand, {
+        Bucket: 'download-bucket',
+        CopySource:
+          '/quarantine-bucket/s3/object/key?versionId=s3-object-version',
+        Key: 's3/download/key',
+        MetadataDirective: 'COPY',
+        TaggingDirective: 'COPY',
+      });
+    });
+  });
+
   describe('deleteFromQuarantine', () => {
     it('deletes files from quarantine', async () => {
       const { letterFileRepository, mocks } = setup();
diff --git a/lambdas/backend-api/src/__tests__/templates/infra/letter-upload-repository.test.ts b/lambdas/backend-api/src/__tests__/templates/infra/letter-upload-repository.test.ts
@@ -18,7 +18,8 @@ const setup = () => {
 
   const letterUploadRepository = new LetterUploadRepository(
     quarantineBucketName,
-    internalBucketName
+    internalBucketName,
+    'download-bucket'
   );
 
   return { letterUploadRepository, mocks: { s3Client } };
diff --git a/lambdas/backend-api/src/templates/api/process-proof.ts b/lambdas/backend-api/src/templates/api/process-proof.ts
@@ -36,11 +36,24 @@ export const createHandler =
       scanResultStatus === 'NO_THREATS_FOUND' ? 'PASSED' : 'FAILED';
 
     if (virusScanResult === 'PASSED') {
-      await letterFileRepository.copyFromQuarantineToInternal(
-        objectKey,
-        versionId,
-        internalKey
+      const downloadKey = LetterProofRepository.getDownloadKey(
+        owner,
+        templateId,
+        fileName
       );
+
+      await Promise.all([
+        letterFileRepository.copyFromQuarantineToInternal(
+          objectKey,
+          versionId,
+          internalKey
+        ),
+        letterFileRepository.copyFromQuarantineToDownload(
+          objectKey,
+          versionId,
+          downloadKey
+        ),
+      ]);
     } else {
       logger.error({
         description: 'File found that did not pass virus scan',
diff --git a/lambdas/backend-api/src/templates/container.ts b/lambdas/backend-api/src/templates/container.ts
@@ -29,7 +29,8 @@ export function createContainer() {
 
   const letterUploadRepository = new LetterUploadRepository(
     config.quarantineBucket,
-    config.internalBucket
+    config.internalBucket,
+    config.downloadBucket
   );
 
   const proofingQueue = new ProofingQueue(
@@ -54,11 +55,12 @@ export function createContainer() {
 }
 
 export const createLetterFileRepositoryContainer = () => {
-  const { quarantineBucket, internalBucket } = loadConfig();
+  const { quarantineBucket, internalBucket, downloadBucket } = loadConfig();
 
   const letterFileRepository = new LetterFileRepository(
     quarantineBucket,
-    internalBucket
+    internalBucket,
+    downloadBucket
   );
 
   return {
diff --git a/lambdas/backend-api/src/templates/infra/config.ts b/lambdas/backend-api/src/templates/infra/config.ts
@@ -8,6 +8,7 @@ const $Env = z.object({
   REQUEST_PROOF_QUEUE_URL: z.string(),
   TEMPLATES_INTERNAL_BUCKET_NAME: z.string(),
   TEMPLATES_QUARANTINE_BUCKET_NAME: z.string(),
+  TEMPLATES_DOWNLOAD_BUCKET_NAME: z.string(),
   TEMPLATES_TABLE_NAME: z.string(),
 });
 
@@ -21,6 +22,7 @@ export function loadConfig() {
     environment: env.ENVIRONMENT,
     internalBucket: env.TEMPLATES_INTERNAL_BUCKET_NAME,
     quarantineBucket: env.TEMPLATES_QUARANTINE_BUCKET_NAME,
+    downloadBucket: env.TEMPLATES_DOWNLOAD_BUCKET_NAME,
     requestProofQueueUrl: env.REQUEST_PROOF_QUEUE_URL,
     templatesTableName: env.TEMPLATES_TABLE_NAME,
   };
diff --git a/lambdas/backend-api/src/templates/infra/letter-file-repository.ts b/lambdas/backend-api/src/templates/infra/letter-file-repository.ts
@@ -9,18 +9,35 @@ export class LetterFileRepository {
 
   constructor(
     protected readonly quarantineBucketName: string,
-    protected readonly internalBucketName: string
+    protected readonly internalBucketName: string,
+    protected readonly downloadBucketName: string
   ) {}
 
   async copyFromQuarantineToInternal(
     key: string,
     versionId: string,
     destinationKey?: string
+  ) {
+    await this.copy(
+      this.quarantineBucketName,
+      this.internalBucketName,
+      key,
+      versionId,
+      destinationKey
+    );
+  }
+
+  private async copy(
+    sourceBucket: string,
+    destinationBucket: string,
+    key: string,
+    versionId: string,
+    destinationKey?: string
   ) {
     await this.client.send(
       new CopyObjectCommand({
-        CopySource: `/${this.quarantineBucketName}/${key}?versionId=${versionId}`,
-        Bucket: this.internalBucketName,
+        CopySource: `/${sourceBucket}/${key}?versionId=${versionId}`,
+        Bucket: destinationBucket,
         Key: destinationKey ?? key,
         MetadataDirective: 'COPY',
         TaggingDirective: 'COPY',
@@ -37,4 +54,18 @@ export class LetterFileRepository {
       })
     );
   }
+
+  async copyFromQuarantineToDownload(
+    key: string,
+    versionId: string,
+    destinationKey: string
+  ) {
+    await this.copy(
+      this.quarantineBucketName,
+      this.downloadBucketName,
+      key,
+      versionId,
+      destinationKey
+    );
+  }
 }
diff --git a/lambdas/backend-api/src/templates/infra/letter-proof-repository.ts b/lambdas/backend-api/src/templates/infra/letter-proof-repository.ts
@@ -28,4 +28,8 @@ export class LetterProofRepository extends LetterFileRepository {
   static getInternalKey(owner: string, templateId: string, fileName: string) {
     return `proofs/${owner}/${templateId}/${fileName}.pdf`;
   }
+
+  static getDownloadKey(owner: string, templateId: string, fileName: string) {
+    return `${owner}/proofs/${templateId}/${fileName}.pdf`;
+  }
 }
diff --git a/tests/test-team/global.d.ts b/tests/test-team/global.d.ts
@@ -12,6 +12,7 @@ declare global {
       TEMPLATES_TABLE_NAME: string;
       TEMPLATES_INTERNAL_BUCKET_NAME: string;
       TEMPLATES_QUARANTINE_BUCKET_NAME: string;
+      TEMPLATES_DOWNLOAD_BUCKET_NAME: string;
     }
   }
 
diff --git a/tests/test-team/helpers/db/template-storage-helper.ts b/tests/test-team/helpers/db/template-storage-helper.ts
@@ -258,21 +258,10 @@ export class TemplateStorageHelper {
     version: string,
     ext: string
   ) {
-    try {
-      return await this.s3.send(
-        new HeadObjectCommand({
-          Bucket: bucket,
-          Key: this.letterFileKey(prefix, key, version, ext),
-          ChecksumMode: 'ENABLED',
-        })
-      );
-    } catch (error) {
-      if (error instanceof NotFound) {
-        return null;
-      }
-
-      throw error;
-    }
+    return await this.getS3Metadata(
+      bucket,
+      this.letterFileKey(prefix, key, version, ext)
+    );
   }
 
   async getLetterProofMetadata(
@@ -282,11 +271,18 @@ export class TemplateStorageHelper {
     version: string,
     ext: string
   ) {
+    return await this.getS3Metadata(
+      bucket,
+      `${prefix}/${templateId}/${version}.${ext}`
+    );
+  }
+
+  async getS3Metadata(bucket: string, key: string) {
     try {
       return await this.s3.send(
         new HeadObjectCommand({
           Bucket: bucket,
-          Key: `${prefix}/${templateId}/${version}.${ext}`,
+          Key: key,
           ChecksumMode: 'ENABLED',
         })
       );
diff --git a/tests/test-team/template-mgmt-e2e-tests/template-mgmt-proofing.e2e.spec.ts b/tests/test-team/template-mgmt-e2e-tests/template-mgmt-proofing.e2e.spec.ts
@@ -117,6 +117,15 @@ test.describe('Letter Proofing', () => {
         expect(internalPdf?.ChecksumSHA256).toEqual(
           pdfUploadFixtures.noCustomPersonalisation.pdf.checksumSha256()
         );
+
+        const downloadPdf = await templateStorageHelper.getS3Metadata(
+          process.env.TEMPLATES_DOWNLOAD_BUCKET_NAME,
+          `${user.userId}/proofs/${templateId}/${fileName}.pdf`
+        );
+
+        expect(downloadPdf?.ChecksumSHA256).toEqual(
+          pdfUploadFixtures.noCustomPersonalisation.pdf.checksumSha256()
+        );
       }
     }).toPass({ timeout: 60_000 });
   });
diff --git a/utils/backend-config/src/backend-config.ts b/utils/backend-config/src/backend-config.ts
@@ -11,6 +11,7 @@ export type BackendConfig = {
   templatesTableName: string;
   templatesInternalBucketName: string;
   templatesQuarantineBucketName: string;
+  templatesDownloadBucketName: string;
   userPoolId: string;
   userPoolClientId: string;
 };
@@ -27,6 +28,8 @@ export const BackendConfigHelper = {
         process.env.TEMPLATES_INTERNAL_BUCKET_NAME ?? '',
       templatesQuarantineBucketName:
         process.env.TEMPLATES_QUARANTINE_BUCKET_NAME ?? '',
+      templatesDownloadBucketName:
+        process.env.TEMPLATES_DOWNLOAD_BUCKET_NAME ?? '',
       userPoolId: process.env.USER_POOL_ID ?? '',
       userPoolClientId: process.env.USER_POOL_CLIENT_ID ?? '',
       sftpPollLambdaName: process.env.SFTP_POLL_LAMBDA_NAME ?? '',
@@ -45,6 +48,8 @@ export const BackendConfigHelper = {
       config.templatesInternalBucketName;
     process.env.TEMPLATES_QUARANTINE_BUCKET_NAME =
       config.templatesQuarantineBucketName;
+    process.env.TEMPLATES_DOWNLOAD_BUCKET_NAME =
+      config.templatesDownloadBucketName;
     process.env.SFTP_POLL_LAMBDA_NAME = config.sftpPollLambdaName;
   },
 
@@ -62,6 +67,8 @@ export const BackendConfigHelper = {
         outputsFileContent.internal_bucket_name?.value ?? '',
       templatesQuarantineBucketName:
         outputsFileContent.quarantine_bucket_name?.value ?? '',
+      templatesDownloadBucketName:
+        outputsFileContent.download_bucket_name?.value ?? '',
       userPoolId: outputsFileContent.cognito_user_pool_id?.value ?? '',
       userPoolClientId:
         outputsFileContent.cognito_user_pool_client_id?.value ?? '',

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,10 @@ output "quarantine_bucket_name" {`
`34`	`34`	`value = module.backend_api.quarantine_bucket_name`
`35`	`35`	`}`
`36`	`36`
	`37`	`+output "download_bucket_name" {`
	`38`	`+ value = module.backend_api.download_bucket_name`
	`39`	`+}`
	`40`	`+`
`37`	`41`	`output "sftp_poll_lambda_name" {`
`38`	`42`	`value = module.backend_api.sftp_poll_lambda_name`
`39`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,7 @@ locals {`
`47`	`47`	`TEMPLATES_EVENT_SOURCE = local.event_source`
`48`	`48`	`TEMPLATES_INTERNAL_BUCKET_NAME = module.s3bucket_internal.id`
`49`	`49`	`TEMPLATES_QUARANTINE_BUCKET_NAME = module.s3bucket_quarantine.id`
	`50`	`+ TEMPLATES_DOWNLOAD_BUCKET_NAME = module.s3bucket_download.id`
`50`	`51`	`TEMPLATES_TABLE_NAME = aws_dynamodb_table.templates.name`
`51`	`52`	`ENABLE_PROOFING = var.enable_proofing`
`52`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,10 @@ output "quarantine_bucket_name" {`
`26`	`26`	`value = module.s3bucket_quarantine.id`
`27`	`27`	`}`
`28`	`28`
	`29`	`+output "download_bucket_name" {`
	`30`	`+ value = module.s3bucket_download.id`
	`31`	`+}`
	`32`	`+`
`29`	`33`	`output "sftp_poll_lambda_name" {`
`30`	`34`	`value = module.lambda_sftp_poll.function_name`
`31`	`35`	`}`