From 2d96dc7eaa09a6b7e266bd6e923a114a850b99f6 Mon Sep 17 00:00:00 2001
From: "Gabriela S. Soria" <gsoria@oreilly.com>
Date: Thu, 9 Nov 2023 16:27:50 -0800
Subject: [PATCH] Modify the backup vault access policies that contain
 restrictive clauses to prevent their deletion

This commit fixes the following error when trying to delete backup vault access policies for vaults (`aws/efs/automatic-backup-vault`)
automatically created when EFS backup is enabled.

```
time="2023-10-05T15:37:07Z" level=error msg="AccessDeniedException: User: arn:aws:sts::X:assumed-role/XRole/SAAssumedRoleSession is not authorized to perform: backup:DeleteBackupVaultAccessPolicy on resource: arn:aws:backup:us-east-1:X:backup-vault:aws/efs/automatic-backup-vault with an explicit deny in a resource-based policy
```

The module before attempting to delete the backup vault access policy, sets a permissive policy
to ensure the `backup:DeleteBackupVaultAccessPolicy` is allowed.

The operation to put a policy to allow `backup:DeleteBackupVaultAccessPolicy` was silently failing due to an
error:

```
The specified policy cannot be added to the vault due to cross-account sharing restrictions.
Amend the policy or the vault's settings, then retry request
```

This commit updates the policy, to use the default as a template, but excluding delete actions.

Signed-off-by: Gabriela S. Soria <gsoria@oreilly.com>
---
 resources/backup-vaults-access-policies.go | 40 ++++++++++++++--------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/resources/backup-vaults-access-policies.go b/resources/backup-vaults-access-policies.go
index 553efc662..e826062c0 100644
--- a/resources/backup-vaults-access-policies.go
+++ b/resources/backup-vaults-access-policies.go
@@ -58,7 +58,7 @@ func ListBackupVaultAccessPolicies(sess *session.Session) ([]Resource, error) {
 }
 
 func (b *BackupVaultAccessPolicy) Remove() error {
-	// Set the policy to a policy that allows deletion before removal.
+	// Set a policy that allows deletion before removal.
 	//
 	// This is required to delete the policy for the automagically created vaults
 	// such as "aws/efs/automatic-backup-vault" from EFS automatic backups
@@ -87,21 +87,31 @@ func (b *BackupVaultAccessPolicy) Remove() error {
 	//     ]
 	// }
 	//
-	// While deletion is Denied, you can update the policy with one that
-	// doesn't deny and then delete at will.
+	// Update the default policy to remove the Deny on Delete* actions
+	// and then delete the policy.
+	//
+	// Why not putting a policy that allows `backup:DeleteBackupVaultAccessPolicy` in the first place?
+	// Because that throws an error:
+	// ' The specified policy cannot be added to the vault due to cross-account sharing restrictions.
+	// Amend the policy or the vault's settings, then retry request'
+	//
 	allowDeletionPolicy := `{
-    "Version": "2012-10-17",
-    "Statement": [
-        {
-            "Effect": "Allow",
-            "Principal": {
-                "AWS": "*"
-            },
-            "Action": "backup:DeleteBackupVaultAccessPolicy",
-            "Resource": "*"
-        }
-    ]
-}`
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Effect": "Deny",
+				"Principal": {
+					"AWS": "*"
+				},
+				"Action": [
+					"backup:StartCopyJob",
+					"backup:StartRestoreJob",
+					"backup:UpdateRecoveryPointLifecycle"
+				],
+				"Resource": "*"
+			}
+		]
+	}`
 	// Ignore error from if we can't put permissive backup vault policy in for some reason, that's OK.
 	_, _ = b.svc.PutBackupVaultAccessPolicy(&backup.PutBackupVaultAccessPolicyInput{
 		BackupVaultName: &b.backupVaultName,