diff --git a/README.md b/README.md index 888d17e58..03a6c6298 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,6 @@ RepokidRole: "iam:ListInstanceProfiles", "iam:ListInstanceProfilesForRole", "iam:ListRolePolicies", - "iam:ListRoles", "iam:PutRolePolicy", "iam:UpdateRoleDescription" ], diff --git a/repokid/cli/repokid_cli.py b/repokid/cli/repokid_cli.py index 6adbb2ecf..8f98220f7 100644 --- a/repokid/cli/repokid_cli.py +++ b/repokid/cli/repokid_cli.py @@ -367,7 +367,8 @@ def update_role_cache(account_number, dynamo_table, config, hooks): conn = config['connection_iam'] conn['account_number'] = account_number - LOGGER.info('Getting current role data for account {} (this may take a while for large accounts)'.format(account_number)) + LOGGER.info('Getting current role data for account {} (this may take a while for large accounts)'.format( + account_number)) role_data = get_account_authorization_details(filter='Role', **conn) role_data_by_id = {item['RoleId']: item for item in role_data} @@ -375,7 +376,7 @@ def update_role_cache(account_number, dynamo_table, config, hooks): for _, data in role_data_by_id.items(): data['RolePolicyList'] = {item['PolicyName']: item['PolicyDocument'] for item in data['RolePolicyList']} - roles = Roles([Role(role_data) for role_data in role_data]) + roles = Roles([Role(rd) for rd in role_data]) active_roles = [] LOGGER.info('Updating role data for account {}'.format(account_number)) @@ -890,7 +891,7 @@ def rollback_role(account_number, role_name, dynamo_table, config, hooks, select # remove the policy name if it's in the list try: policies_to_remove.remove(policy_name) - except Exception: + except Exception: # nosec pass if policies_to_remove: diff --git a/repokid/dispatcher/__init__.py b/repokid/dispatcher/__init__.py index 43da1901b..ba0d2fa8a 100644 --- a/repokid/dispatcher/__init__.py +++ b/repokid/dispatcher/__init__.py @@ -39,13 +39,15 @@ def list_repoable_services(dynamo_table, message): role_data['RepoableServices']) repoable_services = role_data['RepoableServices'] - return ResponderReturn(successful=True, - return_message=('Role {} in account {} has:\n Repoable Services: \n{}' - '\n\n Repoable Permissions: \n{}'.format( - message.role_name, - message.account, - '\n'.join([service for service in repoable_services]), - '\n'.join([perm for perm in repoable_permissions])))) + return ResponderReturn( + successful=True, + return_message=( + 'Role {} in account {} has:\n Repoable Services: \n{}\n\n Repoable Permissions: \n{}'.format( + message.role_name, message.account, '\n'.join([service for service in repoable_services]), + '\n'.join([perm for perm in repoable_permissions]) + ) + ) + ) @implements_command('list_role_rollbacks') @@ -101,9 +103,11 @@ def opt_out(dynamo_table, message): expire_epoch = int((expire_dt - datetime.datetime(1970, 1, 1)).total_seconds()) new_opt_out = {'owner': message.requestor, 'reason': message.reason, 'expire': expire_epoch} dynamo.set_role_data(dynamo_table, role_id, {'OptOut': new_opt_out}) - return ResponderReturn(successful=True, - return_message='Role {} in account {} opted-out until {}'.format( - message.role_name, message.account, expire_dt.strftime('%m/%d/%y'))) + return ResponderReturn( + successful=True, + return_message='Role {} in account {} opted-out until {}'.format( + message.role_name, message.account, expire_dt.strftime('%m/%d/%y')) + ) @implements_command('remove_opt_out') @@ -138,6 +142,8 @@ def rollback_role(dynamo_table, message): if errors: return ResponderReturn(successful=False, return_message='Errors during rollback: {}'.format(errors)) else: - return ResponderReturn(successful=True, - return_message='Successfully rolled back role {} in account {}'.format( - message.role_name, message.account)) + return ResponderReturn( + successful=True, + return_message='Successfully rolled back role {} in account {}'.format( + message.role_name, message.account) + ) diff --git a/repokid/tests/test_repokid_cli.py b/repokid/tests/test_repokid_cli.py index 2be3ea14c..9361727c3 100644 --- a/repokid/tests/test_repokid_cli.py +++ b/repokid/tests/test_repokid_cli.py @@ -45,33 +45,51 @@ ROLE_POLICIES = { 'all_services_used': { - 'iam_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}, - - 's3_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['s3:CreateBucket', 's3:DeleteBucket'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}}, + 'iam_perms': { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], + 'Resource': ['*'], + 'Effect': 'Allow' + } + ] + }, + + 's3_perms': { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': ['s3:CreateBucket', 's3:DeleteBucket'], + 'Resource': ['*'], + 'Effect': 'Allow' + } + ] + } + }, 'unused_ec2': { - 'iam_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}, - - 'ec2_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['ec2:AllocateHosts', 'ec2:AssociateAddress'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}} + 'iam_perms': { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], + 'Resource': ['*'], + 'Effect': 'Allow' + } + ] + }, + + 'ec2_perms': { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': ['ec2:AllocateHosts', 'ec2:AssociateAddress'], + 'Resource': ['*'], + 'Effect': 'Allow' + } + ] + } + } } ROLES = [ @@ -150,11 +168,14 @@ def test_repokid_update_role_cache(self, mock_get_account_authorization_details, mock_find_and_mark_inactive, mock_update_stats): hooks = {} - + role_data = ROLES[:3] - role_data[0]['RolePolicyList'] = [{'PolicyName': 'all_services_used', 'PolicyDocument': ROLE_POLICIES['all_services_used'] }] - role_data[1]['RolePolicyList'] = [{'PolicyName': 'unused_ec2', 'PolicyDocument': ROLE_POLICIES['unused_ec2'] }] - role_data[2]['RolePolicyList'] = [{'PolicyName': 'all_services_used', 'PolicyDocument': ROLE_POLICIES['all_services_used'] }] + role_data[0]['RolePolicyList'] = [{'PolicyName': 'all_services_used', + 'PolicyDocument': ROLE_POLICIES['all_services_used']}] + role_data[1]['RolePolicyList'] = [{'PolicyName': 'unused_ec2', + 'PolicyDocument': ROLE_POLICIES['unused_ec2']}] + role_data[2]['RolePolicyList'] = [{'PolicyName': 'all_services_used', + 'PolicyDocument': ROLE_POLICIES['all_services_used']}] mock_get_account_authorization_details.side_effect = [role_data] mock_get_aardvark_data.return_value = AARDVARK_DATA @@ -185,9 +206,11 @@ def update_role_data(dynamo_table, account_number, role, current_policies): # validate update data called for each role assert mock_update_role_data.mock_calls == [ - call(dynamo_table, account_number, Role(ROLES[0]), {'all_services_used': ROLE_POLICIES['all_services_used']}), + call(dynamo_table, account_number, Role(ROLES[0]), {'all_services_used': + ROLE_POLICIES['all_services_used']}), call(dynamo_table, account_number, Role(ROLES[1]), {'unused_ec2': ROLE_POLICIES['unused_ec2']}), - call(dynamo_table, account_number, Role(ROLES[2]), {'all_services_used': ROLE_POLICIES['all_services_used']})] + call(dynamo_table, account_number, Role(ROLES[2]), {'all_services_used': + ROLE_POLICIES['all_services_used']})] # all roles active assert mock_find_and_mark_inactive.mock_calls == [call(dynamo_table, account_number, diff --git a/repokid/tests/test_roledata.py b/repokid/tests/test_roledata.py index ec47e1852..4d624ddce 100644 --- a/repokid/tests/test_roledata.py +++ b/repokid/tests/test_roledata.py @@ -11,14 +11,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import datetime import time -from dateutil.tz import tzlocal from mock import patch import repokid.utils.roledata from repokid.role import Role +from repokid.tests.test_repokid_cli import ROLE_POLICIES, ROLES AARDVARK_DATA = { @@ -41,68 +40,6 @@ "serviceNamespace": "s3"}] } -ROLE_POLICIES = { - 'all_services_used': { - 'iam_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}, - - 's3_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['s3:CreateBucket', 's3:DeleteBucket'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}}, - 'unused_ec2': { - 'iam_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}, - - 'ec2_perms': { - 'Version': '2012-10-17', - 'Statement': [ - {'Action': ['ec2:AllocateHosts', 'ec2:AssociateAddress'], - 'Resource': ['*'], - 'Effect': 'Allow'}]}} -} - -ROLES = [ - { - "Arn": "arn:aws:iam::123456789012:role/all_services_used", - "CreateDate": datetime.datetime(2017, 1, 31, 12, 0, 0, tzinfo=tzlocal()), - "RoleId": "AROAABCDEFGHIJKLMNOPA", - "RoleName": "all_services_used", - "Active": True - }, - { - "Arn": "arn:aws:iam::123456789012:role/unused_ec2", - "CreateDate": datetime.datetime(2017, 1, 31, 12, 0, 0, tzinfo=tzlocal()), - "RoleId": "AROAABCDEFGHIJKLMNOPB", - "RoleName": "unused_ec2", - "Active": True, - }, - { - "Arn": "arn:aws:iam::123456789012:role/young_role", - "CreateDate": datetime.datetime.now(tzlocal()) - datetime.timedelta(5), - "RoleId": "AROAABCDEFGHIJKLMNOPC", - "RoleName": "young_role", - "Active": True, - }, - { - "Arn": "arn:aws:iam::123456789012:role/inactive_role", - "CreateDate": datetime.datetime.now(tzlocal()) - datetime.timedelta(5), - "RoleId": "AROAABCDEFGHIJKLMNOPD", - "RoleName": "inactive_role", - "Active": False, - } -] - class TestRoledata(object): @patch('repokid.utils.roledata.expand_policy') @@ -141,7 +78,7 @@ def test_get_repoable_permissions(self, mock_call_hooks): {'serviceNamespace': 'service_2', 'lastAuthenticated': (time.time() - 90000) * 1000}, {'serviceNamespace': 'service_3', 'lastAuthenticated': time.time() * 1000}] - no_repo_permissions = {'service_4:action_1': time.time()-1, 'service_4:action_2': time.time()+1000} + no_repo_permissions = {'service_4:action_1': time.time() - 1, 'service_4:action_2': time.time() + 1000} repoable_decision = repokid.utils.roledata.RepoablePermissionDecision() repoable_decision.repoable = True diff --git a/repokid/utils/dynamo.py b/repokid/utils/dynamo.py index 08dff74a0..9d8df69fb 100644 --- a/repokid/utils/dynamo.py +++ b/repokid/utils/dynamo.py @@ -70,11 +70,14 @@ def dynamo_get_or_create_table(**dynamo_config): try: table = resource.create_table( TableName='repokid_roles', - KeySchema=[{ - 'AttributeName': 'RoleId', - 'KeyType': 'HASH' # Partition key - }], - AttributeDefinitions=[{ + KeySchema=[ + { + 'AttributeName': 'RoleId', + 'KeyType': 'HASH' # Partition key + } + ], + AttributeDefinitions=[ + { 'AttributeName': 'RoleId', 'AttributeType': 'S' }, @@ -85,7 +88,8 @@ def dynamo_get_or_create_table(**dynamo_config): { 'AttributeName': 'Account', 'AttributeType': 'S' - }], + } + ], ProvisionedThroughput={ 'ReadCapacityUnits': 50, 'WriteCapacityUnits': 50 @@ -115,7 +119,8 @@ def dynamo_get_or_create_table(**dynamo_config): 'KeyType': 'HASH' } ], - 'Projection': { + 'Projection': + { 'ProjectionType': 'KEYS_ONLY', }, 'ProvisionedThroughput': { diff --git a/repokid/utils/roledata.py b/repokid/utils/roledata.py index f946d73bf..fe85fadac 100644 --- a/repokid/utils/roledata.py +++ b/repokid/utils/roledata.py @@ -114,8 +114,8 @@ def update_no_repo_permissions(dynamo_table, role, newly_added_permissions): Returns: None """ - current_ignored_permissions = get_role_data(dynamo_table, role.role_id, fields=['NoRepoPermissions']).get( - 'NoRepoPermissions', {}) + current_ignored_permissions = get_role_data( + dynamo_table, role.role_id, fields=['NoRepoPermissions']).get('NoRepoPermissions', {}) new_ignored_permissions = {} current_time = int(time.time()) @@ -261,7 +261,7 @@ def _calculate_repo_scores(roles, minimum_age, hooks): repoable_permissions = _get_repoable_permissions(role.account, role.role_name, permissions, role.aa_data, role.no_repo_permissions, minimum_age, hooks) (repoable_permissions_set, repoable_services_set) = _convert_repoable_perms_to_perms_and_services( - permissions, repoable_permissions) + permissions, repoable_permissions) role.repoable_permissions = len(repoable_permissions) diff --git a/setup.py b/setup.py index 000a182f8..e96c5c58f 100644 --- a/setup.py +++ b/setup.py @@ -52,5 +52,5 @@ 'Topic :: Security', 'Topic :: System', 'Topic :: System :: Systems Administration' - ] + ] ) diff --git a/test-requirements.txt b/test-requirements.txt index 89f8443fc..7d4a73563 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,4 @@ -flake8==3.4.1 +flake8 python-dateutil==2.6.0 mock==2.0.0 pytest==3.2.3