From b1740eee79d412729433d696d09c4a62c5cb4d64 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 6 Dec 2021 14:53:47 -0800 Subject: [PATCH] letsencrypt: On domain removal, don't revoke certificate, keep it Closes: #2156. Tests: - Remove a domain from System -> Config, 'letsencrypt revoke' action is not invoked. - Triggering a manual revoke operation still leads to action getting triggered. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/letsencrypt/__init__.py | 18 ++++++++++++++---- .../tests/test_domain_name_changes.py | 3 ++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/plinth/modules/letsencrypt/__init__.py b/plinth/modules/letsencrypt/__init__.py index c035d9f55..e01141c6e 100644 --- a/plinth/modules/letsencrypt/__init__.py +++ b/plinth/modules/letsencrypt/__init__.py @@ -128,9 +128,19 @@ def certificate_reobtain(domain): actions.superuser_run('letsencrypt', ['obtain', '--domain', domain]) -def certificate_revoke(domain): - """Revoke a certificate for a domain and notify handlers.""" - actions.superuser_run('letsencrypt', ['revoke', '--domain', domain]) +def certificate_revoke(domain, really_revoke=True): + """Revoke a certificate for a domain and notify handlers. + + Revoke a certificate unless really requested to. Otherwise, simply trigger + actions as if the certificate has been revoked. On actions such as domain + removed, behave as if certificate has been revoked but don't actually + revoke the certificate. Domains could be re-added later and certificates + could be reused. Certificates are precious (due to a rate limit for + obtaining certificates on the Let's Encrypt servers). + """ + if really_revoke: + actions.superuser_run('letsencrypt', ['revoke', '--domain', domain]) + components.on_certificate_event('revoked', [domain], None) @@ -170,7 +180,7 @@ def on_domain_removed(sender, domain_type, name='', **kwargs): try: if name: logger.info('Revoking certificate for %s', name) - certificate_revoke(name) + certificate_revoke(name, really_revoke=False) return True except ActionError as exception: logger.warning('Failed to revoke certificate for %s: %s', name, diff --git a/plinth/modules/letsencrypt/tests/test_domain_name_changes.py b/plinth/modules/letsencrypt/tests/test_domain_name_changes.py index ca73deba2..2d9f6bdd6 100644 --- a/plinth/modules/letsencrypt/tests/test_domain_name_changes.py +++ b/plinth/modules/letsencrypt/tests/test_domain_name_changes.py @@ -69,6 +69,7 @@ def test_remove_domain(certificate_revoke, domain, revoke, result): """Test removing a domain that can certificates.""" assert result == on_domain_removed('test', 'domain-type-test', domain) if revoke: - certificate_revoke.assert_has_calls([call(domain)]) + certificate_revoke.assert_has_calls( + [call(domain, really_revoke=False)]) else: certificate_revoke.assert_not_called()