diff --git a/pkg/iam/policy/policy.go b/pkg/iam/policy/policy.go index 38585cd18..a4ec9faea 100644 --- a/pkg/iam/policy/policy.go +++ b/pkg/iam/policy/policy.go @@ -106,6 +106,10 @@ func (iamp Policy) isValid() error { for i := range iamp.Statements { for _, statement := range iamp.Statements[i+1:] { + if iamp.Statements[i].Effect != statement.Effect { + continue + } + actions := iamp.Statements[i].Actions.Intersection(statement.Actions) if len(actions) == 0 { continue diff --git a/pkg/iam/policy/policy_test.go b/pkg/iam/policy/policy_test.go index cf0e87089..806dbc6ef 100644 --- a/pkg/iam/policy/policy_test.go +++ b/pkg/iam/policy/policy_test.go @@ -338,6 +338,24 @@ func TestPolicyIsValid(t *testing.T) { }, } + case8Policy := Policy{ + Version: DefaultVersion, + Statements: []Statement{ + NewStatement( + policy.Allow, + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "/myobject*")), + condition.NewFunctions(), + ), + NewStatement( + policy.Allow, + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "/myobject*")), + condition.NewFunctions(), + ), + }, + } + testCases := []struct { policy Policy expectErr bool @@ -353,8 +371,10 @@ func TestPolicyIsValid(t *testing.T) { {case5Policy, true}, // Invalid statement error. {case6Policy, true}, - // Duplicate statement error. - {case7Policy, true}, + // Duplicate statement different Effects. + {case7Policy, false}, + // Duplicate statement same Effects. + {case8Policy, true}, } for i, testCase := range testCases { @@ -921,6 +941,25 @@ func TestPolicyUnmarshalJSON(t *testing.T) { ] }`) + case11Policy := Policy{ + ID: "MyPolicyForMyBucket1", + Version: DefaultVersion, + Statements: []Statement{ + NewStatement( + policy.Allow, + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "myobject*")), + condition.NewFunctions(), + ), + NewStatement( + policy.Deny, + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "myobject*")), + condition.NewFunctions(), + ), + }, + } + testCases := []struct { data []byte expectedResult Policy @@ -938,8 +977,8 @@ func TestPolicyUnmarshalJSON(t *testing.T) { {case9Data, Policy{}, true}, // Duplicate statement error. {case10Data, Policy{}, true}, - // Duplicate statement error (Effect differs). - {case11Data, Policy{}, true}, + // Duplicate statement success (Effect differs). + {case11Data, case11Policy, false}, } for i, testCase := range testCases { diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index 38d374e27..13931c299 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -88,6 +88,10 @@ func (policy Policy) isValid() error { for i := range policy.Statements { for _, statement := range policy.Statements[i+1:] { + if policy.Statements[i].Effect != statement.Effect { + continue + } + principals := policy.Statements[i].Principal.Intersection(statement.Principal) if principals.IsEmpty() { continue diff --git a/pkg/policy/policy_test.go b/pkg/policy/policy_test.go index d2200e2cf..e5312f7ad 100644 --- a/pkg/policy/policy_test.go +++ b/pkg/policy/policy_test.go @@ -356,6 +356,26 @@ func TestPolicyIsValid(t *testing.T) { }, } + case8Policy := Policy{ + Version: DefaultVersion, + Statements: []Statement{ + NewStatement( + Allow, + NewPrincipal("*"), + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "/myobject*")), + condition.NewFunctions(), + ), + NewStatement( + Allow, + NewPrincipal("*"), + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "/myobject*")), + condition.NewFunctions(), + ), + }, + } + testCases := []struct { policy Policy expectErr bool @@ -371,8 +391,10 @@ func TestPolicyIsValid(t *testing.T) { {case5Policy, true}, // Invalid statement error. {case6Policy, true}, + // Duplicate statement success different effects. + {case7Policy, false}, // Duplicate statement error. - {case7Policy, true}, + {case8Policy, true}, } for i, testCase := range testCases { @@ -988,6 +1010,27 @@ func TestPolicyUnmarshalJSON(t *testing.T) { ] }`) + case11Policy := Policy{ + ID: "MyPolicyForMyBucket1", + Version: DefaultVersion, + Statements: []Statement{ + NewStatement( + Allow, + NewPrincipal("*"), + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "myobject*")), + condition.NewFunctions(), + ), + NewStatement( + Deny, + NewPrincipal("*"), + NewActionSet(PutObjectAction), + NewResourceSet(NewResource("mybucket", "myobject*")), + condition.NewFunctions(), + ), + }, + } + testCases := []struct { data []byte expectedResult Policy @@ -1005,8 +1048,8 @@ func TestPolicyUnmarshalJSON(t *testing.T) { {case9Data, Policy{}, true}, // Duplicate statement error. {case10Data, Policy{}, true}, - // Duplicate statement error (Effect differs). - {case11Data, Policy{}, true}, + // Duplicate statement success (Effect differs). + {case11Data, case11Policy, false}, } for i, testCase := range testCases {