Fix changing the type of an existing Vault.

Current implementation does not allow the change of an existingi Vault
type. To allow it, data is retrieved from the current vault, the vault
is modifiend, and then, data is stored again in the new vault.

Due to changing the process of modifying a vault, this change also
fixes the update of asymmetric vault keys. To change the key used,
the task must provide the old private key, used to retrieve data,
and the new public_key, used to store the data again. A new alias
was added to public_key (new_public_key) and public_key_file
(new_public_key_file) so that the playbook better express the
intention of the tak.

Vault tests have been updated to better test against the new update
process, and a new test file has bee added:

    tests/vault/test_vault_change_type.

plugins/modules/ipavault.py

@@ -317,10 +317,11 @@ vault:
 import os
 from base64 import b64decode
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, \
     gen_add_del_lists, compare_args_ipa, module_params_get, exit_raw_json
-from ipalib.errors import EmptyModlist
+from ipalib.errors import EmptyModlist, NotFound
 
 
 def find_vault(module, name, username, service, shared):
@@ -351,7 +352,9 @@ def gen_args(description, username, service, shared, vault_type, salt,
              password, password_file, public_key, public_key_file, vault_data,
              datafile_in, datafile_out):
     _args = {}
+    vault_type = vault_type or to_text("symmetric")
 
+    _args['ipavaulttype'] = vault_type
     if description is not None:
         _args['description'] = description
     if username is not None:
@@ -360,27 +363,32 @@ def gen_args(description, username, service, shared, vault_type, salt,
         _args['service'] = service
     if shared is not None:
         _args['shared'] = shared
-    if vault_type is not None:
-        _args['ipavaulttype'] = vault_type
-    if salt is not None:
-        _args['ipavaultsalt'] = salt
-    if public_key is not None:
-        _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
-    if public_key_file is not None:
-        with open(public_key_file, 'r') as keyfile:
-            keydata = keyfile.read()
-            _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+
+    if vault_type == "symmetric":
+        if salt is not None:
+            _args['ipavaultsalt'] = salt
+        _args['ipavaultpublickey'] = None
+
+    elif vault_type == "asymmetric":
+        if public_key is not None:
+            _args['ipavaultpublickey'] = b64decode(public_key.encode('utf-8'))
+        if public_key_file is not None:
+            with open(public_key_file, 'r') as keyfile:
+                keydata = keyfile.read()
+                _args['ipavaultpublickey'] = keydata.strip().encode('utf-8')
+        _args['ipavaultsalt'] = None
+
+    elif vault_type == "standard":
+        _args['ipavaultsalt'] = None
+        _args['ipavaultpublickey'] = None
 
     return _args
 
 
 def gen_member_args(args, users, groups, services):
-    _args = args.copy()
-
-    for arg in ['ipavaulttype', 'description', 'ipavaultpublickey',
-                'ipavaultsalt']:
-        if arg in _args:
-            del _args[arg]
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if any([users, groups, services]):
         if users is not None:
@@ -395,9 +403,12 @@ def gen_member_args(args, users, groups, services):
     return None
 
 
-def data_storage_args(args, data, password, password_file, private_key,
-                      private_key_file, datafile_in, datafile_out):
-    _args = {}
+def data_storage_args(vault_type, args, data, password, password_file,
+                      private_key, private_key_file, datafile_in,
+                      datafile_out):
+    remove = ['ipavaulttype', 'description', 'ipavaultpublickey',
+              'ipavaultsalt']
+    _args = {k: v for k, v in args.items() if k not in remove}
 
     if 'username' in args:
         _args['username'] = args['username']
@@ -406,15 +417,17 @@ def data_storage_args(args, data, password, password_file, private_key,
     if 'shared' in args:
         _args['shared'] = args['shared']
 
-    if password is not None:
-        _args['password'] = password
-    if password_file is not None:
-        _args['password_file'] = password_file
+    if vault_type is None or vault_type == "symmetric":
+        if password is not None:
+            _args['password'] = password
+        if password_file is not None:
+            _args['password_file'] = password_file
 
-    if private_key is not None:
-        _args['private_key'] = private_key
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
+    if vault_type == "asymmetric":
+        if private_key is not None:
+            _args['private_key'] = private_key
+        if private_key_file is not None:
+            _args['private_key_file'] = private_key_file
 
     if datafile_in is not None:
         _args['in'] = datafile_in
@@ -427,9 +440,6 @@ def data_storage_args(args, data, password, password_file, private_key,
     if datafile_out is not None:
         _args['out'] = datafile_out
 
-    if private_key_file is not None:
-        _args['private_key_file'] = private_key_file
-
     return _args
 
 
@@ -441,7 +451,7 @@ def check_parameters(module, state, action, description, username, service,
                      new_password, new_password_file):
     invalid = []
     if state == "present":
-        invalid = ['private_key', 'private_key_file', 'datafile_out']
+        invalid = ['datafile_out']
 
         if all([password, password_file]) \
            or all([new_password, new_password_file]):
@@ -454,7 +464,7 @@ def check_parameters(module, state, action, description, username, service,
                     "change symmetric vault password.")
 
         if action == "member":
-            invalid.extend(['description'])
+            invalid.extend(['description', 'vault_type'])
 
     elif state == "absent":
         invalid = ['description', 'salt', 'vault_type', 'private_key',
@@ -480,12 +490,6 @@ def check_parameters(module, state, action, description, username, service,
                 msg="Argument '%s' can not be used with state '%s', "
                     "action '%s'" % (arg, state, action))
 
-    for arg in invalid:
-        if vars()[arg] is not None:
-            module.fail_json(
-                msg="Argument '%s' can not be used with state '%s', "
-                    "action '%s'" % (arg, state, action))
-
 
 def check_encryption_params(module, state, action, vault_type, salt,
                             password, password_file, public_key,
@@ -494,6 +498,10 @@ def check_encryption_params(module, state, action, vault_type, salt,
                             new_password, new_password_file, res_find):
     vault_type_invalid = []
 
+    existing_type = None
+    if res_find:
+        existing_type = res_find["ipavaulttype"][0]
+
     if vault_type is None and res_find is not None:
         vault_type = res_find['ipavaulttype']
         if isinstance(vault_type, (tuple, list)):
@@ -536,47 +544,45 @@ def check_encryption_params(module, state, action, vault_type, salt,
                 msg="Assymmetric vault requires public_key "
                     "or public_key_file to store data.")
 
-    for param in vault_type_invalid:
+    valid_fields = []
+    if existing_type == "symmetric":
+        valid_fields = [
+            'password', 'password_file', 'new_password', 'new_password_file',
+            'salt'
+        ]
+    if existing_type == "asymmetric":
+        valid_fields = [
+            'public_key', 'public_key_file', 'private_key', 'private_key_file'
+        ]
+
+    check_fields = [f for f in vault_type_invalid if f not in valid_fields]
+
+    for param in check_fields:
         if vars()[param] is not None:
             module.fail_json(
                 msg="Argument '%s' cannot be used with vault type '%s'" %
                 (param, vault_type or 'symmetric'))
 
 
-def change_password(module, res_find, password, password_file, new_password,
-                    new_password_file):
-    """
-    Change the password of a symmetric vault.
-
-    To change the password of a vault, it is needed to retrieve the stored
-    data with the current password, and store the data again, with the new
-    password, forcing it to override the old one.
-    """
-    # verify parameters.
-    if not any([new_password, new_password_file]):
-        return []
-    if res_find["ipavaulttype"][0] != "symmetric":
-        module.fail_json(msg="Cannot change password of `%s` vault."
-                             % res_find["ipavaulttype"])
-
+def get_stored_data(module, res_find, args):
+    """Retrieve data stored in the vault."""
     # prepare arguments to retrieve data.
     name = res_find["cn"][0]
-    args = {}
-    if password:
-        args["password"] = password
-    if password_file:
-        args["password_file"] = password_file
-    # retrieve current stored data
-    result = api_command(module, 'vault_retrieve', name, args)
+    copy_args = []
+    if res_find['ipavaulttype'][0] == "symmetric":
+        copy_args = ["password", "password_file"]
+    if res_find['ipavaulttype'][0] == "asymmetric":
+        copy_args = ["private_key", "private_key_file"]
 
-    # modify arguments to store data with new password.
-    args = {"override_password": True, "data": result['result']['data']}
-    if new_password:
-        args["password"] = new_password
-    if new_password_file:
-        args["password_file"] = new_password_file
-    # return the command to store data with the new password.
-    return [(name, "vault_archive", args)]
+    pwdargs = {arg: args[arg] for arg in copy_args if arg in args}
+
+    # retrieve vault stored data
+    try:
+        result = api_command(module, 'vault_retrieve', name, pwdargs)
+    except NotFound:
+        return None
+
+    return result['result'].get('data')
 
 
 def main():
@@ -594,10 +600,12 @@ def main():
                             default=None, required=False,
                             choices=["standard", "symmetric", "asymmetric"]),
             vault_public_key=dict(type="str", required=False, default=None,
-                                  aliases=['ipavaultpublickey', 'public_key']),
+                                  aliases=['ipavaultpublickey', 'public_key',
+                                           'new_public_key']),
             vault_public_key_file=dict(type="str", required=False,
                                        default=None,
-                                       aliases=['public_key_file']),
+                                       aliases=['public_key_file',
+                                                'new_public_key_file']),
             vault_private_key=dict(
                 type="str", required=False, default=None, no_log=True,
                 aliases=['ipavaultprivatekey', 'private_key']),
@@ -742,6 +750,11 @@ def main():
             res_find = find_vault(
                 ansible_module, name, username, service, shared)
 
+            # Set default vault_type if needed.
+            res_type = res_find.get('ipavaulttype')[0] if res_find else None
+            if vault_type is None:
+                vault_type = res_type if res_find is not None else u"symmetric"
+
             # Generate args
             args = gen_args(description, username, service, shared, vault_type,
                             salt, password, password_file, public_key,
@@ -749,14 +762,6 @@ def main():
                             datafile_out)
             pwdargs = None
 
-            # Set default vault_type if needed.
-            if vault_type is None and vault_data is not None:
-                if res_find is not None:
-                    res_vault_type = res_find.get('ipavaulttype')[0]
-                    args['ipavaulttype'] = vault_type = res_vault_type
-                else:
-                    args['ipavaulttype'] = vault_type = u"symmetric"
-
             # Create command
             if state == "present":
                 # verify data encription args
@@ -766,16 +771,52 @@ def main():
                     private_key_file, vault_data, datafile_in, datafile_out,
                     new_password, new_password_file, res_find)
 
-                # Found the vault
+                change_passwd = any([
+                    new_password, new_password_file,
+                    (private_key or private_key_file) and
+                    (public_key or public_key_file)
+                ])
                 if action == "vault":
+                    # Found the vault
                     if res_find is not None:
-                        # For all settings is args, check if there are
-                        # different settings in the find result.
-                        # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
-                            commands.append([name, "vault_mod_internal", args])
+                        arg_type = args.get("ipavaulttype")
 
+                        modified = not compare_args_ipa(ansible_module,
+                                                        args, res_find)
+
+                        if arg_type != res_type or change_passwd:
+                            stargs = data_storage_args(
+                                res_type, args, vault_data, password,
+                                password_file, private_key,
+                                private_key_file, datafile_in,
+                                datafile_out)
+                            stored = get_stored_data(
+                                ansible_module, res_find, stargs
+                            )
+                            if stored:
+                                vault_data = \
+                                    (stored or b"").decode("utf-8")
+
+                            remove_attrs = {
+                                "symmetric": ["private_key", "public_key"],
+                                "asymmetric": ["password", "ipavaultsalt"],
+                                "standard": [
+                                    "private_key", "public_key",
+                                    "password", "ipavaultsalt"
+                                ],
+                            }
+                            for attr in remove_attrs.get(arg_type, []):
+                                if attr in args:
+                                    del args[attr]
+
+                            if vault_type == 'symmetric':
+                                if 'ipavaultsalt' not in args:
+                                    args['ipavaultsalt'] = os.urandom(32)
+                            else:
+                                args['ipavaultsalt'] = b''
+
+                        if modified:
+                            commands.append([name, "vault_mod_internal", args])
                     else:
                         if vault_type == 'symmetric' \
                            and 'ipavaultsalt' not in args:
@@ -851,16 +892,22 @@ def main():
                                                      ownerservices)
                         commands.append([name, 'vault_add_owner', owner_args])
 
-                pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
                 if any([vault_data, datafile_in]):
-                    commands.append([name, "vault_archive", pwdargs])
+                    if change_passwd:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, new_password,
+                            new_password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
+                    else:
+                        pwdargs = data_storage_args(
+                            vault_type, args, vault_data, password,
+                            password_file, private_key, private_key_file,
+                            datafile_in, datafile_out)
 
-                cmds = change_password(
-                    ansible_module, res_find, password, password_file,
-                    new_password, new_password_file)
-                commands.extend(cmds)
+                    pwdargs['override_password'] = True
+                    pwdargs.pop("private_key", None)
+                    pwdargs.pop("private_key_file", None)
+                    commands.append([name, "vault_archive", pwdargs])
 
             elif state == "retrieved":
                 if res_find is None:
@@ -875,8 +922,9 @@ def main():
                     new_password, new_password_file, res_find)
 
                 pwdargs = data_storage_args(
-                    args, vault_data, password, password_file, private_key,
-                    private_key_file, datafile_in, datafile_out)
+                    res_find["ipavaulttype"][0], args, vault_data, password,
+                    password_file, private_key, private_key_file, datafile_in,
+                    datafile_out)
                 if 'data' in pwdargs:
                     del pwdargs['data']
 
@@ -888,6 +936,10 @@ def main():
 
                 if action == "vault":
                     if res_find is not None:
+                        remove = ['ipavaultsalt', 'ipavaultpublickey']
+                        args = {
+                            k: v for k, v in args.items() if k not in remove
+                        }
                         commands.append([name, "vault_del", args])
 
                 elif action == "member":

tests/vault/env_cleanup.yml

@@ -38,35 +38,27 @@
       name: vaultgroup
       state: absent
 
-  - name: Remove password file from target host.
+  - name: Remove files from target host.
     file:
-      path: "{{ ansible_env.HOME }}/password.txt"
+      path: "{{ ansible_env.HOME }}/{{ item }}"
       state: absent
+    with_items:
+    - private.pem
+    - public.pem
+    - old_private.pem
+    - old_public.pem
+    - password.txt
+    - data.txt
+    - in.txt
 
-  - name: Remove public key file from target host.
+  - name: Remove files from controller.
     file:
-      path: "{{ ansible_env.HOME }}/public.pem"
+      path: "{{ playbook_dir }}/{{ item }}"
       state: absent
-
-  - name: Remove private key file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/private.pem"
-      state: absent
-
-  - name: Remove output data file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/data.txt"
-      state: absent
-
-  - name: Remove input data file from target host.
-    file:
-      path: "{{ ansible_env.HOME }}/in.txt"
-      state: absent
-
-  - name: Remove private/public key files.
-    shell:
-      cmd: rm -f private.pem public.pem
     delegate_to: localhost
     become: no
-    args:
-      warn: no  # suppres warning for not using the `file` module.
+    with_items:
+    - private.pem
+    - public.pem
+    - old_private.pem
+    - old_public.pem

tests/vault/env_setup.yml

@@ -3,37 +3,28 @@
   - name: Ensure environment is clean.
     import_tasks: env_cleanup.yml
 
-  - name: Create private key file.
+  - name: Create private/public key pair.
     shell:
-      cmd: openssl genrsa -out private.pem 2048
+      cmd: |
+        openssl genrsa -out "{{ item }}private.pem" 2048
+        openssl rsa -in "{{ item }}private.pem" -outform PEM -pubout -out "{{ item }}public.pem"
     delegate_to: localhost
     become: no
+    with_items:
+    - ""
+    - old_
 
-  - name: Create public key file.
-    shell:
-      cmd: openssl rsa -in private.pem -outform PEM -pubout -out public.pem
-    delegate_to: localhost
-    become: no
-
-  - name: Copy password file to target host.
+  - name: Copy files to target host.
     copy:
-      src: "{{ playbook_dir }}/password.txt"
-      dest: "{{ ansible_env.HOME }}/password.txt"
-
-  - name: Copy public key file to target host.
-    copy:
-      src: "{{ playbook_dir }}/public.pem"
-      dest: "{{ ansible_env.HOME }}/public.pem"
-
-  - name: Copy private key file to target host.
-    copy:
-      src: "{{ playbook_dir }}/private.pem"
-      dest: "{{ ansible_env.HOME }}/private.pem"
-
-  - name: Copy input data file to target host.
-    copy:
-      src: "{{ playbook_dir }}/in.txt"
-      dest: "{{ ansible_env.HOME }}/in.txt"
+      src: "{{ playbook_dir }}/{{ item }}"
+      dest: "{{ ansible_env.HOME }}/{{ item }}"
+    with_items:
+    - private.pem
+    - public.pem
+    - old_private.pem
+    - old_public.pem
+    - password.txt
+    - in.txt
 
   - name: Ensure vaultgroup exists.
     ipagroup:

tests/vault/test_vault_asymmetric.yml

@@ -14,18 +14,111 @@
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       vault_type: asymmetric
-      public_key: "{{ lookup('file', 'public.pem', rstrip=False) | b64encode }}"
+      public_key: "{{ lookup('file', 'old_public.pem', rstrip=True) | b64encode }}"
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure asymmetric vault is present, again
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       vault_type: asymmetric
-      public_key: "{{ lookup('file', 'public.pem', rstrip=False) | b64encode }}"
+      public_key: "{{ lookup('file', 'old_public.pem', rstrip=True) | b64encode }}"
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
+
+  - name: Archive data to asymmetric vault using "old" key.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_data: SomeValue
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using "old" key.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'old_private.pem', rstrip=True) | b64encode }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to "new" key.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'public.pem', rstrip=True) | b64encode }}"
+      private_key: "{{ lookup('file', 'old_private.pem', rstrip=True) | b64encode }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using "new" key.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key from_file to "old"
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key_file: old_public.pem
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault using old key file.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key_file: old_private.pem
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to "new" key, using only files
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key_file: public.pem
+      private_key_file: old_private.pem
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault, using new "key".
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'SomeValue'
+
+  - name: Change asymmetric vault key to A, without specifying vault_type.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'A_public.b64') }}"
+      private_key: "{{ lookup('file', 'B_private.b64') }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Change asymmetric vault key to B, with key files, without specifying vault_type.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: asymvault
+      public_key_file: "{{ ansible_env.HOME }}/B_public.pem"
+      private_key_file: "{{ ansible_env.HOME }}/A_private.pem"
+    register: result
+    failed_when: result.failed or not result.changed
 
   - name: Archive data to asymmetric vault, matching `no_log` field.
     ipavault:
@@ -39,12 +132,12 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'SomeADMINpassword' or result.changed
 
-  - name: Archive data to asymmetric vault
+  - name: Change data in asymmetric vault
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
@@ -52,11 +145,11 @@
     register: result
     failed_when: not result.changed
 
-  - name: Retrieve data from asymmetric vault.
+  - name: Retrieve changed data from asymmetric vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Hello World.' or result.changed
@@ -66,7 +159,7 @@
       ipaadmin_password: SomeADMINpassword
       name: asymvault
       out: "{{ ansible_env.HOME }}/data.txt"
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.changed or result.failed or (result.vault.data | default(false))
@@ -89,7 +182,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'The world of π is half rounded.' or result.changed
@@ -107,7 +200,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Another World.' or result.changed
@@ -124,7 +217,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'c' or result.changed
@@ -175,7 +268,7 @@
     ipavault:
       ipaadmin_password: SomeADMINpassword
       name: asymvault
-      private_key: "{{ lookup('file', 'private.pem', rstrip=False) | b64encode }}"
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
       state: retrieved
     register: result
     failed_when: result.vault.data != 'Hello World.' or result.changed
@@ -206,4 +299,4 @@
     failed_when: result.changed
 
   - name: Cleanup testing environment.
-    import_tasks: env_setup.yml
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_change_type.yml

@@ -0,0 +1,304 @@
+---
+- name: Test vault
+  hosts: ipaserver
+  become: true
+  # Need to gather facts for ansible_env.
+  gather_facts: true
+
+  tasks:
+  - name: Setup testing environment.
+    import_tasks: env_setup.yml
+
+  - name: Ensure test_vault is absent.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Create standard vault with no data archived.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: standard
+
+  - name: Change from standard to asymmetric
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'public.pem', rstrip=True) | b64encode }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - block:
+    - name: Change from asymmetric to symmetric
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: symmetric
+        private_key: "{{ lookup('file', 'private.pem') | b64encode }}"
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - block:
+    - name: Change from symmetric to standard
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Change from standard to symmetric
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: symmetric
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - block:
+    - name: Change from symmetric to asymmetric
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: asymmetric
+        password: SomeVAULTpassword
+        public_key: "{{ lookup('file', 'public.pem') | b64encode }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - block:
+    - name: Change from asymmetric to standard
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        private_key: "{{ lookup('file', 'private.pem') | b64encode }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Ensure test_vault is absent.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Create standard vault with data archived.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: standard
+      data: hello
+
+  - name: Change from standard to asymmetric, with data
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: asymmetric
+      public_key: "{{ lookup('file', 'public.pem', rstrip=True) | b64encode }}"
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from asymmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from asymmetric to symmetric, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: symmetric
+        private_key: "{{ lookup('file', 'private.pem') | b64encode }}"
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Retrieve data from symmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      password: SomeVAULTpassword
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from symmetric to standard, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        password: SomeVAULTpassword
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Retrieve data from standard vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - name: Change from standard to symmetric, with data
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      vault_type: symmetric
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or not result.changed
+
+  - name: Retrieve data from symmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+      password: SomeVAULTpassword
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from symmetric to asymmetric, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: asymmetric
+        password: SomeVAULTpassword
+        public_key: "{{ lookup('file', 'public.pem') | b64encode }}"
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Verify salt is not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Salt:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_symmetric
+
+  - name: Retrieve data from asymmetric vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+      private_key: "{{ lookup('file', 'private.pem', rstrip=True) | b64encode }}"
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - block:
+    - name: Change from asymmetric to standard, with data
+      ipavault:
+        ipaadmin_password: SomeADMINpassword
+        name: test_vault
+        vault_type: standard
+        private_key: "{{ lookup('file', 'private.pem') | b64encode }}"
+      register: result
+      failed_when: result.failed or not result.changed or result.failed
+
+    - name: Verify assymetric-only fields are not present.
+      shell: |
+         echo SomeADMINpassword | kinit -c {{ KRB5CCNAME }} admin
+         KRB5CCNAME={{ KRB5CCNAME }} ipa vault-show test_vault
+         kdestroy -A -q -c {{ KRB5CCNAME }}
+      register: result
+      failed_when: result.failed or "Public Key:" in result.stdout
+
+    vars:
+      KRB5CCNAME: verify_change_from_asymmetric
+
+  - name: Retrieve data from standard vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: retrieved
+    register: result
+    failed_when: result.failed or result.changed or result.vault.data != 'hello'
+
+  - name: Remove test_vault.
+    ipavault:
+      ipaadmin_password: SomeADMINpassword
+      name: test_vault
+      state: absent
+
+  - name: Cleanup testing environment.
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_standard.yml

@@ -138,4 +138,4 @@
     failed_when: result.changed
 
   - name: Cleanup testing environment.
-    import_tasks: env_setup.yml
+    import_tasks: env_cleanup.yml

tests/vault/test_vault_symmetric.yml

@@ -43,7 +43,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'SomeADMINpassword' or result.changed
+    failed_when: result.changed or result.failed or result.vault.data != 'SomeADMINpassword'
 
   - name: Archive data to symmetric vault
     ipavault:
@@ -61,7 +61,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.changed or result.failed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault into file {{ ansible_env.HOME }}/data.txt.
     ipavault:
@@ -86,7 +86,7 @@
       password: SomeVAULTpassword
       vault_data: The world of π is half rounded.
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -95,7 +95,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'The world of π is half rounded.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'The world of π is half rounded.'
 
   - name: Archive data in symmetric vault, from file.
     ipavault:
@@ -104,7 +104,7 @@
       in: "{{ ansible_env.HOME }}/in.txt"
       password: SomeVAULTpassword
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -113,7 +113,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Another World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Another World.'
 
   - name: Archive data with single character to symmetric vault
     ipavault:
@@ -122,7 +122,7 @@
       password: SomeVAULTpassword
       vault_data: c
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -131,7 +131,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'c' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'c'
 
   - name: Ensure symmetric vault is absent
     ipavault:
@@ -139,7 +139,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is absent, again
     ipavault:
@@ -147,7 +147,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Ensure symmetric vault is present, with password from file.
     ipavault:
@@ -157,7 +157,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       vault_type: symmetric
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is present, with password from file, again.
     ipavault:
@@ -167,7 +167,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       vault_type: symmetric
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Archive data to symmetric vault
     ipavault:
@@ -176,7 +176,7 @@
       vault_data: Hello World.
       password: SomeVAULTpassword
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Retrieve data from symmetric vault.
     ipavault:
@@ -185,7 +185,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with password file.
     ipavault:
@@ -194,7 +194,7 @@
       password_file: "{{ ansible_env.HOME }}/password.txt"
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with wrong password.
     ipavault:
@@ -203,7 +203,7 @@
       password: SomeWRONGpassword
       state: retrieved
     register: result
-    failed_when: not result.failed or "Invalid credentials" not in result.msg
+    failed_when: result.changed or not result.failed or "Invalid credentials" not in result.msg
 
   - name: Change vault password.
     ipavault:
@@ -212,7 +212,7 @@
       password: SomeVAULTpassword
       new_password: SomeNEWpassword
     register: result
-    failed_when: not result.changed
+    failed_when: not result.changed or result.failed
 
   - name: Retrieve data from symmetric vault, with new password.
     ipavault:
@@ -221,7 +221,7 @@
       password: SomeNEWpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Retrieve data from symmetric vault, with old password.
     ipavault:
@@ -240,7 +240,7 @@
       new_password: SomeVAULTpassword
       salt: AAAAAAAAAAAAAAAAAAAAAAA=
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Change symmetric vault salt, without changing password
     ipavault:
@@ -250,7 +250,7 @@
       new_password: SomeVAULTpassword
       salt: MTIzNDU2Nzg5MDEyMzQ1Ngo=
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Try to change symmetric vault salt, without providing any password
     ipavault:
@@ -258,7 +258,7 @@
       name: symvault
       salt: MTIzNDU2Nzg5MDEyMzQ1Ngo=
     register: result
-    failed_when: not result.failed and  "Vault `salt` can only change when changing the password." not in result.msg
+    failed_when: not result.failed and "Vault `salt` can only change when changing the password." not in result.msg
 
   - name: Try to change symmetric vault salt, without providing `password`
     ipavault:
@@ -294,7 +294,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Ensure symmetric vault is absent, again
     ipavault:
@@ -302,7 +302,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: result.changed
+    failed_when: result.failed or result.changed
 
   - name: Try to change password of inexistent vault.
     ipavault:
@@ -340,7 +340,7 @@
       password: SomeVAULTpassword
       state: retrieved
     register: result
-    failed_when: result.vault.data != 'Hello World.' or result.changed
+    failed_when: result.failed or result.changed or result.vault.data != 'Hello World.'
 
   - name: Ensure symmetric vault is absent
     ipavault:
@@ -348,7 +348,7 @@
       name: symvault
       state: absent
     register: result
-    failed_when: not result.changed
+    failed_when: result.failed or not result.changed
 
   - name: Cleanup testing environment.
     import_tasks: env_cleanup.yml