From f8ad9ca75d9674fc12c25d4d8d31bfffcba87a8b Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Wed, 20 Sep 2017 11:00:42 -0400 Subject: [PATCH] Don't ask for password confirm on 'ansible-vault edit' (#30514) This is to match the 2.3 behavior on: ansible-vault edit encrypted_file.yml Previously, the above command would consider that a 'new password' scenario and prompt accordingly, ie: $ ansible-vault edit encrypted_file.yml New Password: Confirm New Password: The bug was cause by 'create_new_password' being used for 'edit' action. This also causes the previous implicit 'auto prompt' to get triggered and prompt the user. Fix is to make auto prompt explicit in the calling code to handle the 'edit' case where we want to auto prompt but we do not want to request a password confirm. Fixes #30491 (cherry picked from commit 307be59092a9979d4471600ea99010dff104967a) --- CHANGELOG.md | 1 + lib/ansible/cli/__init__.py | 14 +++++++++----- lib/ansible/cli/vault.py | 4 ++-- test/units/cli/test_cli.py | 32 +++++++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67221069e7f..b4e0fa4cff7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Ansible Changes By Release * Fix for Ansible.ModuleUtils.CamelConversion to handle empty lists and lists with one entry * Fix nxos terminal regex to parse username correctly. * Fix colors for selective callback +* Fix for 'New password' prompt on 'ansible-vault edit' (https://github.com/ansible/ansible/issues/30491) diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py index fb1eb0f5855..4e1bd23aa06 100644 --- a/lib/ansible/cli/__init__.py +++ b/lib/ansible/cli/__init__.py @@ -196,7 +196,8 @@ class CLI(with_metaclass(ABCMeta, object)): @staticmethod def build_vault_ids(vault_ids, vault_password_files=None, - ask_vault_pass=None, create_new_password=None): + ask_vault_pass=None, create_new_password=None, + auto_prompt=True): vault_password_files = vault_password_files or [] vault_ids = vault_ids or [] @@ -211,7 +212,7 @@ class CLI(with_metaclass(ABCMeta, object)): # if an action needs an encrypt password (create_new_password=True) and we dont # have other secrets setup, then automatically add a password prompt as well. - if ask_vault_pass or (create_new_password and not vault_ids): + if ask_vault_pass or (auto_prompt and not vault_ids): id_slug = u'%s@%s' % (C.DEFAULT_VAULT_IDENTITY, u'prompt_ask_vault_pass') vault_ids.append(id_slug) @@ -220,7 +221,8 @@ class CLI(with_metaclass(ABCMeta, object)): # TODO: remove the now unused args @staticmethod def setup_vault_secrets(loader, vault_ids, vault_password_files=None, - ask_vault_pass=None, create_new_password=False): + ask_vault_pass=None, create_new_password=False, + auto_prompt=True): # list of tuples vault_secrets = [] @@ -250,7 +252,8 @@ class CLI(with_metaclass(ABCMeta, object)): vault_ids = CLI.build_vault_ids(vault_ids, vault_password_files, ask_vault_pass, - create_new_password) + create_new_password, + auto_prompt=auto_prompt) for vault_id_slug in vault_ids: vault_id_name, vault_id_value = CLI.split_vault_id(vault_id_slug) @@ -777,7 +780,8 @@ class CLI(with_metaclass(ABCMeta, object)): vault_secrets = CLI.setup_vault_secrets(loader, vault_ids=vault_ids, vault_password_files=options.vault_password_files, - ask_vault_pass=options.ask_vault_pass) + ask_vault_pass=options.ask_vault_pass, + auto_prompt=False) loader.set_vault_secrets(vault_secrets) # create the inventory, and filter it based on the subset specified (if any) diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py index efd7d887e8c..762ba82b983 100644 --- a/lib/ansible/cli/vault.py +++ b/lib/ansible/cli/vault.py @@ -165,7 +165,7 @@ class VaultCLI(CLI): # TODO: instead of prompting for these before, we could let VaultEditor # call a callback when it needs it. - if self.action in ['decrypt', 'view', 'rekey']: + if self.action in ['decrypt', 'view', 'rekey', 'edit']: vault_secrets = self.setup_vault_secrets(loader, vault_ids=vault_ids, vault_password_files=self.options.vault_password_files, @@ -173,7 +173,7 @@ class VaultCLI(CLI): if not vault_secrets: raise AnsibleOptionsError("A vault password is required to use Ansible's Vault") - if self.action in ['encrypt', 'encrypt_string', 'create', 'edit']: + if self.action in ['encrypt', 'encrypt_string', 'create']: if len(vault_ids) > 1: raise AnsibleOptionsError("Only one --vault-id can be used for encryption") diff --git a/test/units/cli/test_cli.py b/test/units/cli/test_cli.py index 39ae28c8713..c881cf78c29 100644 --- a/test/units/cli/test_cli.py +++ b/test/units/cli/test_cli.py @@ -54,6 +54,32 @@ class TestCliBuildVaultIds(unittest.TestCase): res = cli.CLI.build_vault_ids([], create_new_password=True) self.assertEqual(res, ['default@prompt_ask_vault_pass']) + def test_create_new_password_no_vault_id_no_auto_prompt(self): + res = cli.CLI.build_vault_ids([], auto_prompt=False, create_new_password=True) + self.assertEqual(res, []) + + def test_no_vault_id_no_auto_prompt(self): + # similate 'ansible-playbook site.yml' with out --ask-vault-pass, should not prompt + res = cli.CLI.build_vault_ids([], auto_prompt=False) + self.assertEqual(res, []) + + def test_no_vault_ids_auto_prompt(self): + # create_new_password=False + # simulate 'ansible-vault edit encrypted.yml' + res = cli.CLI.build_vault_ids([], auto_prompt=True) + self.assertEqual(res, ['default@prompt_ask_vault_pass']) + + def test_no_vault_ids_auto_prompt_ask_vault_pass(self): + # create_new_password=False + # simulate 'ansible-vault edit --ask-vault-pass encrypted.yml' + res = cli.CLI.build_vault_ids([], auto_prompt=True, ask_vault_pass=True) + self.assertEqual(res, ['default@prompt_ask_vault_pass']) + + def test_create_new_password_auto_prompt(self): + # simulate 'ansible-vault encrypt somefile.yml' + res = cli.CLI.build_vault_ids([], auto_prompt=True, create_new_password=True) + self.assertEqual(res, ['default@prompt_ask_vault_pass']) + def test_create_new_password_no_vault_id_ask_vault_pass(self): res = cli.CLI.build_vault_ids([], ask_vault_pass=True, create_new_password=True) @@ -74,7 +100,8 @@ class TestCliBuildVaultIds(unittest.TestCase): vault_password_files=['yet-another-password-file', 'one-more-password-file'], ask_vault_pass=True, - create_new_password=True) + create_new_password=True, + auto_prompt=False) self.assertEqual(set(res), set(['blip@prompt', 'baz@prompt_ask_vault_pass', 'default@prompt_ask_vault_pass', 'some-password-file', 'qux@another-password-file', @@ -92,7 +119,7 @@ class TestCliSetupVaultSecrets(unittest.TestCase): self.tty_patcher.stop() def test(self): - res = cli.CLI.setup_vault_secrets(None, None) + res = cli.CLI.setup_vault_secrets(None, None, auto_prompt=False) self.assertIsInstance(res, list) @patch('ansible.cli.get_file_vault_secret') @@ -134,7 +161,6 @@ class TestCliSetupVaultSecrets(unittest.TestCase): vault_ids=['prompt1@prompt'], ask_vault_pass=True) - print('res: %s' % res) self.assertIsInstance(res, list) self.assertEqual(len(res), 0) matches = vault.match_secrets(res, ['prompt1'])