From 70777020c41b86d0197062ff00957ad6d16cb0c8 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Thu, 22 Aug 2019 15:25:25 +0200 Subject: [PATCH] Fix iam_password_policy integration tests (#60930) * iam_password_policy: (integration tests) Use module defaults for AWS connection details * iam_password_policy: (integration tests) Ensure the policy is removed when tests fail * iam_password_policy: (integration tests) Add regression test for #59102 * iam_password_policy: Only return changed when the policy changes. * iam_password_policy: PasswordReusePrevention must be omitted to remove/set to 0 * #60930 add changelog * Update hacking AWS security policy to allow testing of Password Policy Management --- .../fragments/60930-iam_password_policy.yml | 3 + .../testing_policies/security-policy.json | 10 ++ .../cloud/amazon/iam_password_policy.py | 29 ++- .../iam_password_policy/tasks/main.yaml | 169 ++++++++++-------- 4 files changed, 129 insertions(+), 82 deletions(-) create mode 100644 changelogs/fragments/60930-iam_password_policy.yml diff --git a/changelogs/fragments/60930-iam_password_policy.yml b/changelogs/fragments/60930-iam_password_policy.yml new file mode 100644 index 00000000000..066fad6420e --- /dev/null +++ b/changelogs/fragments/60930-iam_password_policy.yml @@ -0,0 +1,3 @@ +bugfixes: +- iam_password_policy now only returns changed when the policy changes +- iam_password_policy no longer throws errors when you don't set pw_reuse_prevent diff --git a/hacking/aws_config/testing_policies/security-policy.json b/hacking/aws_config/testing_policies/security-policy.json index a1086aadcb0..b7feb6b0b9c 100644 --- a/hacking/aws_config/testing_policies/security-policy.json +++ b/hacking/aws_config/testing_policies/security-policy.json @@ -128,6 +128,16 @@ "iam:GetServerCertificate" ], "Resource": "*" + }, + { + "Sid": "AllowAccessToManagePasswordPolicy", + "Effect": "Allow", + "Action": [ + "iam:GetAccountPasswordPolicy", + "iam:DeleteAccountPasswordPolicy", + "iam:UpdateAccountPasswordPolicy" + ], + "Resource": "*" } ] } diff --git a/lib/ansible/modules/cloud/amazon/iam_password_policy.py b/lib/ansible/modules/cloud/amazon/iam_password_policy.py index 0f838018f93..02b0147f19c 100644 --- a/lib/ansible/modules/cloud/amazon/iam_password_policy.py +++ b/lib/ansible/modules/cloud/amazon/iam_password_policy.py @@ -109,7 +109,6 @@ from ansible.module_utils.ec2 import camel_dict_to_snake_dict, boto3_tag_list_to class IAMConnection(object): - def __init__(self, module): try: self.connection = module.resource('iam') @@ -117,6 +116,17 @@ class IAMConnection(object): except Exception as e: module.fail_json(msg="Failed to connect to AWS: %s" % str(e)) + def policy_to_dict(self, policy): + policy_attributes = [ + 'allow_users_to_change_password', 'expire_passwords', 'hard_expiry', + 'max_password_age', 'minimum_password_length', 'password_reuse_prevention', + 'require_lowercase_characters', 'require_numbers', 'require_symbols', 'require_uppercase_characters' + ] + ret = {} + for attr in policy_attributes: + ret[attr] = getattr(policy, attr) + return ret + def update_password_policy(self, module, policy): min_pw_length = module.params.get('min_pw_length') require_symbols = module.params.get('require_symbols') @@ -135,18 +145,27 @@ class IAMConnection(object): RequireUppercaseCharacters=require_uppercase, RequireLowercaseCharacters=require_lowercase, AllowUsersToChangePassword=allow_pw_change, - PasswordReusePrevention=pw_reuse_prevent, HardExpiry=pw_expire ) + if pw_reuse_prevent: + update_parameters.update(PasswordReusePrevention=pw_reuse_prevent) if pw_max_age: update_parameters.update(MaxPasswordAge=pw_max_age) + try: + original_policy = self.policy_to_dict(policy) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + original_policy = {} + try: results = policy.update(**update_parameters) policy.reload() + updated_policy = self.policy_to_dict(policy) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self.module.fail_json_aws(e, msg="Couldn't update IAM Password Policy") - return camel_dict_to_snake_dict(results) + + changed = (original_policy != updated_policy) + return (changed, updated_policy, camel_dict_to_snake_dict(results)) def delete_password_policy(self, policy): try: @@ -182,8 +201,8 @@ def main(): state = module.params.get('state') if state == 'present': - update_result = resource.update_password_policy(module, policy) - module.exit_json(changed=True, task_status={'IAM': update_result}) + (changed, new_policy, update_result) = resource.update_password_policy(module, policy) + module.exit_json(changed=changed, task_status={'IAM': update_result}, policy=new_policy) if state == 'absent': delete_result = resource.delete_password_policy(policy) diff --git a/test/integration/targets/iam_password_policy/tasks/main.yaml b/test/integration/targets/iam_password_policy/tasks/main.yaml index 09f6afa956a..6cffea003a9 100644 --- a/test/integration/targets/iam_password_policy/tasks/main.yaml +++ b/test/integration/targets/iam_password_policy/tasks/main.yaml @@ -1,90 +1,105 @@ -- name: set connection information for all tasks - set_fact: - aws_connection_info: &aws_connection_info +- module_defaults: + group/aws: aws_access_key: "{{ aws_access_key }}" aws_secret_key: "{{ aws_secret_key }}" - security_token: "{{ security_token }}" + security_token: "{{ security_token | default(omit) }}" region: "{{ aws_region }}" - no_log: true + block: + - name: set iam password policy + iam_password_policy: + state: present + min_pw_length: 8 + require_symbols: false + require_numbers: true + require_uppercase: true + require_lowercase: true + allow_pw_change: true + pw_max_age: 60 + pw_reuse_prevent: 5 + pw_expire: false + register: result -- name: set iam password policy - iam_password_policy: - <<: *aws_connection_info - state: present - min_pw_length: 8 - require_symbols: false - require_numbers: true - require_uppercase: true - require_lowercase: true - allow_pw_change: true - pw_max_age: 60 - pw_reuse_prevent: 5 - pw_expire: false - register: result + - name: assert that changes were made + assert: + that: + - result.changed -- name: assert that changes were made - assert: - that: - - result.changed + - name: verify iam password policy has been created + iam_password_policy: + state: present + min_pw_length: 8 + require_symbols: false + require_numbers: true + require_uppercase: true + require_lowercase: true + allow_pw_change: true + pw_max_age: 60 + pw_reuse_prevent: 5 + pw_expire: false + register: result -- name: verify iam password policy has been created - iam_password_policy: - <<: *aws_connection_info - state: present - min_pw_length: 8 - require_symbols: false - require_numbers: true - require_uppercase: true - require_lowercase: true - allow_pw_change: true - pw_max_age: 60 - pw_reuse_prevent: 5 - pw_expire: false - register: result + - name: assert that no changes were made + assert: + that: + - not result.changed -- name: assert that no changes were made - assert: - that: - - not result.changed + - name: update iam password policy with different settings + iam_password_policy: + state: present + min_pw_length: 15 + require_symbols: true + require_numbers: true + require_uppercase: true + require_lowercase: true + allow_pw_change: true + pw_max_age: 30 + pw_reuse_prevent: 10 + pw_expire: true + register: result -- name: update iam password policy - iam_password_policy: - <<: *aws_connection_info - state: present - min_pw_length: 15 - require_symbols: true - require_numbers: true - require_uppercase: true - require_lowercase: true - allow_pw_change: true - pw_max_age: 30 - pw_reuse_prevent: 10 - pw_expire: true - register: result + - name: assert that updates were made + assert: + that: + - result.changed -- name: assert that updates were made - assert: - that: - - result.changed + # Test for regression of #59102 + - name: update iam password policy without expiry + iam_password_policy: + state: present + min_pw_length: 15 + require_symbols: true + require_numbers: true + require_uppercase: true + require_lowercase: true + allow_pw_change: true + register: result -- name: remove iam password policy - iam_password_policy: - <<: *aws_connection_info - state: absent - register: result + - name: assert that changes were made + assert: + that: + - result.changed -- name: assert password policy has been removed - assert: - that: - - result.changed + - name: remove iam password policy + iam_password_policy: + state: absent + register: result -- name: verify password policy has been removed - iam_password_policy: - <<: *aws_connection_info - state: absent - register: result + - name: assert password policy has been removed + assert: + that: + - result.changed -- name: assert no changes were made - assert: - that: - - not result.changed + - name: verify password policy has been removed + iam_password_policy: + state: absent + register: result + + - name: assert no changes were made + assert: + that: + - not result.changed + always: + - name: remove iam password policy + iam_password_policy: + state: absent + register: result