-
Notifications
You must be signed in to change notification settings - Fork 657
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
Fix condition key name of Amazon SNS topic policies #805
Fix condition key name of Amazon SNS topic policies #805
Conversation
Hi @AkhtarAmir, could you please review this PR? |
plugins/aws/sns/topicPolicies.js
Outdated
@@ -86,7 +86,7 @@ module.exports = { | |||
var conditionExists = (statement.Condition ? true : false); | |||
// Is it a string condition (StringEquals)? Is the SourceOwner open to everyone? | |||
var conditionString = ((statement.Condition && statement.Condition.StringEquals && | |||
(statement.Condition.StringEquals['AWS:SourceOwner'] || !statement.Condition.StringEquals['AWS:SourceOwner'] == '*')) ? true : false); | |||
(statement.Condition.StringEquals['aws:SourceOwner'] || !statement.Condition.StringEquals['aws:SourceOwner'] == '*')) ? true : false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make it case insensitive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed 43d90e2.
@AkhtarAmir also, I found a bug and fixed dd01a28. |
I'm working on fixing errors 🏃 |
Fix condition key case from
AWS:SourceOwner
toaws:SourceOwner
.According to the document, there should be start with
aws:
instead ofAWS:
.https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html
https://docs.aws.amazon.com/sns/latest/dg/sns-access-policy-use-cases.html