Improve the "certbot certificates" output (#3846)

* Begin making "certbot certificates" future safe

* Handle the case where a renewal conf file has no "server" entry

certbot/cert_manager.py

@@ -39,20 +39,22 @@ def _report_human_readable(parsed_certs):
     certinfo = []
     for cert in parsed_certs:
         now = pytz.UTC.fromutc(datetime.datetime.utcnow())
-        if cert.target_expiry <= now:
-            expiration_text = "EXPIRED"
+        if cert.is_test_cert:
+            expiration_text = "INVALID: TEST CERT"
+        elif cert.target_expiry <= now:
+            expiration_text = "INVALID: EXPIRED"
         else:
             diff = cert.target_expiry - now
             if diff.days == 1:
-                expiration_text = "1 day"
+                expiration_text = "VALID: 1 day"
             elif diff.days < 1:
-                expiration_text = "under 1 day"
+                expiration_text = "VALID: {0} hour(s)".format(diff.seconds // 3600)
             else:
-                expiration_text = "{0} days".format(diff.days)
+                expiration_text = "VALID: {0} days".format(diff.days)
         valid_string = "{0} ({1})".format(cert.target_expiry, expiration_text)
         certinfo.append("  Certificate Name: {0}\n"
                         "    Domains: {1}\n"
-                        "    Valid Until: {2}\n"
+                        "    Expiry Date: {2}\n"
                         "    Certificate Path: {3}\n"
                         "    Private Key Path: {4}".format(
                             cert.lineagename,

certbot/renewal.py

@@ -13,7 +13,6 @@ import OpenSSL
 
 from certbot import configuration
 from certbot import cli
-from certbot import constants
 
 from certbot import crypto_util
 from certbot import errors
@@ -209,9 +208,6 @@ def should_renew(config, lineage):
 
 def _avoid_invalidating_lineage(config, lineage, original_server):
     "Do not renew a valid cert with one from a staging server!"
-    def _is_staging(srv):
-        return srv == constants.STAGING_URI or "staging" in srv
-
     # Some lineages may have begun with --staging, but then had production certs
     # added to them
     latest_cert = OpenSSL.crypto.load_certificate(
@@ -220,8 +216,8 @@ def _avoid_invalidating_lineage(config, lineage, original_server):
     # we should test more methodically
     now_valid = "fake" not in repr(latest_cert.get_issuer()).lower()
 
-    if _is_staging(config.server):
-        if not _is_staging(original_server) or now_valid:
+    if util.is_staging(config.server):
+        if not util.is_staging(original_server) or now_valid:
             if not config.break_my_certs:
                 names = ", ".join(lineage.names())
                 raise errors.Error(

certbot/storage.py

@@ -172,7 +172,8 @@ def relevant_values(all_values):
         if _relevant(option) and cli.option_was_set(option, value))
 
 
-class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
+class RenewableCert(object):
+    # pylint: disable=too-many-instance-attributes,too-many-public-methods
     """Renewable certificate.
 
     Represents a lineage of certificates that is under the management of
@@ -281,6 +282,15 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
             return os.path.join(
                 self.cli_config.default_archive_dir, self.lineagename)
 
+    @property
+    def is_test_cert(self):
+        """Returns true if this is a test cert from a staging server."""
+        server = self.configuration["renewalparams"].get("server", None)
+        if server:
+            return util.is_staging(server)
+        else:
+            return False
+
     def _check_symlinks(self):
         """Raises an exception if a symlink doesn't exist"""
         for kind in ALL_FOUR:

certbot/tests/cert_manager_test.py

@@ -157,26 +157,34 @@ class CertificatesTest(BaseCertManagerTest):
         cert = mock.MagicMock(lineagename="nameone")
         cert.target_expiry = expiry
         cert.names.return_value = ["nameone", "nametwo"]
+        cert.is_test_cert = False
         parsed_certs = [cert]
         # pylint: disable=protected-access
         out = cert_manager._report_human_readable(parsed_certs)
-        self.assertTrue('EXPIRED' in out)
+        self.assertTrue("INVALID: EXPIRED" in out)
 
         cert.target_expiry += datetime.timedelta(hours=2)
         # pylint: disable=protected-access
         out = cert_manager._report_human_readable(parsed_certs)
-        self.assertTrue('under 1 day' in out)
+        self.assertTrue('1 hour(s)' in out)
+        self.assertTrue('VALID' in out and not 'INVALID' in out)
 
         cert.target_expiry += datetime.timedelta(days=1)
         # pylint: disable=protected-access
         out = cert_manager._report_human_readable(parsed_certs)
         self.assertTrue('1 day' in out)
         self.assertFalse('under' in out)
+        self.assertTrue('VALID' in out and not 'INVALID' in out)
 
         cert.target_expiry += datetime.timedelta(days=2)
         # pylint: disable=protected-access
         out = cert_manager._report_human_readable(parsed_certs)
         self.assertTrue('3 days' in out)
+        self.assertTrue('VALID' in out and not 'INVALID' in out)
+
+        cert.is_test_cert = True
+        out = cert_manager._report_human_readable(parsed_certs)
+        self.assertTrue('INVALID: TEST CERT' in out)
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/tests/storage_test.py

@@ -705,6 +705,19 @@ class RenewableCertTests(BaseRenewableCertTest):
             self.assertEqual(storage.add_time_interval(base_time, interval),
                              excepted)
 
+    def test_is_test_cert(self):
+        self.test_rc.configuration["renewalparams"] = {}
+        rp = self.test_rc.configuration["renewalparams"]
+        self.assertEqual(self.test_rc.is_test_cert, False)
+        rp["server"] = "https://acme-staging.api.letsencrypt.org/directory"
+        self.assertEqual(self.test_rc.is_test_cert, True)
+        rp["server"] = "https://staging.someotherca.com/directory"
+        self.assertEqual(self.test_rc.is_test_cert, True)
+        rp["server"] = "https://acme-v01.api.letsencrypt.org/directory"
+        self.assertEqual(self.test_rc.is_test_cert, False)
+        rp["server"] = "https://acme-v02.api.letsencrypt.org/directory"
+        self.assertEqual(self.test_rc.is_test_cert, False)
+
     def test_missing_cert(self):
         from certbot import storage
         self.assertRaises(errors.CertStorageError,

certbot/util.py

@@ -17,6 +17,7 @@ import sys
 
 import configargparse
 
+from certbot import constants
 from certbot import errors
 
 
@@ -499,3 +500,14 @@ def get_strict_version(normalized):
     # strict version ending with "a" and a number designates a pre-release
     # pylint: disable=no-member
     return distutils.version.StrictVersion(normalized.replace(".dev", "a"))
+
+
+def is_staging(srv):
+    """
+    Determine whether a given ACME server is a known test / staging server.
+
+    :param str srv: the URI for the ACME server
+    :returns: True iff srv is a known test / staging server
+    :rtype bool:
+    """
+    return srv == constants.STAGING_URI or "staging" in srv