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

Extend has-statement s3 filter to check for full bucket policy definitions in addition to policy names. #1988

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

jahrome
Copy link
Contributor

@jahrome jahrome commented Jan 30, 2018

No description provided.

@kapilt
Copy link
Collaborator

kapilt commented Feb 5, 2018

thanks for the pr. i pushed a commit with some minor changes/cleanups. modified the matching logic to be more explicit, (default not match), also switched value in statement[key] to equality check.

@kapilt kapilt merged commit abf65c3 into cloud-custodian:master Feb 5, 2018
found = False
for key, value in required_statement.items():
if key in statement and value == statement[key]:
found = True
Copy link
Collaborator

@alfredgamulo alfredgamulo Feb 15, 2018

Choose a reason for hiding this comment

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

@kapilt I think the change here is incorrect.
Provided the example policy in the comments above, the logic here says that any one of
key/value pairs in

- Effect: Allow
  Action: 's3:*'
  Principal: '*'

is sufficient to match the statement rather than all key/value pairs matching.

lamyanba pushed a commit to lamyanba/cloud-custodian that referenced this pull request Apr 23, 2019
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.

3 participants