Add policy validation checks #57

Merged
dkirillov merged 1 commit from mbiryukova/policy-engine:feature/add_policy_validation_checks into master 2024-03-11 13:22:17 +00:00
3 changed files with 64 additions and 3 deletions

View file

@ -315,6 +315,7 @@ func TestConverters(t *testing.T) {
t.Run("valid mixed iam/s3 actions", func(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[PrincipalType][]string{AWSPrincipalType: {principal}},
Effect: AllowEffect,

View file

@ -46,6 +46,8 @@ type (
PrincipalType string
)
const policyVersion = "2012-10-17"
const (
GeneralPolicyType PolicyType = iota
IdentityBasedPolicyType
@ -222,11 +224,20 @@ func (p Policy) Validate(typ PolicyType) error {
}
func (p Policy) validate() error {
if p.Version != policyVersion {
return fmt.Errorf("invalid policy version, expected '%s', actual: '%s'", policyVersion, p.Version)
}
if len(p.Statement) == 0 {
return errors.New("'Statement' is missing")
}
sids := make(map[string]struct{}, len(p.Statement))
dkirillov marked this conversation as resolved Outdated

It's better to use make(map[string]struct{}, len(p.Statement)

It's better to use `make(map[string]struct{}, len(p.Statement)`
for _, statement := range p.Statement {
if _, ok := sids[statement.SID]; ok && statement.SID != "" {
return fmt.Errorf("duplicate 'SID': %s", statement.SID)
}
sids[statement.SID] = struct{}{}
if !statement.Effect.IsValid() {
return fmt.Errorf("unknown effect: '%s'", statement.Effect)
}

View file

@ -218,6 +218,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "valid permission boundaries",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -230,6 +231,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid effect",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: "dummy",
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -242,6 +244,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid principal block",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -256,6 +259,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid not principal",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -269,6 +273,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid principal type",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -282,6 +287,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid action block",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
@ -295,6 +301,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "general invalid resource block",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Resource: []string{Wildcard},
@ -307,6 +314,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "invalid resource block",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Resource: []string{},
@ -319,6 +327,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "missing resource block",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
}},
@ -332,9 +341,43 @@ func TestValidatePolicies(t *testing.T) {
typ: GeneralPolicyType,
isValid: false,
},
{
name: "duplicate sid",
policy: Policy{
Version: policyVersion,
Statement: []Statement{
{
SID: "sid",
Effect: AllowEffect,
Action: []string{"s3:*"},
Resource: []string{Wildcard},
},
{
SID: "sid",
Effect: AllowEffect,
Action: []string{"cloudwatch:*"},
Resource: []string{Wildcard},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "missing version",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*"},
Resource: []string{Wildcard},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "identity based valid",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -348,6 +391,7 @@ func TestValidatePolicies(t *testing.T) {
name: "identity based invalid because of id presence",
policy: Policy{
ID: "some-id",
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -360,6 +404,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "identity based invalid because of principal presence",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -373,6 +418,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "identity based invalid because of not principal presence",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -386,6 +432,7 @@ func TestValidatePolicies(t *testing.T) {
{
name: "resource based valid principal",
policy: Policy{
Version: policyVersion,
Statement: []Statement{{
Effect: DenyEffect,
Action: []string{"s3:PutObject"},
@ -400,6 +447,7 @@ func TestValidatePolicies(t *testing.T) {
name: "resource based valid not principal",
policy: Policy{
ID: "some-id",
Version: policyVersion,
Statement: []Statement{{
Effect: DenyEffect,
Action: []string{"s3:PutObject"},
@ -414,6 +462,7 @@ func TestValidatePolicies(t *testing.T) {
name: "resource based invalid missing principal",
policy: Policy{
ID: "some-id",
Version: policyVersion,
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},