cli: vary renewal advice for hookless manual certs (#8914)

* cli: vary renewal advice for hookless manual certs

1. Don't print that the certificate will be automatically renewed,
because it won't be.
2. Add a "NEXT STEP" telling the user that they will need to manually
re-issue the certificate in order to renew it.

* kill superfluous comma

Co-authored-by: ohemorange <ebportnoy@gmail.com>

* clarify wording of the next step

* fix the test

Co-authored-by: ohemorange <ebportnoy@gmail.com>

certbot/certbot/_internal/main.py

@@ -507,6 +507,13 @@ def _report_next_steps(config: interfaces.IConfig, installer_err: Optional[error
                 "Certificates created using --csr will not be renewed automatically by Certbot. "
                 "You will need to renew the certificate before it expires, by running the same "
                 "Certbot command again.")
+        elif _is_interactive_only_auth(config):
+            steps.append(
+                "This certificate will not be renewed automatically. Autorenewal of "
+                "--manual certificates requires the use of an authentication hook script "
+                "(--manual-auth-hook) but one was not provided. To renew this certificate, repeat "
+                f"this same {cli.cli_command} command before the certificate's expiry date."
+            )
         elif not config.preconfigured_renewal:
             steps.append(
                 "The certificate will need to be renewed before it expires. Certbot can "
@@ -556,6 +563,11 @@ def _report_new_cert(config, cert_path, fullchain_path, key_path=None):
 
     assert cert_path and fullchain_path, "No certificates saved to report."
 
+    renewal_msg = ""
+    if config.preconfigured_renewal and not _is_interactive_only_auth(config):
+        renewal_msg = ("\nCertbot has set up a scheduled task to automatically renew this "
+                       "certificate in the background.")
+
     display_util.notify(
         ("\nSuccessfully received certificate.\n"
         "Certificate is saved at: {cert_path}\n{key_msg}"
@@ -564,13 +576,22 @@ def _report_new_cert(config, cert_path, fullchain_path, key_path=None):
             cert_path=fullchain_path,
             expiry=crypto_util.notAfter(cert_path).date(),
             key_msg="Key is saved at:         {}\n".format(key_path) if key_path else "",
-            renewal_msg="\nCertbot has set up a scheduled task to automatically renew this "
-                        "certificate in the background." if config.preconfigured_renewal else "",
+            renewal_msg=renewal_msg,
             nl="\n" if config.verb == "run" else "" # Normalize spacing across verbs
         )
     )
 
 
+def _is_interactive_only_auth(config: interfaces.IConfig) -> bool:
+    """ Whether the current authenticator params only support interactive renewal.
+    """
+    # --manual without --manual-auth-hook can never autorenew
+    if config.authenticator == "manual" and config.manual_auth_hook is None:
+        return True
+
+    return False
+
+
 def _csr_report_new_cert(config: interfaces.IConfig, cert_path: Optional[str],
                          chain_path: Optional[str], fullchain_path: Optional[str]):
     """ --csr variant of _report_new_cert.

certbot/tests/main_test.py

@@ -1901,6 +1901,71 @@ class ReportNewCertTest(unittest.TestCase):
             'This certificate expires on 1970-01-01.'
         )
 
+    def test_manual_no_hooks_report(self):
+        """Shouldn't get a message about autorenewal if no --manual-auth-hook"""
+        self._call(mock.Mock(dry_run=False, authenticator='manual', manual_auth_hook=None),
+                  '/path/to/cert.pem', '/path/to/fullchain.pem',
+                  '/path/to/privkey.pem')
+
+        self.mock_notify.assert_called_with(
+            '\nSuccessfully received certificate.\n'
+            'Certificate is saved at: /path/to/fullchain.pem\n'
+            'Key is saved at:         /path/to/privkey.pem\n'
+            'This certificate expires on 1970-01-01.\n'
+            'These files will be updated when the certificate renews.'
+        )
+
+
+class ReportNextStepsTest(unittest.TestCase):
+    """Tests for certbot._internal.main._report_next_steps"""
+
+    def setUp(self):
+        self.config = mock.MagicMock(
+            cert_name="example.com", preconfigured_renewal=True,
+            csr=None, authenticator="nginx", manual_auth_hook=None)
+        notify_patch = mock.patch('certbot._internal.main.display_util.notify')
+        self.mock_notify = notify_patch.start()
+        self.addCleanup(notify_patch.stop)
+        self.old_stdout = sys.stdout
+        sys.stdout = io.StringIO()
+
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
+    @classmethod
+    def _call(cls, *args, **kwargs):
+        from certbot._internal.main import _report_next_steps
+        _report_next_steps(*args, **kwargs)
+
+    def _output(self) -> str:
+        self.mock_notify.assert_called_once()
+        return self.mock_notify.call_args_list[0][0][0]
+
+    def test_report(self):
+        """No steps for a normal renewal"""
+        self.config.authenticator = "manual"
+        self.config.manual_auth_hook = "/bin/true"
+        self._call(self.config, None, None)
+        self.mock_notify.assert_not_called()
+
+    def test_csr_report(self):
+        """--csr requires manual renewal"""
+        self.config.csr = "foo.csr"
+        self._call(self.config, None, None)
+        self.assertIn("--csr will not be renewed", self._output())
+
+    def test_manual_no_hook_renewal(self):
+        """--manual without a hook requires manual renewal"""
+        self.config.authenticator = "manual"
+        self._call(self.config, None, None)
+        self.assertIn("--manual certificates requires", self._output())
+
+    def test_no_preconfigured_renewal(self):
+        """No --preconfigured-renewal needs manual cron setup"""
+        self.config.preconfigured_renewal = False
+        self._call(self.config, None, None)
+        self.assertIn("https://certbot.org/renewal-setup", self._output())
+
 
 class UpdateAccountTest(test_util.ConfigTestCase):
     """Tests for certbot._internal.main.update_account"""