[#36] iam: Keep s3/iam prefixes in resources

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

iam/converter.go

@@ -244,50 +244,38 @@ func parsePrincipalAsIAMUser(principal string) (account string, user string, err
 	return account, user, nil
 }
 
-func parseResourceAsS3ARN(resource string) (bucket string, object string, err error) {
+func validateResource(resource string) error {
 	if resource == Wildcard {
-		return Wildcard, Wildcard, nil
+		return nil
 	}
 
-	if !strings.HasPrefix(resource, s3ResourcePrefix) {
-		return "", "", ErrInvalidResourceFormat
+	if !strings.HasPrefix(resource, s3ResourcePrefix) && !strings.HasPrefix(resource, arnIAMPrefix) {
+		return ErrInvalidResourceFormat
 	}
 
-	// iam arn format arn:aws:s3:::<bucket-name>/<object-name>
-	s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix)
-	sepIndex := strings.Index(s3Resource, "/")
-	if sepIndex < 0 {
-		return s3Resource, "", nil
+	index := strings.IndexByte(resource, Wildcard[0])
+	if index != -1 && index != utf8.RuneCountInString(resource)-1 {
+		return ErrInvalidResourceFormat
 	}
 
-	bucket = s3Resource[:sepIndex]
-	object = s3Resource[sepIndex+1:]
-	if len(object) == 0 {
-		return bucket, Wildcard, nil
-	}
-
-	if bucket == Wildcard && object != Wildcard {
-		return "", "", ErrInvalidResourceFormat
-	}
-
-	return bucket, object, nil
+	return nil
 }
 
-func parseActionAsS3Action(action string) (string, error) {
+func validateAction(action string) error {
 	if action == Wildcard {
-		return Wildcard, nil
+		return nil
 	}
 
 	if !strings.HasPrefix(action, s3ActionPrefix) && !strings.HasPrefix(action, iamActionPrefix) {
-		return "", ErrInvalidActionFormat
+		return ErrInvalidActionFormat
 	}
 
 	index := strings.IndexByte(action, Wildcard[0])
 	if index != -1 && index != utf8.RuneCountInString(action)-1 {
-		return "", ErrInvalidActionFormat
+		return ErrInvalidActionFormat
 	}
 
-	return action, nil
+	return nil
 }
 
 func splitGroupedConditions(groupedConditions []GroupedConditions) [][]chain.Condition {

iam/converter_native.go

@@ -161,30 +161,50 @@ func formNativeResourceNamesAndConditions(names []string, resolver NativeResolve
 
 	var combined []string
 
-	for i := range names {
-		bkt, obj, err := parseResourceAsS3ARN(names[i])
-		if err != nil {
+	for _, resource := range names {
+		if err := validateResource(resource); err != nil {
 			return nil, err
 		}
 
-		if bkt == Wildcard {
+		if resource == Wildcard {
 			res = res[:0]
 			return append(res, GroupedResources{Names: []string{native.ResourceFormatAllObjects}}), nil
 		}
 
+		if !strings.HasPrefix(resource, s3ResourcePrefix) {
+			continue
+		}
+
+		var bkt, obj string
+		s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix)
+		if s3Resource == Wildcard {
+			res = res[:0]
+			return append(res, GroupedResources{Names: []string{native.ResourceFormatAllObjects}}), nil
+		}
+
+		if sepIndex := strings.Index(s3Resource, "/"); sepIndex < 0 {
+			bkt = s3Resource
+		} else {
+			bkt = s3Resource[:sepIndex]
+			obj = s3Resource[sepIndex+1:]
+			if len(obj) == 0 {
+				obj = Wildcard
+			}
+		}
+
 		cnrID, err := resolver.GetBucketCID(bkt)
 		if err != nil {
 			return nil, err
 		}
-		resource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID)
+		nativeResource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID)
 
 		if obj == Wildcard || obj == "" {
-			combined = append(combined, resource)
+			combined = append(combined, nativeResource)
 			continue
 		}
 
 		res = append(res, GroupedResources{
-			Names: []string{resource},
+			Names: []string{nativeResource},
 			Conditions: []chain.Condition{
 				{
 					Op:     chain.CondStringLike,
@@ -233,14 +253,23 @@ func formPrincipalKey(principal string, resolver NativeResolver) (string, error)
 func formNativeActionNames(names []string) ([]string, error) {
 	res := make([]string, 0, len(names))
 
-	for i := range names {
-		action, err := parseActionAsS3Action(names[i])
-		if err != nil {
+	for _, action := range names {
+		if err := validateAction(action); err != nil {
 			return nil, err
 		}
-		if action == Wildcard || strings.TrimPrefix(action, s3ActionPrefix) == Wildcard {
+
+		if action == Wildcard {
 			return []string{Wildcard}, nil
 		}
+
+		if !strings.HasPrefix(action, s3ActionPrefix) {
+			continue
+		}
+
+		if strings.TrimPrefix(action, s3ActionPrefix) == Wildcard {
+			return []string{Wildcard}, nil
+		}
+
 		res = append(res, actionToOpMap[action]...)
 	}
 

iam/converter_s3.go

@@ -21,19 +21,17 @@ func ConvertToS3Chain(p Policy, resolver S3Resolver) (*chain.Chain, error) {
 	for _, statement := range p.Statement {
 		status := formStatus(statement)
 
-		action, actionInverted := statement.action()
-		s3Actions, err := formS3ActionNames(action)
-		if err != nil {
+		actions, actionInverted := statement.action()
+		if err := validateS3ActionNames(actions); err != nil {
 			return nil, err
 		}
-		ruleAction := chain.Actions{Inverted: actionInverted, Names: s3Actions}
+		ruleAction := chain.Actions{Inverted: actionInverted, Names: actions}
 
-		resource, resourceInverted := statement.resource()
-		s3Resources, err := formS3ResourceNames(resource)
-		if err != nil {
+		resources, resourceInverted := statement.resource()
+		if err := validateS3ResourceNames(resources); err != nil {
 			return nil, err
 		}
-		ruleResource := chain.Resources{Inverted: resourceInverted, Names: s3Resources}
+		ruleResource := chain.Resources{Inverted: resourceInverted, Names: resources}
 
 		groupedConditions, err := convertToS3ChainCondition(statement.Conditions, resolver)
 		if err != nil {
@@ -141,33 +139,22 @@ func formPrincipalOwner(principal string, resolver S3Resolver) (string, error) {
 	return address, nil
 }
 
-func formS3ResourceNames(names []string) ([]string, error) {
-	res := make([]string, len(names))
+func validateS3ResourceNames(names []string) error {
 	for i := range names {
-		bkt, obj, err := parseResourceAsS3ARN(names[i])
-		if err != nil {
-			return nil, err
-		}
-
-		if bkt == Wildcard || obj == "" {
-			res[i] = bkt
-			continue
-		}
-
-		res[i] = bkt + "/" + obj
-	}
-
-	return res, nil
-}
-
-func formS3ActionNames(names []string) ([]string, error) {
-	var err error
-	res := make([]string, len(names))
-	for i := range names {
-		if res[i], err = parseActionAsS3Action(names[i]); err != nil {
-			return nil, err
+		if err := validateResource(names[i]); err != nil {
+			return err
 		}
 	}
 
-	return res, nil
+	return nil
+}
+
+func validateS3ActionNames(names []string) error {
+	for i := range names {
+		if err := validateAction(names[i]); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }

iam/converter_test.go

@@ -69,7 +69,7 @@ func TestConverters(t *testing.T) {
 	principal := "arn:aws:iam::" + namespace + ":user/" + userName
 	bktName := "DOC-EXAMPLE-BUCKET"
 	objName := "object-name"
-	resource := bktName + "/*"
+	resource := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName)
 	s3action := "s3:PutObject"
 
 	mockResolver := newMockUserResolver([]string{user}, []string{bktName})
@@ -83,7 +83,7 @@ func TestConverters(t *testing.T) {
 				},
 				Effect:   AllowEffect,
 				Action:   []string{"s3:PutObject"},
-				Resource: []string{"arn:aws:s3:::" + resource},
+				Resource: []string{resource},
 				Conditions: map[string]Condition{
 					CondStringEquals: {
 						"s3:RequestObjectTag/Department": {"Finance"},
@@ -128,7 +128,7 @@ func TestConverters(t *testing.T) {
 				},
 				Effect:   AllowEffect,
 				Action:   []string{"s3:PutObject"},
-				Resource: []string{"arn:aws:s3:::" + resource},
+				Resource: []string{resource},
 			}},
 		}
 
@@ -162,7 +162,7 @@ func TestConverters(t *testing.T) {
 				},
 				Effect:      DenyEffect,
 				NotAction:   []string{"s3:PutObject"},
-				NotResource: []string{"arn:aws:s3:::" + resource},
+				NotResource: []string{resource},
 			}},
 		}
 
@@ -196,7 +196,7 @@ func TestConverters(t *testing.T) {
 				},
 				Effect:      DenyEffect,
 				NotAction:   []string{"s3:GetObject"},
-				NotResource: []string{"arn:aws:s3:::" + bktName + "/" + objName},
+				NotResource: []string{fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName, objName)},
 			}},
 		}
 
@@ -892,9 +892,9 @@ func TestComplexS3Conditions(t *testing.T) {
 	principal2 := "arn:aws:iam::" + namespace + ":user/" + userName2
 	bktName1, bktName2, bktName3 := "bktName", "bktName2", "bktName3"
 	objName1 := "objName1"
-	resource1 := bktName1 + "/" + objName1
-	resource2 := bktName2 + "/*"
-	resource3 := bktName3 + "/*"
+	resource1 := fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName1, objName1)
+	resource2 := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName2)
+	resource3 := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName3)
 	action := "s3:PutObject"
 
 	key1, key2 := "key1", "key2"
@@ -910,7 +910,7 @@ func TestComplexS3Conditions(t *testing.T) {
 			},
 			Effect:   DenyEffect,
 			Action:   []string{action},
-			Resource: []string{"arn:aws:s3:::" + resource1, "arn:aws:s3:::" + resource2, "arn:aws:s3:::" + resource3},
+			Resource: []string{resource1, resource2, resource3},
 			Conditions: map[string]Condition{
 				CondStringEquals: {key1: {val0, val1}},
 				CondStringLike:   {key2: {val2}},
@@ -1001,7 +1001,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "bucket resource3, all conditions matched",
 			action:   action,
-			resource: bktName3 + "/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName3, "some-obj"),
 			requestMap: map[string]string{
 				s3.PropertyKeyOwner: mockResolver.users[user1],
 				key1:                val0,
@@ -1012,7 +1012,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "bucket resource, user condition mismatched",
 			action:   action,
-			resource: bktName2 + "/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "some-obj"),
 			requestMap: map[string]string{
 				key1: val0,
 				key2: val2,
@@ -1022,7 +1022,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "bucket resource, key2 condition mismatched",
 			action:   action,
-			resource: bktName3 + "/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName3, "some-obj"),
 			requestMap: map[string]string{
 				s3.PropertyKeyOwner: mockResolver.users[user1],
 				key1:                val0,
@@ -1033,7 +1033,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "bucket resource, key1 condition mismatched",
 			action:   action,
-			resource: bktName2 + "/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "some-obj"),
 			requestMap: map[string]string{
 				s3.PropertyKeyOwner: mockResolver.users[user1],
 				key2:                val2,
@@ -1054,7 +1054,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "bucket/object resource, resource mismatched",
 			action:   action,
-			resource: bktName1 + "/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName1, "some-obj"),
 			requestMap: map[string]string{
 				s3.PropertyKeyOwner: mockResolver.users[user1],
 				key1:                val0,
@@ -1097,7 +1097,7 @@ func TestComplexS3Conditions(t *testing.T) {
 		{
 			name:     "resource mismatched",
 			action:   action,
-			resource: "some-bkt/some-obj",
+			resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, "some-bkt", "some-obj"),
 			requestMap: map[string]string{
 				s3.PropertyKeyOwner: mockResolver.users[user1],
 				key1:                val0,
@@ -1120,7 +1120,7 @@ func TestS3BucketResource(t *testing.T) {
 	bktName1, bktName2 := "bucket1", "bucket2"
 	chainName := chain.Name("name")
 
-	mockResolver := newMockUserResolver([]string{}, []string{bktName1})
+	mockResolver := newMockUserResolver([]string{}, []string{})
 
 	p := Policy{
 		Version: "2012-10-17",
@@ -1129,13 +1129,13 @@ func TestS3BucketResource(t *testing.T) {
 				Principal: map[PrincipalType][]string{Wildcard: nil},
 				Effect:    DenyEffect,
 				Action:    []string{"s3:HeadBucket"},
-				Resource:  []string{"arn:aws:s3:::" + bktName1},
+				Resource:  []string{fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName1)},
 			},
 			{
 				Principal: map[PrincipalType][]string{Wildcard: nil},
 				Effect:    AllowEffect,
 				Action:    []string{"*"},
-				Resource:  []string{"arn:aws:s3:::*"},
+				Resource:  []string{s3.ResourceFormatS3All},
 			},
 		},
 	}
@@ -1148,19 +1148,19 @@ func TestS3BucketResource(t *testing.T) {
 	require.NoError(t, err)
 
 	// check we match just "bucket1" resource
-	req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(bktName1, nil), nil)
+	req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, 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("s3:HeadBucket", testutil.NewResource(bktName2, nil), nil)
+	req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, 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("s3:PutObject", testutil.NewResource(bktName2+"/object", nil), nil)
+	req = testutil.NewRequest("s3:PutObject", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3BucketObject, 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())
@@ -1182,58 +1182,46 @@ func TestWildcardConverters(t *testing.T) {
 
 func TestActionParsing(t *testing.T) {
 	for _, tc := range []struct {
-		action   string
-		expected string
-		err      bool
+		action string
+		err    bool
 	}{
 		{
-			action:   "withoutPrefix",
-			expected: "",
-			err:      true,
+			action: "withoutPrefix",
+			err:    true,
 		},
 		{
-			action:   "s3:*Object",
-			expected: "",
-			err:      true,
+			action: "s3:*Object",
+			err:    true,
 		},
 		{
-			action:   "*",
-			expected: "*",
+			action: "*",
 		},
 		{
-			action:   "s3:PutObject",
-			expected: "s3:PutObject",
+			action: "s3:PutObject",
 		},
 		{
-			action:   "s3:Put*",
-			expected: "s3:Put*",
+			action: "s3:Put*",
 		},
 		{
-			action:   "s3:*",
-			expected: "s3:*",
+			action: "s3:*",
 		},
 		{
-			action:   "s3:",
-			expected: "s3:",
+			action: "s3:",
 		},
 		{
-			action:   "iam:ListAccessKeys",
-			expected: "iam:ListAccessKeys",
+			action: "iam:ListAccessKeys",
 		},
 		{
-			action:   "iam:*",
-			expected: "iam:*",
+			action: "iam:*",
 		},
 	} {
 		t.Run("", func(t *testing.T) {
-			actual, err := parseActionAsS3Action(tc.action)
+			err := validateAction(tc.action)
 			if tc.err {
 				require.Error(t, err)
-				return
+			} else {
+				require.NoError(t, err)
 			}
-
-			require.NoError(t, err)
-			require.Equal(t, tc.expected, actual)
 		})
 	}
 }
@@ -1303,55 +1291,24 @@ func TestPrincipalParsing(t *testing.T) {
 
 func TestResourceParsing(t *testing.T) {
 	for _, tc := range []struct {
-		resource       string
-		expectedBucket string
-		expectedObject string
-		err            bool
+		resource string
+		err      bool
 	}{
-		{
-			resource: "withoutPrefixAnd",
-			err:      true,
-		},
-		{
-			resource: "arn:aws:s3:::*/obj",
-			err:      true,
-		},
-		{
-			resource:       "arn:aws:s3:::bkt/*",
-			expectedBucket: "bkt",
-			expectedObject: "*",
-		},
-		{
-			resource:       "arn:aws:s3:::bkt",
-			expectedBucket: "bkt",
-			expectedObject: "",
-		},
-		{
-			resource:       "arn:aws:s3:::bkt/",
-			expectedBucket: "bkt",
-			expectedObject: "*",
-		},
-		{
-			resource:       "arn:aws:s3:::*",
-			expectedBucket: "*",
-			expectedObject: "",
-		},
-		{
-			resource:       "*",
-			expectedBucket: "*",
-			expectedObject: "*",
-		},
+		{resource: "withoutPrefixAnd", err: true},
+		{resource: "arn:aws:s3:::*/obj", err: true},
+		{resource: "arn:aws:s3:::bkt/*"},
+		{resource: "arn:aws:s3:::bkt"},
+		{resource: "arn:aws:s3:::bkt/"},
+		{resource: "arn:aws:s3:::*"},
+		{resource: "*"},
 	} {
 		t.Run("", func(t *testing.T) {
-			bkt, obj, err := parseResourceAsS3ARN(tc.resource)
+			err := validateResource(tc.resource)
 			if tc.err {
 				require.Error(t, err)
-				return
+			} else {
+				require.NoError(t, err)
 			}
-
-			require.NoError(t, err)
-			require.Equal(t, tc.expectedBucket, bkt)
-			require.Equal(t, tc.expectedObject, obj)
 		})
 	}
 }

schema/s3/consts.go

@@ -6,4 +6,12 @@ const (
 	PropertyKeyDelimiter = "s3:delimiter"
 	PropertyKeyPrefix    = "s3:prefix"
 	PropertyKeyVersionID = "s3:VersionId"
+
+	ResourceFormatS3All           = "arn:aws:s3:::*"
+	ResourceFormatS3Bucket        = "arn:aws:s3:::%s"
+	ResourceFormatS3BucketObjects = "arn:aws:s3:::%s/*"
+	ResourceFormatS3BucketObject  = "arn:aws:s3:::%s/%s"
+
+	ResourceFormatIAMNamespaceUser  = "arn:aws:iam::%s:user/%s"
+	ResourceFormatIAMNamespaceGroup = "arn:aws:iam::%s:group/%s"
 )