Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permit leading whitespace in IAM policies by normalizing before check… #5887

Closed
wants to merge 3 commits into from
Closed

Permit leading whitespace in IAM policies by normalizing before check… #5887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ecc9b2e
Permit leading whitespace in IAM policies by normalizing before check…
Sep 14, 2018
0bc3b85
Minor cleanup
Sep 15, 2018
b766c61
Modify the IAM Role test policy to be invalid JSON
Sep 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions aws/resource_aws_iam_role_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,15 @@ resource "aws_iam_role_policy" "foo" {
name = "tf_test_policy_%s"
role = "${aws_iam_role.role.name}"
policy = <<EOF
{
x{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this deliberate?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That resource is intended to trigger the "invalid JSON" path. Before this patch, it was triggered by the indentation, which is now considered valid. In retrospect, instead of "x" I should have put something ilke "this_json_is_intentionally_invalid_for_testing".

I haven't touched this code in ~6 months but I will try to find time this week to rebase, verify tests still pass, and make this intent clearer.

"Version": "2012-10-17",
"Statement": {
"Effect": "Allow",
"Action": "*",
"Resource": "*"
}
"Statement": [
{
"Effect": "Allow",
"Action": "*",
"Resource": "*"
}
]
}
EOF
}
Expand Down
13 changes: 7 additions & 6 deletions aws/validators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,17 +652,18 @@ func validateJsonString(v interface{}, k string) (ws []string, errors []error) {

func validateIAMPolicyJson(v interface{}, k string) (ws []string, errors []error) {
// IAM Policy documents need to be valid JSON, and pass legacy parsing
value := v.(string)
if len(value) < 1 {
if len(v.(string)) < 1 {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy", k))
return
}
if value[:1] != "{" {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy", k))
vNormalized, err := structure.NormalizeJsonString(v)
if err != nil {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
return
}
if _, err := structure.NormalizeJsonString(v); err != nil {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
if vNormalized[:1] != "{" {
errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy", k))
return
}
return
}
Expand Down
4 changes: 0 additions & 4 deletions aws/validators_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,6 @@ func TestValidateIAMPolicyJsonString(t *testing.T) {
Value: ``,
ErrCount: 1,
},
{
Value: ` {"xyz": "foo"}`,
ErrCount: 1,
},
}

for _, tc := range invalidCases {
Expand Down