From 93c2852fdba34f14b294b38dcd27a0a8d94fe053 Mon Sep 17 00:00:00 2001 From: osirisinferi Date: Mon, 27 Dec 2021 09:12:52 +0100 Subject: [PATCH] Add `show_account` subcommand to retrieve account info from ACME server (#9127) * Fetch and print account contacts from ACME server * Add tests * Add changelog entryAdd changelog entry * Add account URI and thumbprint output Only show these items when verbosity > 0 * Add test case for account URI and thumbprint * Move changelog entry to new placeholder * Add test for `cb_client.acme` (coverage) * Address comments * Update changelog * Few small word changes * Add server to error messages * Remove phone contact parts --- certbot/CHANGELOG.md | 4 +- .../certbot/_internal/cli/cli_constants.py | 1 + certbot/certbot/_internal/cli/helpful.py | 1 + certbot/certbot/_internal/cli/paths_parser.py | 2 +- certbot/certbot/_internal/cli/verb_help.py | 5 + certbot/certbot/_internal/main.py | 55 ++++++++- certbot/tests/main_test.py | 107 +++++++++++++++++- 7 files changed, 167 insertions(+), 8 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 95c6e6f29..b43154db7 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,7 +6,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* Added `show_account` subcommand, which will fetch the account information + from the ACME server and show the account details (account URL and, if + applicable, email address or addresses) ### Changed diff --git a/certbot/certbot/_internal/cli/cli_constants.py b/certbot/certbot/_internal/cli/cli_constants.py index df815f2e6..64b94a8ab 100644 --- a/certbot/certbot/_internal/cli/cli_constants.py +++ b/certbot/certbot/_internal/cli/cli_constants.py @@ -43,6 +43,7 @@ manage your account: register Create an ACME account unregister Deactivate an ACME account update_account Update an ACME account + show_account Display account details --agree-tos Agree to the ACME server's Subscriber Agreement -m EMAIL Email address for important account notifications """ diff --git a/certbot/certbot/_internal/cli/helpful.py b/certbot/certbot/_internal/cli/helpful.py index 2c57178bb..cf3615fd4 100644 --- a/certbot/certbot/_internal/cli/helpful.py +++ b/certbot/certbot/_internal/cli/helpful.py @@ -56,6 +56,7 @@ class HelpfulArgumentParser: "plugins": main.plugins_cmd, "register": main.register, "update_account": main.update_account, + "show_account": main.show_account, "unregister": main.unregister, "renew": main.renew, "revoke": main.revoke, diff --git a/certbot/certbot/_internal/cli/paths_parser.py b/certbot/certbot/_internal/cli/paths_parser.py index 69a112cfa..de199c568 100644 --- a/certbot/certbot/_internal/cli/paths_parser.py +++ b/certbot/certbot/_internal/cli/paths_parser.py @@ -46,5 +46,5 @@ def _paths_parser(helpful: "helpful.HelpfulArgumentParser") -> None: help=config_help("work_dir")) add("paths", "--logs-dir", default=flag_default("logs_dir"), help="Logs directory.") - add("paths", "--server", default=flag_default("server"), + add(["paths", "show_account"], "--server", default=flag_default("server"), help=config_help("server")) diff --git a/certbot/certbot/_internal/cli/verb_help.py b/certbot/certbot/_internal/cli/verb_help.py index 1319c6358..b15be4631 100644 --- a/certbot/certbot/_internal/cli/verb_help.py +++ b/certbot/certbot/_internal/cli/verb_help.py @@ -97,6 +97,11 @@ VERB_HELP = [ "to already existing configuration."), "usage": "\n\n certbot enhance [options]\n\n" }), + ("show_account", { + "short": "Show account details from an ACME server", + "opts": 'Options useful for the "show_account" subcommand:', + "usage": "\n\n certbot show_account [options]\n\n" + }), ] diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 9c29b9560..15a7239f6 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -811,7 +811,7 @@ def unregister(config: configuration.NamespaceConfig, accounts = account_storage.find_all() if not accounts: - return "Could not find existing account to deactivate." + return f"Could not find existing account for server {config.server}." prompt = ("Are you sure you would like to irrevocably deactivate " "your account?") wants_deactivate = display_util.yesno(prompt, yes_label='Deactivate', no_label='Abort', @@ -846,7 +846,7 @@ def register(config: configuration.NamespaceConfig, :param unused_plugins: List of plugins (deprecated) :type unused_plugins: plugins_disco.PluginsRegistry - :returns: `None` or a string indicating and error + :returns: `None` or a string indicating an error :rtype: None or str """ @@ -877,7 +877,7 @@ def update_account(config: configuration.NamespaceConfig, :param unused_plugins: List of plugins (deprecated) :type unused_plugins: plugins_disco.PluginsRegistry - :returns: `None` or a string indicating and error + :returns: `None` or a string indicating an error :rtype: None or str """ @@ -887,7 +887,7 @@ def update_account(config: configuration.NamespaceConfig, accounts = account_storage.find_all() if not accounts: - return "Could not find an existing account to update." + return f"Could not find an existing account for server {config.server}." if config.email is None and not config.register_unsafely_without_email: config.email = display_ops.get_email(optional=False) @@ -921,6 +921,53 @@ def update_account(config: configuration.NamespaceConfig, return None +def show_account(config: configuration.NamespaceConfig, + unused_plugins: plugins_disco.PluginsRegistry) -> Optional[str]: + """Fetch account info from the ACME server and show it to the user. + + :param config: Configuration object + :type config: configuration.NamespaceConfig + + :param unused_plugins: List of plugins (deprecated) + :type unused_plugins: plugins_disco.PluginsRegistry + + :returns: `None` or a string indicating an error + :rtype: None or str + + """ + # Portion of _determine_account logic to see whether accounts already + # exist or not. + account_storage = account.AccountFileStorage(config) + accounts = account_storage.find_all() + + if not accounts: + return f"Could not find an existing account for server {config.server}." + + acc, acme = _determine_account(config) + cb_client = client.Client(config, acc, None, None, acme=acme) + + if not cb_client.acme: + raise errors.Error("ACME client is not set.") + + regr = cb_client.acme.query_registration(acc.regr) + output = [f"Account details for server {config.server}:", + f" Account URL: {regr.uri}"] + + emails = [] + + for contact in regr.body.contact: + if contact.startswith('mailto:'): + emails.append(contact[7:]) + + output.append(" Email contact{}: {}".format( + "s" if len(emails) > 1 else "", + ", ".join(emails) if len(emails) > 0 else "none")) + + display_util.notify("\n".join(output)) + + return None + + def _cert_name_from_config_or_lineage(config: configuration.NamespaceConfig, lineage: Optional[storage.RenewableCert]) -> Optional[str]: if lineage: diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 8c4ab883e..9a67f0853 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1595,10 +1595,11 @@ class UnregisterTest(unittest.TestCase): self.mocks['client'].Client.return_value = cb_client config = mock.MagicMock() + config.server = "https://acme.example.com/directory" unused_plugins = mock.MagicMock() res = main.unregister(config, unused_plugins) - m = "Could not find existing account to deactivate." + m = "Could not find existing account for server https://acme.example.com/directory." self.assertEqual(res, m) self.assertIs(cb_client.acme.deactivate_registration.called, False) @@ -2025,7 +2026,8 @@ class UpdateAccountTest(test_util.ConfigTestCase): mock_storage.find_all.return_value = [] self.mocks['account'].AccountFileStorage.return_value = mock_storage self.assertEqual(self._call(['update_account', '--email', 'user@example.org']), - 'Could not find an existing account to update.') + 'Could not find an existing account for server' + ' https://acme-v02.api.letsencrypt.org/directory.') def test_update_account_remove_email(self): """Test that --register-unsafely-without-email is handled as no email""" @@ -2070,5 +2072,106 @@ class UpdateAccountTest(test_util.ConfigTestCase): 'Your e-mail address was updated to user@example.com,user@example.org.') +class ShowAccountTest(test_util.ConfigTestCase): + """Tests for certbot._internal.main.show_account""" + + def setUp(self): + patches = { + 'account': mock.patch('certbot._internal.main.account'), + 'atexit': mock.patch('certbot.util.atexit'), + 'client': mock.patch('certbot._internal.main.client'), + 'determine_account': mock.patch('certbot._internal.main._determine_account'), + 'notify': mock.patch('certbot._internal.main.display_util.notify'), + 'util': test_util.patch_display_util() + } + self.mocks = { k: patches[k].start() for k in patches } + for patch in patches.values(): + self.addCleanup(patch.stop) + + return super().setUp() + + def _call(self, args): + with mock.patch('certbot._internal.main.sys.stdout'), \ + mock.patch('certbot._internal.main.sys.stderr'): + args = ['--config-dir', self.config.config_dir, + '--work-dir', self.config.work_dir, + '--logs-dir', self.config.logs_dir, '--text'] + args + return main.main(args[:]) # NOTE: parser can alter its args! + + def _prepare_mock_account(self): + mock_storage = mock.MagicMock() + mock_account = mock.MagicMock() + mock_regr = mock.MagicMock() + mock_storage.find_all.return_value = [mock_account] + self.mocks['account'].AccountFileStorage.return_value = mock_storage + mock_account.regr.body = mock_regr.body + self.mocks['determine_account'].return_value = (mock_account, mock.MagicMock()) + + def _test_show_account(self, contact): + self._prepare_mock_account() + mock_client = mock.MagicMock() + mock_regr = mock.MagicMock() + mock_regr.body.contact = contact + mock_regr.uri = 'https://www.letsencrypt-demo.org/acme/reg/1' + mock_regr.body.key.thumbprint.return_value = b'foobarbaz' + mock_client.acme.query_registration.return_value = mock_regr + self.mocks['client'].Client.return_value = mock_client + + args = ['show_account'] + + self._call(args) + + self.assertEqual(mock_client.acme.query_registration.call_count, 1) + + def test_no_existing_accounts(self): + """Test that no existing account is handled correctly""" + mock_storage = mock.MagicMock() + mock_storage.find_all.return_value = [] + self.mocks['account'].AccountFileStorage.return_value = mock_storage + self.assertEqual(self._call(['show_account']), + 'Could not find an existing account for server' + ' https://acme-v02.api.letsencrypt.org/directory.') + + def test_no_existing_client(self): + """Test that issues with the ACME client are handled correctly""" + self._prepare_mock_account() + mock_client = mock.MagicMock() + mock_client.acme = None + self.mocks['client'].Client.return_value = mock_client + try: + self._call(['show_account']) + except errors.Error as e: + self.assertEqual('ACME client is not set.', str(e)) + + def test_no_contacts(self): + self._test_show_account(()) + + self.assertEqual(self.mocks['notify'].call_count, 1) + self.mocks['notify'].assert_has_calls([ + mock.call('Account details for server https://acme-v02.api.letsencr' + 'ypt.org/directory:\n Account URL: https://www.letsencry' + 'pt-demo.org/acme/reg/1\n Email contact: none')]) + + def test_single_email(self): + contact = ('mailto:foo@example.com',) + self._test_show_account(contact) + + self.assertEqual(self.mocks['notify'].call_count, 1) + self.mocks['notify'].assert_has_calls([ + mock.call('Account details for server https://acme-v02.api.letsencr' + 'ypt.org/directory:\n Account URL: https://www.letsencry' + 'pt-demo.org/acme/reg/1\n Email contact: foo@example.com')]) + + def test_double_email(self): + contact = ('mailto:foo@example.com', 'mailto:bar@example.com') + self._test_show_account(contact) + + self.assertEqual(self.mocks['notify'].call_count, 1) + self.mocks['notify'].assert_has_calls([ + mock.call('Account details for server https://acme-v02.api.letsencr' + 'ypt.org/directory:\n Account URL: https://www.letsencry' + 'pt-demo.org/acme/reg/1\n Email contacts: foo@example.com, bar@example.com')]) + + if __name__ == '__main__': unittest.main() # pragma: no cover