CCM-23434: Add DL getLetterData permissions - #684
Conversation
masl2
changed the title
There was a problem hiding this comment.
🔵 Needs a closer look
The getLetterData Lambda policy adds access to a new S3 bucket but is missing the corresponding KMS permissions needed for SSE-KMS encrypted objects in the Digital Letters bucket.
Pull request overview
This PR updates the Supplier API Terraform component to grant the getLetterData Lambda access to the Digital Letters PDF storage bucket, using new variables to identify the Digital Letters AWS account and environment.
Changes:
- Add Terraform inputs for
digital_letters_account_idanddigital_letters_environment. - Extend the
getLetterDataLambda IAM policy to include S3 access to the Digital Letters PDF bucket ARN (and object ARNs). - Add locals for the Digital Letters bucket ARN and KMS key alias naming.
File summaries
| File | Description |
|---|---|
| infrastructure/terraform/components/api/variables.tf | Adds new input variables for Digital Letters account/environment. |
| infrastructure/terraform/components/api/README.md | Updates Terraform-docs inputs list to include the new variables. |
| infrastructure/terraform/components/api/module_lambda_get_letter_data.tf | Extends IAM policy S3 permissions for getLetterData to cover the Digital Letters bucket. |
| infrastructure/terraform/components/api/locals.tf | Defines Digital Letters bucket ARN and KMS alias locals for use in policy/resources. |
Review details
Suppressed comments (1)
infrastructure/terraform/components/api/module_lambda_get_letter_data.tf:94
- Digital Letters S3 bucket access is being added to the IAM policy, but there is no corresponding KMS permission statement for the Digital Letters SSE-KMS key. If the DL bucket is encrypted with its KMS key (as implied by the new
digital_letters_s3_kms_key_alias_namelocal), the presigned GET will fail with KMS access denied even though S3 permissions are granted.
"${local.digital_letters_pdf_bucket_arn}/*",
]
}
statement {
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The change is a focused IAM policy update to support Digital Letters S3/KMS access, with only a minor non-blocking comment noted about an inline comment’s accuracy.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
|
Description
Use new TF variables identifying associated DL account and environment to grant appropriate bucket permissions for the getLetterData function
Context
GetLetterData is a supplier api function to allow suppliers to retrived PDFs for printing. This lambda's prinicpal requires permissions for the storage location of the PDFs such that when it generates a pre-signed URL, said URL has the appropriate bucket permissions
Type of changes
Checklist
DT3-Specific Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.