From 921d52b3608db7d2c9ce3dab0c64a12bb6726e14 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Fri, 13 Mar 2020 00:03:31 -0700 Subject: [PATCH 1/9] fixing get_dns_challenge() logic so duplicate domains (such as wildcard and not wildcard) do not match the wrong authorziations --- lemur/plugins/lemur_acme/plugin.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 91781966..e566352c 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -54,18 +54,30 @@ class AcmeHandler(object): current_app.logger.error(f"Unable to fetch DNS Providers: {e}") self.all_dns_providers = [] - def find_dns_challenge(self, host, authorizations): + def get_dns_challenges(self, host, authorizations): + """Get final domain to validate and dns challenges for it""" + + domain_to_validate, is_wildcard = self.strip_wildcard(host) dns_challenges = [] for authz in authorizations: - if not authz.body.identifier.value.lower() == host.lower(): + if not authz.body.identifier.value.lower() == domain_to_validate.lower(): + continue + if is_wildcard and not authz.body.wildcard: + continue + if not is_wildcard and authz.body.wildcard: continue for combo in authz.body.challenges: if isinstance(combo.chall, challenges.DNS01): dns_challenges.append(combo) - return dns_challenges - def maybe_remove_wildcard(self, host): - return host.replace("*.", "") + return domain_to_validate, dns_challenges + + def strip_wildcard(self, host): + """Removes the leading *. and returns Host and whether it was removed or not (True/False)""" + prefix = "*." + if host.startswith(prefix): + return host[len(prefix):], True + return host, False def maybe_add_extension(self, host, dns_provider_options): if dns_provider_options and dns_provider_options.get( @@ -86,9 +98,7 @@ class AcmeHandler(object): current_app.logger.debug("Starting DNS challenge for {0}".format(host)) change_ids = [] - - host_to_validate = self.maybe_remove_wildcard(host) - dns_challenges = self.find_dns_challenge(host_to_validate, order.authorizations) + host_to_validate, dns_challenges = self.get_dns_challenges(host, order.authorizations) host_to_validate = self.maybe_add_extension( host_to_validate, dns_provider_options ) @@ -325,7 +335,7 @@ class AcmeHandler(object): ) dns_provider_options = json.loads(dns_provider.credentials) account_number = dns_provider_options.get("account_id") - host_to_validate = self.maybe_remove_wildcard(authz_record.host) + host_to_validate, _ = self.strip_wildcard(authz_record.host) host_to_validate = self.maybe_add_extension( host_to_validate, dns_provider_options ) @@ -357,7 +367,7 @@ class AcmeHandler(object): dns_provider_options = json.loads(dns_provider.credentials) account_number = dns_provider_options.get("account_id") dns_challenges = authz_record.dns_challenge - host_to_validate = self.maybe_remove_wildcard(authz_record.host) + host_to_validate, _ = self.strip_wildcard(authz_record.host) host_to_validate = self.maybe_add_extension( host_to_validate, dns_provider_options ) From 593c35776c868cfe937c79a119bc52a03dec446d Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Sat, 14 Mar 2020 20:17:05 -0700 Subject: [PATCH 2/9] adding new methods for getting pending clean --- lemur/certificates/service.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 0e91b563..ff558284 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -117,6 +117,41 @@ def get_all_pending_cleaning(source): ) +def get_all_pending_cleaning_about_to_expire_certs(source, days_to_expire): + """ + Retrieves all certificates that are available for cleaning: not attached to endpoint, + and within X days from expiration. + + :param days_to_expire: + :param source: + :return: + """ + expiration_window = arrow.now().shift(days=+days_to_expire).format("YYYY-MM-DD") + return ( + Certificate.query.filter(Certificate.sources.any(id=source.id)) + .filter(not_(Certificate.endpoints.any())) + .filter(Certificate.not_after < expiration_window) + .all() + ) + + +def get_all_pending_cleaning_not_in_use_certs(source, days_since_issuance): + """ + Retrieves all certificates that are available for cleaning: not attached to endpoint, and X days since issuance. + + :param days_since_issuance: + :param source: + :return: + """ + not_in_use_window = arrow.now().shift(days=-days_since_issuance).format("YYYY-MM-DD") + return ( + Certificate.query.filter(Certificate.sources.any(id=source.id)) + .filter(not_(Certificate.endpoints.any())) + .filter(Certificate.date_created < not_in_use_window) + .all() + ) + + def get_all_pending_reissue(): """ Retrieves all certificates that need to be rotated. From c96695c966229039e78e4a5f799bd525a0d122af Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Sat, 14 Mar 2020 20:18:07 -0700 Subject: [PATCH 3/9] refactor --- lemur/sources/cli.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lemur/sources/cli.py b/lemur/sources/cli.py index c41a1cf7..c28600c2 100644 --- a/lemur/sources/cli.py +++ b/lemur/sources/cli.py @@ -54,6 +54,17 @@ def validate_sources(source_strings): return sources +def execute_clean(plugin, certificate, source): + try: + plugin.clean(certificate, source.options) + certificate.sources.remove(source) + certificate_service.database.update(certificate) + return SUCCESS_METRIC_STATUS + except Exception as e: + current_app.logger.exception(e) + sentry.captureException() + + @manager.option( "-s", "--sources", @@ -147,14 +158,7 @@ def clean(source_strings, commit): for certificate in certificate_service.get_all_pending_cleaning(source): status = FAILURE_METRIC_STATUS if commit: - try: - s.clean(certificate, source.options) - certificate.sources.remove(source) - certificate_service.database.update(certificate) - status = SUCCESS_METRIC_STATUS - except Exception as e: - current_app.logger.exception(e) - sentry.captureException() + status = execute_clean(s, certificate, source) metrics.send( "clean", From b28b4f9a28feba72c1d705d2085b30e57c75574d Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Sat, 14 Mar 2020 20:19:26 -0700 Subject: [PATCH 4/9] adding to new cli commands for cleaning certificates from source: a) either about to expire in X days and not attached to an endpoint a) or issued since X days but still not attached to an endpoint --- lemur/sources/cli.py | 155 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 136 insertions(+), 19 deletions(-) diff --git a/lemur/sources/cli.py b/lemur/sources/cli.py index c28600c2..a5b670a0 100644 --- a/lemur/sources/cli.py +++ b/lemur/sources/cli.py @@ -143,11 +143,9 @@ def clean(source_strings, commit): s = plugins.get(source.plugin_name) if not hasattr(s, "clean"): - print( - "Cannot clean source: {0}, source plugin does not implement 'clean()'".format( - source.label - ) - ) + info_text = f"Cannot clean source: {source.label}, source plugin does not implement 'clean()'" + current_app.logger.warning(info_text) + print(info_text) continue start_time = time.time() @@ -155,28 +153,147 @@ def clean(source_strings, commit): print("[+] Staring to clean source: {label}!\n".format(label=source.label)) cleaned = 0 - for certificate in certificate_service.get_all_pending_cleaning(source): + certificates = certificate_service.get_all_pending_cleaning(source) + for certificate in certificates: status = FAILURE_METRIC_STATUS if commit: status = execute_clean(s, certificate, source) metrics.send( - "clean", + "certificate_clean", "counter", 1, - metric_tags={"source": source.label, "status": status}, + metric_tags={"status": status, "source": source.label, "certificate": certificate.name}, ) - - current_app.logger.warning( - "Removed {0} from source {1} during cleaning".format( - certificate.name, source.label - ) - ) - + current_app.logger.warning(f"Removed {certificate.name} from source {source.label} during cleaning") cleaned += 1 - print( - "[+] Finished cleaning source: {label}. Removed {cleaned} certificates from source. Run Time: {time}\n".format( - label=source.label, time=(time.time() - start_time), cleaned=cleaned + info_text = f"[+] Finished cleaning source: {source.label}. " \ + f"Removed {cleaned} certificates from source. " \ + f"Run Time: {(time.time() - start_time)}\n" + print(info_text) + current_app.logger.warning(info_text) + + +@manager.option( + "-s", + "--sources", + dest="source_strings", + action="append", + help="Sources to operate on.", +) +@manager.option( + "-d", + "--days", + dest="days_to_expire", + type=int, + action="store", + required=True, + help="The expiry range within days.", +) +@manager.option( + "-c", + "--commit", + dest="commit", + action="store_true", + default=False, + help="Persist changes.", +) +def clean_unused_and_expiring_within_days(source_strings, days_to_expire, commit): + sources = validate_sources(source_strings) + for source in sources: + s = plugins.get(source.plugin_name) + + if not hasattr(s, "clean"): + info_text = f"Cannot clean source: {source.label}, source plugin does not implement 'clean()'" + current_app.logger.warning(info_text) + print(info_text) + continue + + start_time = time.time() + + print("[+] Staring to clean source: {label}!\n".format(label=source.label)) + + cleaned = 0 + certificates = certificate_service.get_all_pending_cleaning_about_to_expire_certs(source, days_to_expire) + for certificate in certificates: + status = FAILURE_METRIC_STATUS + if commit: + status = execute_clean(s, certificate, source) + + metrics.send( + "certificate_clean", + "counter", + 1, + metric_tags={"status": status, "source": source.label, "certificate": certificate.name}, ) - ) + current_app.logger.warning(f"Removed {certificate.name} from source {source.label} during cleaning") + cleaned += 1 + + info_text = f"[+] Finished cleaning source: {source.label}. " \ + f"Removed {cleaned} certificates from source. " \ + f"Run Time: {(time.time() - start_time)}\n" + print(info_text) + current_app.logger.warning(info_text) + + +@manager.option( + "-s", + "--sources", + dest="source_strings", + action="append", + help="Sources to operate on.", +) +@manager.option( + "-d", + "--days", + dest="days_since_issuance", + type=int, + action="store", + required=True, + help="Days since issuance.", +) +@manager.option( + "-c", + "--commit", + dest="commit", + action="store_true", + default=False, + help="Persist changes.", +) +def clean_unused_and_issued_since_days(source_strings, days_since_issuance, commit): + sources = validate_sources(source_strings) + for source in sources: + s = plugins.get(source.plugin_name) + + if not hasattr(s, "clean"): + info_text = f"Cannot clean source: {source.label}, source plugin does not implement 'clean()'" + current_app.logger.warning(info_text) + print(info_text) + continue + + start_time = time.time() + + print("[+] Staring to clean source: {label}!\n".format(label=source.label)) + + cleaned = 0 + certificates = certificate_service.get_all_pending_cleaning_not_in_use_certs(source, days_since_issuance) + for certificate in certificates: + status = FAILURE_METRIC_STATUS + if commit: + status = execute_clean(s, certificate, source) + + metrics.send( + "certificate_clean", + "counter", + 1, + metric_tags={"status": status, "source": source.label, "certificate": certificate.name}, + ) + current_app.logger.warning(f"Removed {certificate.name} from source {source.label} during cleaning") + cleaned += 1 + + info_text = f"[+] Finished cleaning source: {source.label}. " \ + f"Removed {cleaned} certificates from source. " \ + f"Run Time: {(time.time() - start_time)}\n" + print(info_text) + current_app.logger.warning(info_text) From 34d23503def14520e951d2b976d9db6ad6e4b5e8 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Sat, 14 Mar 2020 20:41:03 -0700 Subject: [PATCH 5/9] fixing the data bug --- lemur/certificates/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index ff558284..0889f64c 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -147,7 +147,7 @@ def get_all_pending_cleaning_not_in_use_certs(source, days_since_issuance): return ( Certificate.query.filter(Certificate.sources.any(id=source.id)) .filter(not_(Certificate.endpoints.any())) - .filter(Certificate.date_created < not_in_use_window) + .filter(Certificate.date_created > not_in_use_window) .all() ) From 1a19e250bb6561e52e3ab20bbb8dd9b82b36009b Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Mon, 16 Mar 2020 11:24:17 -0700 Subject: [PATCH 6/9] updating and cleaning up tests --- lemur/plugins/lemur_acme/plugin.py | 7 +- lemur/plugins/lemur_acme/tests/test_acme.py | 105 +++++++++++--------- 2 files changed, 63 insertions(+), 49 deletions(-) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index e566352c..3fc1df61 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/lemur/plugins/lemur_acme/plugin.py @@ -55,7 +55,7 @@ class AcmeHandler(object): self.all_dns_providers = [] def get_dns_challenges(self, host, authorizations): - """Get final domain to validate and dns challenges for it""" + """Get dns challenges for provided domain""" domain_to_validate, is_wildcard = self.strip_wildcard(host) dns_challenges = [] @@ -70,7 +70,7 @@ class AcmeHandler(object): if isinstance(combo.chall, challenges.DNS01): dns_challenges.append(combo) - return domain_to_validate, dns_challenges + return dns_challenges def strip_wildcard(self, host): """Removes the leading *. and returns Host and whether it was removed or not (True/False)""" @@ -98,7 +98,8 @@ class AcmeHandler(object): current_app.logger.debug("Starting DNS challenge for {0}".format(host)) change_ids = [] - host_to_validate, dns_challenges = self.get_dns_challenges(host, order.authorizations) + dns_challenges = self.get_dns_challenges(host, order.authorizations) + host_to_validate, _ = self.strip_wildcard(host) host_to_validate = self.maybe_add_extension( host_to_validate, dns_provider_options ) diff --git a/lemur/plugins/lemur_acme/tests/test_acme.py b/lemur/plugins/lemur_acme/tests/test_acme.py index 990a556e..eefd6c5b 100644 --- a/lemur/plugins/lemur_acme/tests/test_acme.py +++ b/lemur/plugins/lemur_acme/tests/test_acme.py @@ -23,11 +23,12 @@ class TestAcme(unittest.TestCase): } @patch("lemur.plugins.lemur_acme.plugin.len", return_value=1) - def test_find_dns_challenge(self, mock_len): + def test_get_dns_challenges(self, mock_len): assert mock_len from acme import challenges + host = "example.com" c = challenges.DNS01() mock_authz = Mock() @@ -35,9 +36,18 @@ class TestAcme(unittest.TestCase): mock_entry = Mock() mock_entry.chall = c mock_authz.body.resolved_combinations.append(mock_entry) - result = yield self.acme.find_dns_challenge(mock_authz) + result = yield self.acme.get_dns_challenges(host, mock_authz) self.assertEqual(result, mock_entry) + def test_strip_wildcard(self): + expected = ("example.com", False) + result = self.acme.strip_wildcard("example.com") + self.assertEqual(expected, result) + + expected = ("example.com", True) + result = self.acme.strip_wildcard("*.example.com") + self.assertEqual(expected, result) + def test_authz_record(self): a = plugin.AuthorizationRecord("host", "authz", "challenge", "id") self.assertEqual(type(a), plugin.AuthorizationRecord) @@ -45,9 +55,9 @@ class TestAcme(unittest.TestCase): @patch("acme.client.Client") @patch("lemur.plugins.lemur_acme.plugin.current_app") @patch("lemur.plugins.lemur_acme.plugin.len", return_value=1) - @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.find_dns_challenge") + @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.get_dns_challenges") def test_start_dns_challenge( - self, mock_find_dns_challenge, mock_len, mock_app, mock_acme + self, mock_get_dns_challenges, mock_len, mock_app, mock_acme ): assert mock_len mock_order = Mock() @@ -65,9 +75,12 @@ class TestAcme(unittest.TestCase): mock_dns_provider.create_txt_record = Mock(return_value=1) values = [mock_entry] - iterable = mock_find_dns_challenge.return_value + iterable = mock_get_dns_challenges.return_value iterator = iter(values) iterable.__iter__.return_value = iterator + + # mock_get_dns_challenges = Mock(return_value="") + result = self.acme.start_dns_challenge( mock_acme, "accountid", "host", mock_dns_provider, mock_order, {} ) @@ -102,7 +115,7 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.current_app") @patch("lemur.plugins.lemur_acme.cloudflare.wait_for_dns_change") def test_complete_dns_challenge_fail( - self, mock_wait_for_dns_change, mock_current_app, mock_acme + self, mock_wait_for_dns_change, mock_current_app, mock_acme ): mock_dns_provider = Mock() mock_dns_provider.wait_for_dns_change = Mock(return_value=True) @@ -127,15 +140,15 @@ class TestAcme(unittest.TestCase): @patch("acme.client.Client") @patch("OpenSSL.crypto", return_value="mock_cert") @patch("josepy.util.ComparableX509") - @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.find_dns_challenge") + @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.get_dns_challenges") @patch("lemur.plugins.lemur_acme.plugin.current_app") def test_request_certificate( - self, - mock_current_app, - mock_find_dns_challenge, - mock_jose, - mock_crypto, - mock_acme, + self, + mock_current_app, + mock_get_dns_challenges, + mock_jose, + mock_crypto, + mock_acme, ): mock_cert_response = Mock() mock_cert_response.body = "123" @@ -256,11 +269,11 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.cloudflare.current_app") @patch("lemur.plugins.lemur_acme.plugin.dns_provider_service") def test_get_dns_provider( - self, - mock_dns_provider_service, - mock_current_app_cloudflare, - mock_current_app_dyn, - mock_current_app, + self, + mock_dns_provider_service, + mock_current_app_cloudflare, + mock_current_app_dyn, + mock_current_app, ): provider = plugin.ACMEIssuerPlugin() route53 = provider.get_dns_provider("route53") @@ -278,14 +291,14 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.finalize_authorizations") @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.request_certificate") def test_get_ordered_certificate( - self, - mock_request_certificate, - mock_finalize_authorizations, - mock_get_authorizations, - mock_dns_provider_service, - mock_authorization_service, - mock_current_app, - mock_acme, + self, + mock_request_certificate, + mock_finalize_authorizations, + mock_get_authorizations, + mock_dns_provider_service, + mock_authorization_service, + mock_current_app, + mock_acme, ): mock_client = Mock() mock_acme.return_value = (mock_client, "") @@ -309,14 +322,14 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.finalize_authorizations") @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.request_certificate") def test_get_ordered_certificates( - self, - mock_request_certificate, - mock_finalize_authorizations, - mock_get_authorizations, - mock_dns_provider_service, - mock_authorization_service, - mock_current_app, - mock_acme, + self, + mock_request_certificate, + mock_finalize_authorizations, + mock_get_authorizations, + mock_dns_provider_service, + mock_authorization_service, + mock_current_app, + mock_acme, ): mock_client = Mock() mock_acme.return_value = (mock_client, "") @@ -349,14 +362,14 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.request_certificate") @patch("lemur.plugins.lemur_acme.plugin.authorization_service") def test_create_certificate( - self, - mock_authorization_service, - mock_request_certificate, - mock_finalize_authorizations, - mock_get_authorizations, - mock_current_app, - mock_dns_provider_service, - mock_acme, + self, + mock_authorization_service, + mock_request_certificate, + mock_finalize_authorizations, + mock_get_authorizations, + mock_current_app, + mock_dns_provider_service, + mock_acme, ): provider = plugin.ACMEIssuerPlugin() mock_authority = Mock() @@ -423,10 +436,10 @@ class TestAcme(unittest.TestCase): ultradns._post = Mock() ultradns._get = Mock() ultradns._get.return_value = {'zoneName': 'test.example.com.com', - 'rrSets': [{'ownerName': '_acme-challenge.test.example.com.', - 'rrtype': 'TXT (16)', 'ttl': 5, 'rdata': ['ABCDEFGHIJ']}], - 'queryInfo': {'sort': 'OWNER', 'reverse': False, 'limit': 100}, - 'resultInfo': {'totalCount': 1, 'offset': 0, 'returnedCount': 1}} + 'rrSets': [{'ownerName': '_acme-challenge.test.example.com.', + 'rrtype': 'TXT (16)', 'ttl': 5, 'rdata': ['ABCDEFGHIJ']}], + 'queryInfo': {'sort': 'OWNER', 'reverse': False, 'limit': 100}, + 'resultInfo': {'totalCount': 1, 'offset': 0, 'returnedCount': 1}} ultradns._delete = Mock() mock_metrics.send = Mock() ultradns.delete_txt_record(change_id, account_number, domain, token) From 07dc31bed7c92650f7ab5e5e3247bf36bebb2c52 Mon Sep 17 00:00:00 2001 From: csine-nflx Date: Mon, 16 Mar 2020 11:41:05 -0700 Subject: [PATCH 7/9] cleaning up whitespace changes --- lemur/plugins/lemur_acme/tests/test_acme.py | 49 ++++++++++----------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/lemur/plugins/lemur_acme/tests/test_acme.py b/lemur/plugins/lemur_acme/tests/test_acme.py index eefd6c5b..b2c32eec 100644 --- a/lemur/plugins/lemur_acme/tests/test_acme.py +++ b/lemur/plugins/lemur_acme/tests/test_acme.py @@ -78,9 +78,6 @@ class TestAcme(unittest.TestCase): iterable = mock_get_dns_challenges.return_value iterator = iter(values) iterable.__iter__.return_value = iterator - - # mock_get_dns_challenges = Mock(return_value="") - result = self.acme.start_dns_challenge( mock_acme, "accountid", "host", mock_dns_provider, mock_order, {} ) @@ -115,7 +112,7 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.current_app") @patch("lemur.plugins.lemur_acme.cloudflare.wait_for_dns_change") def test_complete_dns_challenge_fail( - self, mock_wait_for_dns_change, mock_current_app, mock_acme + self, mock_wait_for_dns_change, mock_current_app, mock_acme ): mock_dns_provider = Mock() mock_dns_provider.wait_for_dns_change = Mock(return_value=True) @@ -143,12 +140,12 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.get_dns_challenges") @patch("lemur.plugins.lemur_acme.plugin.current_app") def test_request_certificate( - self, - mock_current_app, - mock_get_dns_challenges, - mock_jose, - mock_crypto, - mock_acme, + self, + mock_current_app, + mock_get_dns_challenges, + mock_jose, + mock_crypto, + mock_acme, ): mock_cert_response = Mock() mock_cert_response.body = "123" @@ -291,14 +288,14 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.finalize_authorizations") @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.request_certificate") def test_get_ordered_certificate( - self, - mock_request_certificate, - mock_finalize_authorizations, - mock_get_authorizations, - mock_dns_provider_service, - mock_authorization_service, - mock_current_app, - mock_acme, + self, + mock_request_certificate, + mock_finalize_authorizations, + mock_get_authorizations, + mock_dns_provider_service, + mock_authorization_service, + mock_current_app, + mock_acme, ): mock_client = Mock() mock_acme.return_value = (mock_client, "") @@ -322,14 +319,14 @@ class TestAcme(unittest.TestCase): @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.finalize_authorizations") @patch("lemur.plugins.lemur_acme.plugin.AcmeHandler.request_certificate") def test_get_ordered_certificates( - self, - mock_request_certificate, - mock_finalize_authorizations, - mock_get_authorizations, - mock_dns_provider_service, - mock_authorization_service, - mock_current_app, - mock_acme, + self, + mock_request_certificate, + mock_finalize_authorizations, + mock_get_authorizations, + mock_dns_provider_service, + mock_authorization_service, + mock_current_app, + mock_acme, ): mock_client = Mock() mock_acme.return_value = (mock_client, "") From ecca003ab4a2d8710caa55b620a92e1a39f62650 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Tue, 17 Mar 2020 16:55:36 -0700 Subject: [PATCH 8/9] improving the documentation and method naming --- lemur/certificates/service.py | 27 ++++++++++++++------------- lemur/sources/cli.py | 6 +++--- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 0889f64c..1fa4d64e 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -102,12 +102,13 @@ def get_all_certs(): return Certificate.query.all() -def get_all_pending_cleaning(source): +def get_all_pending_cleaning_expired(source): """ - Retrieves all certificates that are available for cleaning. + Retrieves all certificates that are available for cleaning. These are certificates which are expired and are not + attached to any endpoints. - :param source: - :return: + :param source: the source to search for certificates + :return: the pending certificates """ return ( Certificate.query.filter(Certificate.sources.any(id=source.id)) @@ -117,14 +118,14 @@ def get_all_pending_cleaning(source): ) -def get_all_pending_cleaning_about_to_expire_certs(source, days_to_expire): +def get_all_pending_cleaning_expiring_in_days(source, days_to_expire): """ - Retrieves all certificates that are available for cleaning: not attached to endpoint, + Retrieves all certificates that are available for cleaning, not attached to endpoint, and within X days from expiration. - :param days_to_expire: - :param source: - :return: + :param days_to_expire: defines how many days till the certificate is expired + :param source: the source to search for certificates + :return: the pending certificates """ expiration_window = arrow.now().shift(days=+days_to_expire).format("YYYY-MM-DD") return ( @@ -135,13 +136,13 @@ def get_all_pending_cleaning_about_to_expire_certs(source, days_to_expire): ) -def get_all_pending_cleaning_not_in_use_certs(source, days_since_issuance): +def get_all_pending_cleaning_issued_since_days(source, days_since_issuance): """ Retrieves all certificates that are available for cleaning: not attached to endpoint, and X days since issuance. - :param days_since_issuance: - :param source: - :return: + :param days_since_issuance: defines how many days since the certificate is issued + :param source: the source to search for certificates + :return: the pending certificates """ not_in_use_window = arrow.now().shift(days=-days_since_issuance).format("YYYY-MM-DD") return ( diff --git a/lemur/sources/cli.py b/lemur/sources/cli.py index a5b670a0..0d537500 100644 --- a/lemur/sources/cli.py +++ b/lemur/sources/cli.py @@ -153,7 +153,7 @@ def clean(source_strings, commit): print("[+] Staring to clean source: {label}!\n".format(label=source.label)) cleaned = 0 - certificates = certificate_service.get_all_pending_cleaning(source) + certificates = certificate_service.get_all_pending_cleaning_expired(source) for certificate in certificates: status = FAILURE_METRIC_STATUS if commit: @@ -215,7 +215,7 @@ def clean_unused_and_expiring_within_days(source_strings, days_to_expire, commit print("[+] Staring to clean source: {label}!\n".format(label=source.label)) cleaned = 0 - certificates = certificate_service.get_all_pending_cleaning_about_to_expire_certs(source, days_to_expire) + certificates = certificate_service.get_all_pending_cleaning_expiring_in_days(source, days_to_expire) for certificate in certificates: status = FAILURE_METRIC_STATUS if commit: @@ -277,7 +277,7 @@ def clean_unused_and_issued_since_days(source_strings, days_since_issuance, comm print("[+] Staring to clean source: {label}!\n".format(label=source.label)) cleaned = 0 - certificates = certificate_service.get_all_pending_cleaning_not_in_use_certs(source, days_since_issuance) + certificates = certificate_service.get_all_pending_cleaning_issued_since_days(source, days_since_issuance) for certificate in certificates: status = FAILURE_METRIC_STATUS if commit: From 1d4da0e3d808b1b747daab2196364498c8f61f2a Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Tue, 17 Mar 2020 16:59:09 -0700 Subject: [PATCH 9/9] another polish --- lemur/certificates/service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 1fa4d64e..a6bbba30 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -108,7 +108,7 @@ def get_all_pending_cleaning_expired(source): attached to any endpoints. :param source: the source to search for certificates - :return: the pending certificates + :return: list of pending certificates """ return ( Certificate.query.filter(Certificate.sources.any(id=source.id)) @@ -125,7 +125,7 @@ def get_all_pending_cleaning_expiring_in_days(source, days_to_expire): :param days_to_expire: defines how many days till the certificate is expired :param source: the source to search for certificates - :return: the pending certificates + :return: list of pending certificates """ expiration_window = arrow.now().shift(days=+days_to_expire).format("YYYY-MM-DD") return ( @@ -142,7 +142,7 @@ def get_all_pending_cleaning_issued_since_days(source, days_since_issuance): :param days_since_issuance: defines how many days since the certificate is issued :param source: the source to search for certificates - :return: the pending certificates + :return: list of pending certificates """ not_in_use_window = arrow.now().shift(days=-days_since_issuance).format("YYYY-MM-DD") return (