Skip to content

Commit f28d4cc

Browse files
authored
Update policy principal validation logic (#3400)
* Change up the IAM policy validation * Add testing for CDK and sub not join
1 parent ed39a19 commit f28d4cc

File tree

5 files changed

+201
-23
lines changed

5 files changed

+201
-23
lines changed

Diff for: src/cfnlint/data/schemas/other/iam/policy.json

+25-23
Original file line numberDiff line numberDiff line change
@@ -151,31 +151,33 @@
151151
"type": "object"
152152
},
153153
"Principal": {
154-
"anyOf": [
155-
{
156-
"$ref": "#/definitions/Wildcard"
154+
"if": {
155+
"type": "string"
156+
},
157+
"properties": {
158+
"AWS": {
159+
"scalarOrArray": {
160+
"$ref": "#/definitions/AwsPrincipalArn"
161+
}
157162
},
158-
{
159-
"properties": {
160-
"AWS": {
161-
"scalarOrArray": {
162-
"$ref": "#/definitions/AwsPrincipalArn"
163-
}
164-
},
165-
"CanonicalUser": {
166-
"$ref": "#/definitions/StringOrStringArray"
167-
},
168-
"Federated": {
169-
"$ref": "#/definitions/StringOrStringArray"
170-
},
171-
"Service": {
172-
"scalarOrArray": {
173-
"type": "string"
174-
}
175-
}
176-
},
177-
"type": "object"
163+
"CanonicalUser": {
164+
"$ref": "#/definitions/StringOrStringArray"
165+
},
166+
"Federated": {
167+
"$ref": "#/definitions/StringOrStringArray"
168+
},
169+
"Service": {
170+
"scalarOrArray": {
171+
"type": "string"
172+
}
178173
}
174+
},
175+
"then": {
176+
"$ref": "#/definitions/Wildcard"
177+
},
178+
"type": [
179+
"object",
180+
"string"
179181
]
180182
},
181183
"Resource": {

Diff for: src/cfnlint/rules/functions/SubNotJoin.py

+3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ def _check_elements(self, elements):
4444
def validate(
4545
self, validator: Validator, s: Any, instance: Any, schema: Any
4646
) -> ValidationResult:
47+
if validator.cfn.is_cdk_template():
48+
return
49+
4750
key = list(instance.keys())[0]
4851
value = instance.get(key)
4952

Diff for: test/unit/module/jsonschema/test_keywords.py

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
from collections import deque
7+
8+
import pytest
9+
10+
from cfnlint.jsonschema import ValidationError, _keywords
11+
from cfnlint.jsonschema.validators import CfnTemplateValidator
12+
from cfnlint.rules import CloudFormationLintRule
13+
14+
15+
class Error(CloudFormationLintRule):
16+
id = "E1111"
17+
18+
def validate(self, validator, s, instance, schema):
19+
print(instance)
20+
if s:
21+
yield ValidationError(
22+
"Error",
23+
rule=self,
24+
)
25+
26+
27+
@pytest.fixture
28+
def validator():
29+
validator = CfnTemplateValidator(schema={})
30+
validator = validator.extend(
31+
validators={
32+
"error": Error().validate,
33+
}
34+
)
35+
return validator({})
36+
37+
38+
@pytest.mark.parametrize(
39+
"name,instance,schema,expected",
40+
[
41+
(
42+
"Valid anyOf",
43+
"foo",
44+
[{"const": "foo"}, {"const": "bar"}],
45+
[],
46+
),
47+
(
48+
"Valid anyOf with error rule",
49+
"foo",
50+
[{"const": "foo"}, {"error": True}],
51+
[],
52+
),
53+
(
54+
"Invalid anyOf with error rule",
55+
"foo",
56+
[{"error": True}, {"error": True}],
57+
[
58+
ValidationError(
59+
"'foo' is not valid under any of the given schemas",
60+
path=deque([]),
61+
schema_path=deque([]),
62+
context=[
63+
ValidationError(
64+
"Error",
65+
rule=Error(),
66+
path=deque([]),
67+
validator="error",
68+
schema_path=deque([0, "error"]),
69+
),
70+
ValidationError(
71+
"Error",
72+
rule=Error(),
73+
path=deque([]),
74+
validator="error",
75+
schema_path=deque([1, "error"]),
76+
),
77+
],
78+
),
79+
],
80+
),
81+
],
82+
)
83+
def test_anyof(name, instance, schema, validator, expected):
84+
errs = list(_keywords.anyOf(validator, schema, instance, schema))
85+
assert errs == expected, f"{name!r} got errors {errs!r}"

Diff for: test/unit/rules/functions/test_sub_not_join_cdk.py

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
import pytest
7+
8+
from cfnlint.rules.functions.SubNotJoin import SubNotJoin
9+
10+
11+
@pytest.fixture(scope="module")
12+
def rule():
13+
rule = SubNotJoin()
14+
yield rule
15+
16+
17+
@pytest.fixture
18+
def template():
19+
return {
20+
"Resources": {
21+
"MyResource": {
22+
"Type": "AWS::S3::Bucket",
23+
},
24+
"CDK": {
25+
"Type": "AWS::CDK::Metadata",
26+
},
27+
},
28+
}
29+
30+
31+
@pytest.mark.parametrize(
32+
"name,instance,schema,expected",
33+
[
34+
(
35+
"Invalid Fn::Join with an empty string",
36+
{"Fn::Join": ["", ["foo", "bar"]]},
37+
{"type": "string"},
38+
[],
39+
),
40+
],
41+
)
42+
def test_validate(name, instance, schema, expected, rule, validator):
43+
errs = list(rule.validate(validator, schema, instance, {}))
44+
assert errs == expected, f"Test {name!r} got {errs!r}"

Diff for: test/unit/rules/resources/iam/test_resource_policy.py

+44
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,47 @@ def test_string_statements(self):
180180
errs[2].message, "'2012-10-18' is not one of ['2008-10-17', '2012-10-17']"
181181
)
182182
self.assertListEqual(list(errs[2].path), ["Version"])
183+
184+
def test_principal_wildcard(self):
185+
validator = CfnTemplateValidator({}).evolve(
186+
context=Context(functions=FUNCTIONS)
187+
)
188+
189+
policy = {
190+
"Version": "2012-10-17",
191+
"Statement": [
192+
{
193+
"Effect": "Allow",
194+
"Action": "*",
195+
"Resource": {
196+
"Fn::Sub": "arn:${AWS::Partition}:iam::123456789012:role/object-role"
197+
},
198+
"Principal": "*",
199+
},
200+
{
201+
"Effect": "Allow",
202+
"Action": "*",
203+
"Resource": {
204+
"Fn::Sub": "arn:${AWS::Partition}:iam::123456789012:role/object-role"
205+
},
206+
"Principal": {
207+
"AWS": "*",
208+
},
209+
},
210+
{
211+
"Effect": "Allow",
212+
"Action": "*",
213+
"Resource": {
214+
"Fn::Sub": "arn:${AWS::Partition}:iam::123456789012:role/object-role"
215+
},
216+
"Principal": {"Fn::Sub": "*"},
217+
},
218+
],
219+
}
220+
221+
errs = list(
222+
self.rule.validate(
223+
validator=validator, policy=policy, schema={}, policy_type=None
224+
)
225+
)
226+
self.assertListEqual(errs, [])

0 commit comments

Comments
 (0)