From c2e7c94f9c67a7565f74249cab48d869ef061949 Mon Sep 17 00:00:00 2001
From: Elim Tsiagbey <elimty02@gmail.com>
Date: Wed, 24 Jul 2024 11:48:14 -0400
Subject: [PATCH] fix: Append list error for permission validation (#48)

* fix: Append list error for permission validation

Instead of returning the error right away, append the error when `ListObject` is successful but the key expected if not found. Set the `Read` permission state as `Unknown` if the response from the list calls does not find the expected key.

This can be caused if the credentials do not have `s3:PutObject` but have `s3:ListBucket`. Without `s3:PutObject`, the object will not be written to S3 so other calls to find the object will not be successful even if they have valid permissions.

This prevents us from returning a `write` error on Boundary because the list call will always fail. Without this, the  Permissions are not wrapped and the SBC states are all set to `unknown` even though we know certain permission failed.
---
 plugin/service/storage/plugin.go      |  3 ++-
 plugin/service/storage/plugin_test.go | 37 +++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/plugin/service/storage/plugin.go b/plugin/service/storage/plugin.go
index ac1818c..f973678 100644
--- a/plugin/service/storage/plugin.go
+++ b/plugin/service/storage/plugin.go
@@ -820,7 +820,8 @@ func dryRunValidation(ctx context.Context, state *awsStoragePersistedState, attr
 		st, permissions.Read = errors.ParseAWSError(err, "failed to list object")
 		errs = multierror.Append(errs, st.Err())
 	} else if res == nil || len(res.Contents) != 1 || *res.Contents[0].Key != objectKey {
-		return status.New(codes.Aborted, fmt.Sprintf("list response did not contain the expected key: %+v", res))
+		permissions.Read = &pb.Permission{State: pb.StateType_STATE_TYPE_UNKNOWN, CheckedAt: timestamppb.Now()}
+		errs = multierror.Append(errs, status.New(codes.Aborted, fmt.Sprintf("list response did not contain the expected key: %+v", res)).Err())
 	}
 
 	if _, err := client.DeleteObject(ctx, &s3.DeleteObjectInput{
diff --git a/plugin/service/storage/plugin_test.go b/plugin/service/storage/plugin_test.go
index 5674fb7..108899e 100644
--- a/plugin/service/storage/plugin_test.go
+++ b/plugin/service/storage/plugin_test.go
@@ -2666,6 +2666,43 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) {
 				),
 			},
 		},
+		{
+			name:     "dryRunValidation failed putObject with missing list expected key",
+			req:      validRequest(),
+			credOpts: validSTSMock(),
+			storageOpts: []awsStoragePersistedStateOption{
+				withTestS3APIFunc(newTestMockS3(
+					nil,
+					testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "PutObject", "not authorized")),
+					testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}),
+					testMockS3WithHeadObjectOutput(&s3.HeadObjectOutput{}),
+					testMockS3WithListObjectsV2OutputFunc(func(i *s3.ListObjectsV2Input) *s3.ListObjectsV2Output {
+						return &s3.ListObjectsV2Output{
+							Contents: []s3types.Object{},
+						}
+					}),
+				)),
+			},
+			expectedErrContains: "aws service s3: invalid credentials error: failed to put object",
+			expectedErrCode:     codes.FailedPrecondition,
+			expectedDetails: &pb.StorageBucketCredentialState{
+				State: &pb.Permissions{
+					Read: &pb.Permission{
+						State:     pb.StateType_STATE_TYPE_UNKNOWN,
+						CheckedAt: timestamppb.Now(),
+					},
+					Write: &pb.Permission{
+						State:        pb.StateType_STATE_TYPE_ERROR,
+						ErrorDetails: "not authorized",
+						CheckedAt:    timestamppb.Now(),
+					},
+					Delete: &pb.Permission{
+						State:     pb.StateType_STATE_TYPE_OK,
+						CheckedAt: timestamppb.Now(),
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range cases {