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 IAM definition update code #253

Closed
kmcquade opened this issue Oct 15, 2020 · 3 comments
Closed

Fix IAM definition update code #253

kmcquade opened this issue Oct 15, 2020 · 3 comments
Assignees

Comments

@kmcquade
Copy link
Collaborator

kmcquade commented Oct 15, 2020

AWS changed the format of the Actions, Resources, and Condition Keys page slightly. Since we use that as the data source for the IAM Definition (as described in our documentation here), we need to update the IAM update process.

Luckily, Scott Piper figured out the modifications that are required in this PR to duo-labs/parliament. Since we both use the same approach for our IAM update process, it is relatively straightforward to make this fix.

The key code, which matches up pretty well with his code, is here.

def create_database(destination_directory, access_level_overrides_file):

If anyone takes this up, I suggest looking at the diff from his PR, matching that up with the corresponding parts in ours, and then running the utils/update_iam_data.py script to see if it updates properly. You can check the iam_definition.json file in this folder to see if it worked successfully.

@reetasingh
Copy link
Contributor

reetasingh commented Oct 16, 2020

@kmcquade taking this up. ok to assign this to me?

@reetasingh
Copy link
Contributor

reetasingh commented Oct 16, 2020

@kmcquade there is no file utils/update_iam_data.py

@kmcquade
Copy link
Collaborator Author

This was closed by #254 thanks to @reetasingh.

In the pull request, I explained some of the testing strategy. I am providing that text here for documentation purposes.

Testing strategy so far

Generally, my testing strategy has gone like this so far:

  • File mapping: Unit test files map directly to the .py files in the Policy Sentry package
  • Folder mapping: Folder structure of the test files maps directly to the folder structure in the Policy Sentry package
  • GitHub issue mapping: As users have opened up new bugs, I add unit tests that correspond directly to those GitHub issues to show that my fix work specifically for their ask. I prefixed them with test_gh_999 if 999 is the GitHub issue number. For example:
    def test_gh_211_write_with_empty_access_level_lists(self):
  • Adaptable to IAM Definition changes: When I started unit tests for the write-policy command, I initially wrote my tests so that they would compare the results with the expected_results.
    • Over time, as AWS added new IAM actions, that meant that there would be more actions per access level
    • Example: let's say that hypothetically the secretsmanager service had 4 actions at the "write" access level that could be restricted to the secret resource type. If I wrote the tests so that they tested the value of the generated policy, that works for the initial tests. But if AWS adds two new actions that can be restricted to that access level and resource type, then the tests would fail.
    • This only really applies for where the expected test values would be dependent on the current state of the IAM definition (aka the IAM Database, iam_definition.json) and the actions in there. So, validating the output of write-policy related commands, and query related commands.
    • After I realized that, I started writing my tests so that the expected output that showed what actions you'd want would be in the comments instead of the expected_results, so the human knows what's going on. And then I would just check that the size of the array holding the actions is greater than or equal to the size that it is currently at. So, in the secretsmanager example above, the test would probably just check that the size of the actions is greater than or equal to 4.

I'm going to paste this very informative text about the testing strategy in the relevant GitHub issue so others can find it later lol.

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

No branches or pull requests

2 participants