[#22] iam: Fix converters

Validate that actions and resources contain wildcard only at the end

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

iam/converter.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 )
@@ -63,6 +64,9 @@ var (
 
 	// ErrInvalidResourceFormat occurs when resource has unknown/unsupported format.
 	ErrInvalidResourceFormat = errors.New("invalid resource format")
+
+	// ErrInvalidActionFormat occurs when action has unknown/unsupported format.
+	ErrInvalidActionFormat = errors.New("invalid action format")
 )
 
 type formPrincipalConditionFunc func(string) chain.Condition
@@ -240,6 +244,10 @@ func parsePrincipalAsIAMUser(principal string) (account string, user string, err
 }
 
 func parseResourceAsS3ARN(resource string) (bucket string, object string, err error) {
+	if resource == Wildcard {
+		return Wildcard, Wildcard, nil
+	}
+
 	if !strings.HasPrefix(resource, s3ResourcePrefix) {
 		return "", "", ErrInvalidResourceFormat
 	}
@@ -264,6 +272,26 @@ func parseResourceAsS3ARN(resource string) (bucket string, object string, err er
 	return bucket, object, nil
 }
 
+func parseActionAsS3Action(action string) (string, error) {
+	if action == Wildcard {
+		return Wildcard, nil
+	}
+
+	if !strings.HasPrefix(action, s3ActionPrefix) {
+		return "", ErrInvalidActionFormat
+	}
+
+	// iam arn format :s3:<action-name>
+	s3Action := strings.TrimPrefix(action, s3ActionPrefix)
+
+	index := strings.IndexByte(s3Action, Wildcard[0])
+	if index != -1 && index != utf8.RuneCountInString(s3Action)-1 {
+		return "", ErrInvalidActionFormat
+	}
+
+	return s3Action, nil
+}
+
 func splitGroupedConditions(groupedConditions []GroupedConditions) [][]chain.Condition {
 	var orConditions []chain.Condition
 	commonConditions := make([]chain.Condition, 0, len(groupedConditions))

iam/converter_native.go

@@ -3,7 +3,6 @@ package iam
 import (
 	"errors"
 	"fmt"
-	"strings"
 
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 	"git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
@@ -46,7 +45,11 @@ func ConvertToNativeChain(p Policy, resolver NativeResolver) (*chain.Chain, erro
 		status := formStatus(statement)
 
 		action, actionInverted := statement.action()
-		ruleAction := chain.Actions{Inverted: actionInverted, Names: formNativeActionNames(action)}
+		nativeActions, err := formNativeActionNames(action)
+		if err != nil {
+			return nil, err
+		}
+		ruleAction := chain.Actions{Inverted: actionInverted, Names: nativeActions}
 		if len(ruleAction.Names) == 0 {
 			continue
 		}
@@ -226,16 +229,19 @@ func formPrincipalKey(principal string, resolver NativeResolver) (string, error)
 	return key, nil
 }
 
-func formNativeActionNames(names []string) []string {
+func formNativeActionNames(names []string) ([]string, error) {
 	res := make([]string, 0, len(names))
 
 	for i := range names {
-		trimmed := strings.TrimPrefix(names[i], s3ActionPrefix)
-		if trimmed == Wildcard {
-			return []string{Wildcard}
+		action, err := parseActionAsS3Action(names[i])
+		if err != nil {
+			return nil, err
 		}
-		res = append(res, actionToOpMap[trimmed]...)
+		if action == Wildcard {
+			return []string{Wildcard}, nil
+		}
+		res = append(res, actionToOpMap[action]...)
 	}
 
-	return res
+	return res, nil
 }

iam/converter_s3.go

@@ -2,7 +2,6 @@ package iam
 
 import (
 	"fmt"
-	"strings"
 
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 	"git.frostfs.info/TrueCloudLab/policy-engine/schema/s3"
@@ -23,10 +22,18 @@ func ConvertToS3Chain(p Policy, resolver S3Resolver) (*chain.Chain, error) {
 		status := formStatus(statement)
 
 		action, actionInverted := statement.action()
-		ruleAction := chain.Actions{Inverted: actionInverted, Names: formS3ActionNames(action)}
+		s3Actions, err := formS3ActionNames(action)
+		if err != nil {
+			return nil, err
+		}
+		ruleAction := chain.Actions{Inverted: actionInverted, Names: s3Actions}
 
 		resource, resourceInverted := statement.resource()
-		ruleResource := chain.Resources{Inverted: resourceInverted, Names: formS3ResourceNamesAndConditions(resource)}
+		s3Resources, err := formS3ResourceNames(resource)
+		if err != nil {
+			return nil, err
+		}
+		ruleResource := chain.Resources{Inverted: resourceInverted, Names: s3Resources}
 
 		groupedConditions, err := convertToS3ChainCondition(statement.Conditions, resolver)
 		if err != nil {
@@ -134,20 +141,33 @@ func formPrincipalOwner(principal string, resolver S3Resolver) (string, error) {
 	return address, nil
 }
 
-func formS3ResourceNamesAndConditions(names []string) []string {
+func formS3ResourceNames(names []string) ([]string, error) {
 	res := make([]string, len(names))
 	for i := range names {
-		res[i] = strings.TrimPrefix(names[i], s3ResourcePrefix)
+		bkt, obj, err := parseResourceAsS3ARN(names[i])
+		if err != nil {
+			return nil, err
 		}
 
-	return res
+		if bkt == Wildcard {
+			res[i] = bkt
+			continue
 		}
 
-func formS3ActionNames(names []string) []string {
+		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 {
-		res[i] = strings.TrimPrefix(names[i], s3ActionPrefix)
+		if res[i], err = parseActionAsS3Action(names[i]); err != nil {
+			return nil, err
+		}
 	}
 
-	return res
+	return res, nil
 }

iam/converter_test.go

@@ -1,6 +1,7 @@
 package iam
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strconv"
@@ -1071,6 +1072,189 @@ func TestComplexS3Conditions(t *testing.T) {
 	}
 }
 
+func TestWildcardConverters(t *testing.T) {
+	policy := `{"Version":"2012-10-17","Statement":{"Effect":"Allow", "Principal": "*", "Action":"*","Resource":"*"}}`
+
+	var p Policy
+	err := json.Unmarshal([]byte(policy), &p)
+	require.NoError(t, err)
+
+	_, err = ConvertToS3Chain(p, newMockUserResolver(nil, nil))
+	require.NoError(t, err)
+
+	_, err = ConvertToNativeChain(p, newMockUserResolver(nil, nil))
+	require.NoError(t, err)
+}
+
+func TestActionParsing(t *testing.T) {
+	for _, tc := range []struct {
+		action   string
+		expected string
+		err      bool
+	}{
+		{
+			action:   "withoutPrefix",
+			expected: "",
+			err:      true,
+		},
+		{
+			action:   "s3:*Object",
+			expected: "",
+			err:      true,
+		},
+		{
+			action:   "*",
+			expected: "*",
+		},
+		{
+			action:   "s3:PutObject",
+			expected: "PutObject",
+		},
+		{
+			action:   "s3:Put*",
+			expected: "Put*",
+		},
+		{
+			action:   "s3:*",
+			expected: "*",
+		},
+		{
+			action: "s3:",
+
+			expected: "",
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			actual, err := parseActionAsS3Action(tc.action)
+			if tc.err {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}
+
+func TestPrincipalParsing(t *testing.T) {
+	for _, tc := range []struct {
+		principal       string
+		expectedAccount string
+		expectedUser    string
+		err             bool
+	}{
+		{
+			principal: "withoutPrefix",
+			err:       true,
+		},
+		{
+			principal: "*",
+			err:       true,
+		},
+		{
+			principal: "arn:aws:iam:::dummy",
+			err:       true,
+		},
+		{
+			principal: "arn:aws:iam::",
+			err:       true,
+		},
+		{
+			principal: "arn:aws:iam:::dummy/test",
+			err:       true,
+		},
+		{
+			principal: "arn:aws:iam:::user/",
+			err:       true,
+		},
+		{
+			principal: "arn:aws:iam:::user/user/",
+			err:       true,
+		},
+		{
+			principal:    "arn:aws:iam:::user/name",
+			expectedUser: "name",
+		},
+		{
+			principal:    "arn:aws:iam:::user/path/name",
+			expectedUser: "name",
+		},
+		{
+			principal:       "arn:aws:iam::root:user/path/name",
+			expectedAccount: "root",
+			expectedUser:    "name",
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			account, user, err := parsePrincipalAsIAMUser(tc.principal)
+			if tc.err {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedAccount, account)
+			require.Equal(t, tc.expectedUser, user)
+		})
+	}
+}
+
+func TestResourceParsing(t *testing.T) {
+	for _, tc := range []struct {
+		resource       string
+		expectedBucket string
+		expectedObject 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: "*",
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			bkt, obj, err := parseResourceAsS3ARN(tc.resource)
+			if tc.err {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedBucket, bkt)
+			require.Equal(t, tc.expectedObject, obj)
+		})
+	}
+}
+
 func requireChainRulesMatch(t *testing.T, expected, actual []chain.Rule) {
 	require.Equal(t, len(expected), len(actual), "length of chain rules differ")