From 062a72bdaf89651e702046465235499e78bb1e17 Mon Sep 17 00:00:00 2001
From: Mark Ramsden <mark.ramsden5@nhs.net>
Date: Wed, 7 May 2025 14:00:02 +0100
Subject: [PATCH] CCM-9554: fix s3 permissions for public keys

---
 .../module_s3bucket_public_signing_keys.tf    | 75 +------------------
 .../s3_bucket_policy_public_signing_keys.tf   | 10 +--
 2 files changed, 5 insertions(+), 80 deletions(-)

diff --git a/infrastructure/terraform/modules/public-signing-keys/module_s3bucket_public_signing_keys.tf b/infrastructure/terraform/modules/public-signing-keys/module_s3bucket_public_signing_keys.tf
index 9cdb532b..8ddf5ea5 100644
--- a/infrastructure/terraform/modules/public-signing-keys/module_s3bucket_public_signing_keys.tf
+++ b/infrastructure/terraform/modules/public-signing-keys/module_s3bucket_public_signing_keys.tf
@@ -39,7 +39,7 @@ module "s3bucket_public_signing_keys" {
   ]
 
   policy_documents = [
-    data.aws_iam_policy_document.s3bucket_public_keys.json
+    data.aws_iam_policy_document.bucket_policy_public_signing_keys.json
   ]
 
   public_access = {
@@ -55,79 +55,6 @@ module "s3bucket_public_signing_keys" {
   }
 }
 
-data "aws_iam_policy_document" "s3bucket_public_keys" {
-  statement {
-    sid    = "DontAllowNonSecureConnection"
-    effect = "Deny"
-
-    actions = [
-      "s3:*",
-    ]
-
-    resources = [
-      module.s3bucket_public_signing_keys.arn,
-      "${module.s3bucket_public_signing_keys.arn}/*",
-    ]
-
-    principals {
-      type = "AWS"
-
-      identifiers = [
-        "*",
-      ]
-    }
-
-    condition {
-      test     = "Bool"
-      variable = "aws:SecureTransport"
-
-      values = [
-        "false",
-      ]
-    }
-  }
-
-  statement {
-    sid    = "AllowManagedAccountsToList"
-    effect = "Allow"
-
-    actions = [
-      "s3:ListBucket",
-    ]
-
-    resources = [
-      module.s3bucket_public_signing_keys.arn,
-    ]
-
-    principals {
-      type = "AWS"
-      identifiers = [
-        "arn:aws:iam::${var.aws_account_id}:root"
-      ]
-    }
-  }
-
-  statement {
-    sid    = "AllowManagedAccountsToGet"
-    effect = "Allow"
-
-    actions = [
-      "s3:GetObject",
-    ]
-
-    resources = [
-      "${module.s3bucket_public_signing_keys.arn}/*",
-    ]
-
-    principals {
-      type = "AWS"
-      identifiers = [
-        "arn:aws:iam::${var.aws_account_id}:root"
-      ]
-    }
-  }
-}
-
 resource "aws_s3_bucket_cors_configuration" "public_public_keys" {
   bucket = module.s3bucket_public_signing_keys.bucket
 
diff --git a/infrastructure/terraform/modules/public-signing-keys/s3_bucket_policy_public_signing_keys.tf b/infrastructure/terraform/modules/public-signing-keys/s3_bucket_policy_public_signing_keys.tf
index 68d1cae6..3aa1ec5e 100644
--- a/infrastructure/terraform/modules/public-signing-keys/s3_bucket_policy_public_signing_keys.tf
+++ b/infrastructure/terraform/modules/public-signing-keys/s3_bucket_policy_public_signing_keys.tf
@@ -1,10 +1,6 @@
-resource "aws_s3_bucket_policy" "public_signing_keys" {
-  bucket = module.s3bucket_public_signing_keys.id
-  policy = data.aws_iam_policy_document.bucket_policy_public_signing_keys.json
-}
-
 data "aws_iam_policy_document" "bucket_policy_public_signing_keys" {
   statement {
+    sid     = "AllowManagedAccountsToRead"
     actions = ["s3:GetObject", "s3:ListBucket"]
     resources = [
       module.s3bucket_public_signing_keys.arn,
@@ -18,8 +14,9 @@ data "aws_iam_policy_document" "bucket_policy_public_signing_keys" {
   }
 
   dynamic "statement" {
-    for_each = var.deploy_cdn ? [1]: []
+    for_each = var.deploy_cdn ? [1] : []
     content {
+      sid     = "AllowCDNAccess"
       actions = ["s3:GetObject", "s3:ListBucket"]
       resources = [
         module.s3bucket_public_signing_keys.arn,
@@ -34,6 +31,7 @@ data "aws_iam_policy_document" "bucket_policy_public_signing_keys" {
   }
 
   statement {
+    sid     = "DontAllowNonSecureConnection"
     effect  = "Deny"
     actions = ["s3:*"]
     resources = [