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

fix(sns): allow default SNS policy with SourceOwner #2698

Merged
merged 6 commits into from
Aug 10, 2023

Conversation

christiandavilakoobin
Copy link
Contributor

Context

Prowler alerts for sns public accessibilty checks on topics that are generated from the AWS console.

Description

The AWS console uses the not very well documented AWS:SourceOwner attribute on defaults policies: awsdocs/iam-user-guide#111

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@christiandavilakoobin christiandavilakoobin requested a review from a team as a code owner August 10, 2023 08:56
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

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

Good catch!!

Please include tests for the policy_condition_parser library.

@christiandavilakoobin
Copy link
Contributor Author

Done! I don't find how to run the tests localy before the PR. I checked the CONTRIB docs, but it only says how to create or modify the tests, but not how to run them.

@sergargar
Copy link
Member

sergargar commented Aug 10, 2023

Done! I don't find how to run the tests localy before the PR. I checked the CONTRIB docs, but it only says how to create or modify the tests, but not how to run them.

You have to do a pre-commit install inside the repo :) You can see more in our Developer Guide

@jfagoagas jfagoagas added the provider/aws Issues/PRs related with the AWS provider label Aug 10, 2023
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

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

Thanks again @christiandavilakoobin !

I've fixed some tests, so this will be merged once the CI pass.

@sergargar sergargar merged commit ade511d into prowler-cloud:master Aug 10, 2023
4 checks passed
sergargar added a commit that referenced this pull request Aug 10, 2023
Co-authored-by: Azure Pipeplines CI <monitor@koobin.com>
Co-authored-by: Sergio Garcia <sergargar1@gmail.com>
Co-authored-by: Pepe Fagoaga <pepe@verica.io>
sergargar added a commit that referenced this pull request Aug 10, 2023
Co-authored-by: Azure Pipeplines CI <monitor@koobin.com>
Co-authored-by: Sergio Garcia <sergargar1@gmail.com>
Co-authored-by: Pepe Fagoaga <pepe@verica.io>
sergargar added a commit that referenced this pull request Aug 10, 2023
Co-authored-by: Azure Pipeplines CI <monitor@koobin.com>
Co-authored-by: Sergio Garcia <sergargar1@gmail.com>
Co-authored-by: Pepe Fagoaga <pepe@verica.io>
sergargar added a commit that referenced this pull request Aug 10, 2023
Co-authored-by: Azure Pipeplines CI <monitor@koobin.com>
Co-authored-by: Sergio Garcia <sergargar1@gmail.com>
Co-authored-by: Pepe Fagoaga <pepe@verica.io>
jfagoagas added a commit that referenced this pull request Aug 14, 2023
Co-authored-by: Azure Pipeplines CI <monitor@koobin.com>
Co-authored-by: Sergio Garcia <sergargar1@gmail.com>
Co-authored-by: Pepe Fagoaga <pepe@verica.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/aws Issues/PRs related with the AWS provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants