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

Codefixes #22

Closed
wants to merge 4 commits into from
Closed

Codefixes #22

wants to merge 4 commits into from

Conversation

obijan42
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Exception handling
  • Fix hanging when attempting to subscribe the master to itself
  • Implemented prompt-skipping for use in automation
  • Don't re-do standards that are already active
  • CSV file is now optional. It can autodetect the entire org instead
  • Changed references to python3

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

- Exception handling
- Fix hanging when attempting to subscribe the master to itself
- Don't re-do standards that are already active
@ryanholland
Copy link
Contributor

Hi,
Appreciate the changes you've made, however I do not think we can accept the commit to add Organization support as it leverages protected APIs, specifically ListAccounts, which can only be called from the Organization Master account and will result in a failure if called from any other account. We guide users to minimize use of the Organization Master account and in many cases that account will not be accessible to those deploying Security Hub.

- Elegant fallback when org api fails
@obijan42
Copy link
Contributor Author

The added support is an option, which nobody is forced to use. The way the latest version works:
IF you don't supply a CSV, then
If the org call works, it uses that, else
It complains that you didn't specify a list

So that should be backwards compatible for all cases.

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

2 participants