Better iam policies (#3530)

aws-cloudformation · Jul 24, 2024 · c6b8148 · c6b8148
1 parent 32545a7
commit c6b8148
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 89 deletions.
diff --git a/src/cfnlint/data/schemas/other/iam/policy.json b/src/cfnlint/data/schemas/other/iam/policy.json
@@ -3,32 +3,66 @@
  "additionalProperties": false,
  "definitions": {
   "Action": {
-   "anyOf": [
-    {
-     "$ref": "#/definitions/Wildcard"
-    },
-    {
-     "$ref": "#/definitions/StringOrStringArray"
-    }
-   ],
    "cfnLint": [
     "AWS::IAM::Policy/Properties/PolicyDocument/Statement/Action"
+   ],
+   "items": {
+    "type": "string"
+   },
+   "type": [
+    "string",
+    "array"
    ]
   },
   "AwsArn": {
-   "pattern": "(^arn:(aws|aws-cn|aws-us-gov):[^:]+:[^:]*(:(?:\\d{12}|\\*|aws)?:.+|)|\\*)$",
-   "type": "string"
+   "pattern": "(^arn:(aws|aws-cn|aws-us-gov):[^:]+:[^:]*(:(?:\\d{12}|\\*|aws)?:.+|)|\\*)$"
   },
   "AwsPrincipalArn": {
    "anyOf": [
     {
-     "pattern": "^((arn:(aws|aws-cn|aws-us-gov):iam::\\d{12}:(?:root|user|group|role)|\\*)|\\d{12})"
+     "const": "*"
+    },
+    {
+     "pattern": "^\\d{12}$"
+    },
+    {
+     "pattern": "^arn:(aws|aws-cn|aws-us-gov):iam::\\d{12}:(?:root|user|group|role)"
     },
     {
      "pattern": "^arn:(aws|aws-cn|aws-us-gov):iam::cloudfront:user/.+$"
     }
-   ],
-   "type": "string"
+   ]
+  },
+  "Boolean": {
+   "enum": [
+    "true",
+    "false",
+    true,
+    false
+   ]
+  },
+  "Booleans": {
+   "if": {
+    "type": [
+     "string",
+     "boolean"
+    ]
+   },
+   "items": {
+    "$ref": "#/definitions/Boolean",
+    "type": [
+     "string",
+     "boolean"
+    ]
+   },
+   "then": {
+    "$ref": "#/definitions/Boolean"
+   },
+   "type": [
+    "string",
+    "array",
+    "boolean"
+   ]
   },
   "Condition": {
    "patternProperties": {
@@ -114,14 +148,7 @@
     },
     "Null": {
      "additionalProperties": {
-      "scalarOrArray": {
-       "enum": [
-        "true",
-        "false",
-        true,
-        false
-       ]
-      }
+      "$ref": "#/definitions/Booleans"
      },
      "type": "object"
     }
@@ -130,22 +157,23 @@
   },
   "ConditionSetValue": {
    "additionalProperties": {
-    "$ref": "#/definitions/StringArray"
+    "items": {
+     "type": "string"
+    },
+    "type": "array"
    },
    "type": "object"
   },
   "ConditionValue": {
    "additionalProperties": {
-    "anyOf": [
-     {
-      "$ref": "#/definitions/StringOrStringArray"
-     },
-     {
-      "type": "boolean"
-     },
-     {
-      "type": "number"
-     }
+    "items": {
+     "type": "string"
+    },
+    "type": [
+     "boolean",
+     "number",
+     "string",
+     "array"
     ]
    },
    "type": "object"
@@ -156,20 +184,42 @@
    },
    "properties": {
     "AWS": {
-     "scalarOrArray": {
-      "$ref": "#/definitions/AwsPrincipalArn"
-     }
+     "$ref": "#/definitions/AwsPrincipalArn",
+     "items": {
+      "$ref": "#/definitions/AwsPrincipalArn",
+      "type": "string"
+     },
+     "type": [
+      "string",
+      "array"
+     ]
     },
     "CanonicalUser": {
-     "$ref": "#/definitions/StringOrStringArray"
+     "items": {
+      "type": "string"
+     },
+     "type": [
+      "string",
+      "array"
+     ]
     },
     "Federated": {
-     "$ref": "#/definitions/StringOrStringArray"
+     "items": {
+      "type": "string"
+     },
+     "type": [
+      "string",
+      "array"
+     ]
     },
     "Service": {
-     "scalarOrArray": {
+     "items": {
       "type": "string"
-     }
+     },
+     "type": [
+      "string",
+      "array"
+     ]
     }
    },
    "then": {
@@ -181,9 +231,15 @@
    ]
   },
   "Resource": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/AwsArn"
-   }
+   "$ref": "#/definitions/AwsArn",
+   "items": {
+    "$ref": "#/definitions/AwsArn",
+    "type": "string"
+   },
+   "type": [
+    "string",
+    "array"
+   ]
   },
   "Statement": {
    "additionalProperties": false,
@@ -252,16 +308,7 @@
    },
    "required": [
     "Effect"
-   ],
-   "type": "object"
-  },
-  "StringArray": {
-   "type": "string"
-  },
-  "StringOrStringArray": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/StringArray"
-   }
+   ]
   },
   "Wildcard": {
    "const": "*"
@@ -272,9 +319,15 @@
    "type": "string"
   },
   "Statement": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/Statement"
-   }
+   "$ref": "#/definitions/Statement",
+   "items": {
+    "$ref": "#/definitions/Statement",
+    "type": "object"
+   },
+   "type": [
+    "object",
+    "array"
+   ]
   },
   "Version": {
    "cfnLint": [

diff --git a/src/cfnlint/data/schemas/other/iam/policy_identity.json b/src/cfnlint/data/schemas/other/iam/policy_identity.json
@@ -45,18 +45,23 @@
     "Sid": {
      "$ref": "policy#/definitions/Statement/properties/Sid"
     }
-   },
-   "type": "object"
+   }
   }
  },
  "properties": {
   "Id": {
    "$ref": "policy#/properties/Id"
   },
   "Statement": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/Statement"
-   }
+   "$ref": "#/definitions/Statement",
+   "items": {
+    "$ref": "#/definitions/Statement",
+    "type": "object"
+   },
+   "type": [
+    "object",
+    "array"
+   ]
   },
   "Version": {
    "$ref": "policy#/properties/Version"

diff --git a/src/cfnlint/data/schemas/other/iam/policy_resource.json b/src/cfnlint/data/schemas/other/iam/policy_resource.json
@@ -57,18 +57,23 @@
     "Sid": {
      "$ref": "policy#/definitions/Statement/properties/Sid"
     }
-   },
-   "type": "object"
+   }
   }
  },
  "properties": {
   "Id": {
    "$ref": "policy#/properties/Id"
   },
   "Statement": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/Statement"
-   }
+   "$ref": "#/definitions/Statement",
+   "items": {
+    "$ref": "#/definitions/Statement",
+    "type": "object"
+   },
+   "type": [
+    "object",
+    "array"
+   ]
   },
   "Version": {
    "$ref": "policy#/properties/Version"

diff --git a/src/cfnlint/data/schemas/other/iam/policy_resource_ecr.json b/src/cfnlint/data/schemas/other/iam/policy_resource_ecr.json
@@ -51,18 +51,23 @@
     "Sid": {
      "$ref": "policy#/definitions/Statement/properties/Sid"
     }
-   },
-   "type": "object"
+   }
   }
  },
  "properties": {
   "Id": {
    "$ref": "policy#/properties/Id"
   },
   "Statement": {
-   "scalarOrArray": {
-    "$ref": "#/definitions/Statement"
-   }
+   "$ref": "#/definitions/Statement",
+   "items": {
+    "$ref": "#/definitions/Statement",
+    "type": "object"
+   },
+   "type": [
+    "object",
+    "array"
+   ]
   },
   "Version": {
    "$ref": "policy#/properties/Version"

diff --git a/src/cfnlint/rules/resources/iam/Policy.py b/src/cfnlint/rules/resources/iam/Policy.py
@@ -14,15 +14,6 @@
 from cfnlint.schema.resolver import RefResolver
 
 
-# pylint: disable=unused-argument
-def _scalar_or_array(validator: Validator, subschema: Any, instance: Any, schema: Any):
-    if validator.is_type(instance, "array"):
-        for index, i in enumerate(instance):
-            yield from validator.descend(i, subschema, path=index)
-    else:
-        yield from validator.descend(instance, subschema)
-
-
 class Policy(CfnLintJsonSchema):
     """Check IAM policies"""
 
@@ -61,28 +52,22 @@ def validate(
         # so we can run this now
         if validator.is_type(policy, "string"):
             try:
-                iam_validator = validator.extend(
-                    validators={
-                        "scalarOrArray": _scalar_or_array,
-                    },
-                )(schema=self.identity_schema).evolve(
+                iam_validator = validator.evolve(
                     context=validator.context.evolve(
                         functions=[],
                     ),
                     resolver=self.resolver,
+                    schema=self.identity_schema,
                 )
                 policy = json.loads(policy)
             except json.JSONDecodeError:
                 return
         else:
-            iam_validator = validator.extend(
-                validators={
-                    "scalarOrArray": _scalar_or_array,
-                },
-            )(schema=self.identity_schema).evolve(
+            iam_validator = validator.evolve(
                 cfn=validator.cfn,
                 context=validator.context,
                 resolver=self.resolver,
+                schema=self.identity_schema,
             )
 
         for err in iam_validator.iter_errors(policy):