From c323af7be9fc90956b607e3ced6176db87af1592 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 6 Mar 2025 14:38:52 -0800 Subject: [PATCH] Remove warnings about empty email. (#10214) We strongly encouraged providing an email address because we wanted people to get expiration notices to ensure that even if their Certbot install broke, they could fix it before their site goes down. Now that Let's Encrypt is getting rid of expiration notices (https://letsencrypt.org/2025/01/22/ending-expiration-emails/), we can remove some of the encouragement, providing a smoother user experience. --- certbot/CHANGELOG.md | 2 + certbot/certbot/_internal/cli/__init__.py | 6 +-- certbot/certbot/_internal/client.py | 10 +--- certbot/certbot/_internal/main.py | 2 +- .../certbot/_internal/tests/client_test.py | 6 +-- .../_internal/tests/display/ops_test.py | 16 ++----- certbot/certbot/_internal/tests/main_test.py | 2 - certbot/certbot/display/ops.py | 47 +++++-------------- 8 files changed, 23 insertions(+), 68 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index fc7613847..7cfb7f827 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -10,6 +10,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed +* The --register-unsafely-without-email flag is no longer needed in non-interactive mode. +* In interactive mode, pressing Enter at the email prompt will register without an email. * deprecated `acme.crypto_util.dump_pyopenssl_chain` * deprecated `acme.crypto_util._pyopenssl_cert_or_req_all_names` * deprecated `acme.crypto_util._pyopenssl_cert_or_req_san` diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py index b5240ed2e..302a49e91 100644 --- a/certbot/certbot/_internal/cli/__init__.py +++ b/certbot/certbot/_internal/cli/__init__.py @@ -171,11 +171,7 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[st helpful.add( ["register", "automation"], "--register-unsafely-without-email", action="store_true", default=flag_default("register_unsafely_without_email"), - help="Specifying this flag enables registering an account with no " - "email address. This is strongly discouraged, because you will be " - "unable to receive notice about impending expiration or " - "revocation of your certificates or problems with your Certbot " - "installation that will lead to failure to renew.") + help=argparse.SUPPRESS) helpful.add( ["register", "update_account", "unregister", "automation"], "-m", "--email", default=flag_default("email"), diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index e51842be6..f794eef8c 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -186,15 +186,9 @@ def register(config: configuration.NamespaceConfig, account_storage: AccountStor # Log non-standard actions, potentially wrong API calls if account_storage.find_all(): logger.info("There are already existing accounts for %s", config.server) - if config.email is None: - if not config.register_unsafely_without_email: - msg = ("No email was provided and " - "--register-unsafely-without-email was not present.") - logger.error(msg) - raise errors.Error(msg) - if not config.dry_run: - logger.debug("Registering without email!") + if config.email == "": + config.email = None # If --dry-run is used, and there is no staging account, create one with no email. if config.dry_run: config.email = None diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index ce44bd6ab..c1352b49e 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -932,7 +932,7 @@ def update_account(config: configuration.NamespaceConfig, if not accounts: 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) + config.email = display_ops.get_email() acc, acme = _determine_account(config) cb_client = client.Client(config, acc, None, None, acme=acme) diff --git a/certbot/certbot/_internal/tests/client_test.py b/certbot/certbot/_internal/tests/client_test.py index 75ddfa948..ad06533a5 100644 --- a/certbot/certbot/_internal/tests/client_test.py +++ b/certbot/certbot/_internal/tests/client_test.py @@ -158,9 +158,10 @@ class RegisterTest(test_util.ConfigTestCase): with pytest.raises(errors.Error): self._call() - def test_needs_email(self): + def test_no_email_is_chill(self): self.config.email = None - with pytest.raises(errors.Error): + with self._patched_acme_client() as mock_client: + mock_client().external_account_required.side_effect = self._false_mock self._call() @mock.patch("certbot._internal.client.logger") @@ -172,7 +173,6 @@ class RegisterTest(test_util.ConfigTestCase): self.config.register_unsafely_without_email = True self.config.dry_run = False self._call() - mock_logger.debug.assert_called_once_with(mock.ANY) assert mock_prepare.called is True @mock.patch("certbot._internal.client.display_ops.get_email") diff --git a/certbot/certbot/_internal/tests/display/ops_test.py b/certbot/certbot/_internal/tests/display/ops_test.py index 3363bfe2b..b971f780c 100644 --- a/certbot/certbot/_internal/tests/display/ops_test.py +++ b/certbot/certbot/_internal/tests/display/ops_test.py @@ -35,7 +35,7 @@ class GetEmailTest(unittest.TestCase): with pytest.raises(errors.Error): self._call() with pytest.raises(errors.Error): - self._call(optional=False) + self._call() @test_util.patch_display_util() def test_ok_safe(self, mock_get_utility): @@ -55,7 +55,7 @@ class GetEmailTest(unittest.TestCase): @test_util.patch_display_util() def test_invalid_flag(self, mock_get_utility): - invalid_txt = "There seem to be problems" + invalid_txt = "The server reported a problem" mock_input = mock_get_utility().input mock_input.return_value = (display_util.OK, "foo@bar.baz") with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: @@ -65,19 +65,9 @@ class GetEmailTest(unittest.TestCase): self._call(invalid=True) assert invalid_txt in mock_input.call_args[0][0] - @test_util.patch_display_util() - def test_optional_flag(self, mock_get_utility): - mock_input = mock_get_utility().input - mock_input.return_value = (display_util.OK, "foo@bar.baz") - with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: - mock_safe_email.side_effect = [False, True] - self._call(optional=False) - for call in mock_input.call_args_list: - assert "--register-unsafely-without-email" not in call[0][0] - @test_util.patch_display_util() def test_optional_invalid_unsafe(self, mock_get_utility): - invalid_txt = "There seem to be problems" + invalid_txt = "There is a problem" mock_input = mock_get_utility().input mock_input.return_value = (display_util.OK, "foo@bar.baz") with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: diff --git a/certbot/certbot/_internal/tests/main_test.py b/certbot/certbot/_internal/tests/main_test.py index 4b76e9423..59fb594fb 100644 --- a/certbot/certbot/_internal/tests/main_test.py +++ b/certbot/certbot/_internal/tests/main_test.py @@ -1062,8 +1062,6 @@ class MainTest(test_util.ConfigTestCase): def test_noninteractive(self, _): args = ['-n', 'certonly'] self._cli_missing_flag(args, "specify a plugin") - args.extend(['--standalone', '-d', 'eg.is']) - self._cli_missing_flag(args, "register before running") @mock.patch('certbot._internal.eff.handle_subscription') @mock.patch('certbot._internal.log.post_arg_parse_setup') diff --git a/certbot/certbot/display/ops.py b/certbot/certbot/display/ops.py index d5b2c2420..68694338f 100644 --- a/certbot/certbot/display/ops.py +++ b/certbot/certbot/display/ops.py @@ -19,12 +19,10 @@ from certbot.display import util as display_util logger = logging.getLogger(__name__) -def get_email(invalid: bool = False, optional: bool = True) -> str: +def get_email(invalid: bool = False, **kwargs: Any) -> str: """Prompt for valid email address. :param bool invalid: True if an invalid address was provided by the user - :param bool optional: True if the user can use - --register-unsafely-without-email to avoid providing an e-mail :returns: e-mail address :rtype: str @@ -32,45 +30,22 @@ def get_email(invalid: bool = False, optional: bool = True) -> str: :raises errors.Error: if the user cancels """ - invalid_prefix = "There seem to be problems with that address. " - msg = "Enter email address (used for urgent renewal and security notices)\n" - unsafe_suggestion = ("\n\nIf you really want to skip this, you can run " - "the client with --register-unsafely-without-email " - "but you will then be unable to receive notice about " - "impending expiration or revocation of your " - "certificates or problems with your Certbot " - "installation that will lead to failure to renew.\n\n") - if optional: - if invalid: - msg += unsafe_suggestion - suggest_unsafe = False - else: - suggest_unsafe = True - else: - suggest_unsafe = False + # pylint: disable=unused-argument + invalid_prefix = "" + if invalid: + invalid_prefix = "The server reported a problem with your email address. " + msg = "Enter email address or hit Enter to skip.\n" while True: - try: - code, email = display_util.input_text(invalid_prefix + msg if invalid else msg, - force_interactive=True) - except errors.MissingCommandlineFlag: - msg = ("You should register before running non-interactively, " - "or provide --agree-tos and --email flags.") - raise errors.MissingCommandlineFlag(msg) + code, email = display_util.input_text(invalid_prefix + msg, default="") if code != display_util.OK: - if optional: - raise errors.Error( - "An e-mail address or " - "--register-unsafely-without-email must be provided.") - raise errors.Error("An e-mail address must be provided.") + raise errors.Error("Error getting email address.") + if email == "": + return "" if util.safe_email(email): return email - if suggest_unsafe: - msg = unsafe_suggestion + msg - suggest_unsafe = False # add this message at most once - - invalid = bool(email) + invalid_prefix = "There is a problem with your email address. " def choose_account(accounts: List[account.Account]) -> Optional[account.Account]: