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.

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

Iam sid errors #1812

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

jnewbigin
Copy link

Issue #, if available:

Description of changes:
Validate IAM Sid against the specified regex and check for uniqueness in the policy document.

I based it on the IAM Version validator. I can't say I really understand how that works so happy to make changes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

self.resource_property_types.append(resource_type)
for resource_type in self.idp_and_keys:
self.resource_property_types.append(resource_type)
self.sid_regex = re.compile('^[A-Za-z0-9]*$')
Copy link
Contributor

Choose a reason for hiding this comment

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

SIDs for ManagedPolicies have a different regex than SIDs in an inline policy, IIRC. I believe spaces are allowed in an inline policy SID

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar discussion here: #708

Copy link
Author

Choose a reason for hiding this comment

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

I started working off the cloudformation error I revieved on my stack.
Then I turned to the AWS docs to try and find more details.
The docs are pretty vague - particularly for resource policies
If I get a chance I'll set up some test templates and try them out. Seems to be where #708 got to

Copy link
Author

Choose a reason for hiding this comment

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

I can confirm that ECR and KMS policies do not limit to that regex or require uniqueness.
I have not tested the other types but it seems that the rules are definitely different between resource types.
Given that, I think for now these rules should only apply to IAM resources.
I will update my PR to reflect that

Copy link
Author

Choose a reason for hiding this comment

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

Still to confirm the correctness for inline policies

@kddejong kddejong changed the base branch from master to main June 10, 2021 18:08
@lobinator
Copy link

any progress planned on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants