From 1d07331f5df5a143602062fcb4fdd74ff295d373 Mon Sep 17 00:00:00 2001
From: Denis Kirillov <d.kirillov@yadro.com>
Date: Mon, 11 Dec 2023 14:43:47 +0300
Subject: [PATCH] [#28] iam: Fix converters

Handle resource without object as bucket name instead of bucket with any object

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
---
 iam/converter.go        |  2 +-
 iam/converter_native.go |  2 +-
 iam/converter_s3.go     |  2 +-
 iam/converter_test.go   | 55 +++++++++++++++++++++++++++++++++++++++--
 4 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/iam/converter.go b/iam/converter.go
index b288de3..4749526 100644
--- a/iam/converter.go
+++ b/iam/converter.go
@@ -256,7 +256,7 @@ func parseResourceAsS3ARN(resource string) (bucket string, object string, err er
 	s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix)
 	sepIndex := strings.Index(s3Resource, "/")
 	if sepIndex < 0 {
-		return s3Resource, Wildcard, nil
+		return s3Resource, "", nil
 	}
 
 	bucket = s3Resource[:sepIndex]
diff --git a/iam/converter_native.go b/iam/converter_native.go
index 4abfcdf..7ddcd99 100644
--- a/iam/converter_native.go
+++ b/iam/converter_native.go
@@ -177,7 +177,7 @@ func formNativeResourceNamesAndConditions(names []string, resolver NativeResolve
 		}
 		resource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID)
 
-		if obj == Wildcard {
+		if obj == Wildcard || obj == "" {
 			combined = append(combined, resource)
 			continue
 		}
diff --git a/iam/converter_s3.go b/iam/converter_s3.go
index 094c3d3..5380097 100644
--- a/iam/converter_s3.go
+++ b/iam/converter_s3.go
@@ -149,7 +149,7 @@ func formS3ResourceNames(names []string) ([]string, error) {
 			return nil, err
 		}
 
-		if bkt == Wildcard {
+		if bkt == Wildcard || obj == "" {
 			res[i] = bkt
 			continue
 		}
diff --git a/iam/converter_test.go b/iam/converter_test.go
index 7ec177d..f73106d 100644
--- a/iam/converter_test.go
+++ b/iam/converter_test.go
@@ -1072,6 +1072,57 @@ func TestComplexS3Conditions(t *testing.T) {
 	}
 }
 
+func TestS3BucketResource(t *testing.T) {
+	namespace := "ns"
+	bktName1, bktName2 := "bucket1", "bucket2"
+	chainName := chain.Name("name")
+
+	mockResolver := newMockUserResolver([]string{}, []string{bktName1})
+
+	p := Policy{
+		Version: "2012-10-17",
+		Statement: []Statement{
+			{
+				Principal: map[PrincipalType][]string{Wildcard: nil},
+				Effect:    DenyEffect,
+				Action:    []string{"s3:HeadBucket"},
+				Resource:  []string{"arn:aws:s3:::" + bktName1},
+			},
+			{
+				Principal: map[PrincipalType][]string{Wildcard: nil},
+				Effect:    AllowEffect,
+				Action:    []string{"*"},
+				Resource:  []string{"arn:aws:s3:::*"},
+			},
+		},
+	}
+
+	s3Chain, err := ConvertToS3Chain(p, mockResolver)
+	require.NoError(t, err)
+
+	s := inmemory.NewInMemory()
+	_, _, err = s.MorphRuleChainStorage().AddMorphRuleChain(chainName, engine.NamespaceTarget(namespace), s3Chain)
+	require.NoError(t, err)
+
+	// check we match just "bucket1" resource
+	req := testutil.NewRequest("HeadBucket", testutil.NewResource(bktName1, nil), nil)
+	status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	require.NoError(t, err)
+	require.Equal(t, chain.AccessDenied.String(), status.String())
+
+	// check we match just "bucket2" resource
+	req = testutil.NewRequest("HeadBucket", testutil.NewResource(bktName2, nil), nil)
+	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	require.NoError(t, err)
+	require.Equal(t, chain.Allow.String(), status.String())
+
+	// check we also match "bucket2/object" resource
+	req = testutil.NewRequest("PutObject", testutil.NewResource(bktName2+"/object", nil), nil)
+	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	require.NoError(t, err)
+	require.Equal(t, chain.Allow.String(), status.String())
+}
+
 func TestWildcardConverters(t *testing.T) {
 	policy := `{"Version":"2012-10-17","Statement":{"Effect":"Allow", "Principal": "*", "Action":"*","Resource":"*"}}`
 
@@ -1223,7 +1274,7 @@ func TestResourceParsing(t *testing.T) {
 		{
 			resource:       "arn:aws:s3:::bkt",
 			expectedBucket: "bkt",
-			expectedObject: "*",
+			expectedObject: "",
 		},
 		{
 			resource:       "arn:aws:s3:::bkt/",
@@ -1233,7 +1284,7 @@ func TestResourceParsing(t *testing.T) {
 		{
 			resource:       "arn:aws:s3:::*",
 			expectedBucket: "*",
-			expectedObject: "*",
+			expectedObject: "",
 		},
 		{
 			resource:       "*",