From 8abf95063cf53c0087b5c54c2e5a2e952bac4b5f Mon Sep 17 00:00:00 2001 From: Ronald Moesbergen Date: Thu, 14 Feb 2019 11:57:27 +0100 Subject: [PATCH 1/3] Implement a ALLOW_CERT_DELETION option (boolean, default False). When enabled, the certificate delete API call will work and the UI will no longer display deleted certificates. When disabled (the default), the delete API call will not work (405 method not allowed) and the UI will show all certificates, regardless of the 'deleted' flag. --- docs/administration.rst | 7 +++++++ lemur/certificates/models.py | 2 +- lemur/certificates/service.py | 3 +++ lemur/certificates/views.py | 16 +++++++-------- lemur/migrations/versions/318b66568358_.py | 23 ++++++++++++++++++++++ lemur/tests/conf.py | 2 ++ lemur/tests/test_certificates.py | 4 ++-- 7 files changed, 46 insertions(+), 11 deletions(-) create mode 100644 lemur/migrations/versions/318b66568358_.py diff --git a/docs/administration.rst b/docs/administration.rst index 9d6c8d12..352318f5 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -161,6 +161,13 @@ Specifying the `SQLALCHEMY_MAX_OVERFLOW` to 0 will enforce limit to not create c Dump all imported or generated CSR and certificate details to stdout using OpenSSL. (default: `False`) +.. data:: ALLOW_CERT_DELETION + :noindex: + + When set to True, certificates can be marked as deleted via the API and deleted certificates will not be displayed + in the UI. When set to False (the default), the certificate delete API will always return "405 method not allowed" + and deleted certificates will always be visible in the UI. (default: `False`) + Certificate Default Options --------------------------- diff --git a/lemur/certificates/models.py b/lemur/certificates/models.py index 34305cc2..ab43cd01 100644 --- a/lemur/certificates/models.py +++ b/lemur/certificates/models.py @@ -101,7 +101,7 @@ class Certificate(db.Model): issuer = Column(String(128)) serial = Column(String(128)) cn = Column(String(128)) - deleted = Column(Boolean, index=True) + deleted = Column(Boolean, index=True, default=False) dns_provider_id = Column(Integer(), ForeignKey('dns_providers.id', ondelete='CASCADE'), nullable=True) not_before = Column(ArrowType) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index d5012012..22009043 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -381,6 +381,9 @@ def render(args): now = arrow.now().format('YYYY-MM-DD') query = query.filter(Certificate.not_after <= to).filter(Certificate.not_after >= now) + if current_app.config.get('ALLOW_CERT_DELETION', False): + query = query.filter(Certificate.deleted == False) # noqa + result = database.sort_and_page(query, Certificate, args) return result diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index 948c44d6..37ebf518 100644 --- a/lemur/certificates/views.py +++ b/lemur/certificates/views.py @@ -6,10 +6,9 @@ .. moduleauthor:: Kevin Glisson """ import base64 -import arrow from builtins import str -from flask import Blueprint, make_response, jsonify, g +from flask import Blueprint, make_response, jsonify, g, current_app from flask_restful import reqparse, Api, inputs from lemur.common.schema import validate_schema @@ -678,17 +677,21 @@ class Certificates(AuthenticatedResource): .. sourcecode:: http - HTTP/1.1 200 OK + HTTP/1.1 204 OK :reqheader Authorization: OAuth token to authenticate :statuscode 204: no error :statuscode 403: unauthenticated :statusoode 404: certificate not found + :statusoode 405: certificate deletion is disabled """ + if not current_app.config.get('ALLOW_CERT_DELETION', False): + return dict(message="Certificate deletion is disabled"), 405 + cert = service.get(certificate_id) - if not cert: + if not cert or cert.deleted: return dict(message="Cannot find specified certificate"), 404 # allow creators @@ -699,12 +702,9 @@ class Certificates(AuthenticatedResource): if not permission.can(): return dict(message='You are not authorized to delete this certificate'), 403 - if arrow.get(cert.not_after) > arrow.utcnow(): - return dict(message='Certificate is still valid, only expired certificates can be deleted'), 412 - service.update(certificate_id, deleted=True) log_service.create(g.current_user, 'delete_cert', certificate=cert) - return '', 204 + return 'Certificate deleted', 204 class NotificationCertificatesList(AuthenticatedResource): diff --git a/lemur/migrations/versions/318b66568358_.py b/lemur/migrations/versions/318b66568358_.py new file mode 100644 index 00000000..9d4aa48d --- /dev/null +++ b/lemur/migrations/versions/318b66568358_.py @@ -0,0 +1,23 @@ +""" Set 'deleted' flag from null to false on all certificates once + +Revision ID: 318b66568358 +Revises: 9f79024fe67b +Create Date: 2019-02-05 15:42:25.477587 + +""" + +# revision identifiers, used by Alembic. +revision = '318b66568358' +down_revision = '9f79024fe67b' + +from alembic import op + + +def upgrade(): + connection = op.get_bind() + # Delete duplicate entries + connection.execute('UPDATE certificates SET deleted = false WHERE deleted IS NULL') + + +def downgrade(): + pass diff --git a/lemur/tests/conf.py b/lemur/tests/conf.py index bbe155cd..525200cf 100644 --- a/lemur/tests/conf.py +++ b/lemur/tests/conf.py @@ -186,3 +186,5 @@ LDAP_BASE_DN = 'dc=example,dc=com' LDAP_EMAIL_DOMAIN = 'example.com' LDAP_REQUIRED_GROUP = 'Lemur Access' LDAP_DEFAULT_ROLE = 'role1' + +ALLOW_CERT_DELETION = True diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index 8247c36b..75a29e16 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -737,8 +737,8 @@ def test_certificate_put_with_data(client, certificate, issuer_plugin): @pytest.mark.parametrize("token,status", [ (VALID_USER_HEADER_TOKEN, 403), - (VALID_ADMIN_HEADER_TOKEN, 412), - (VALID_ADMIN_API_TOKEN, 412), + (VALID_ADMIN_HEADER_TOKEN, 204), + (VALID_ADMIN_API_TOKEN, 404), ('', 401) ]) def test_certificate_delete(client, token, status): From 29bda6c00d2351cc4a08ed797c9940d8615a9c73 Mon Sep 17 00:00:00 2001 From: Ronald Moesbergen Date: Thu, 14 Feb 2019 11:58:29 +0100 Subject: [PATCH 2/3] Fix typo's --- lemur/certificates/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index 37ebf518..b464b3ed 100644 --- a/lemur/certificates/views.py +++ b/lemur/certificates/views.py @@ -682,8 +682,8 @@ class Certificates(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 204: no error :statuscode 403: unauthenticated - :statusoode 404: certificate not found - :statusoode 405: certificate deletion is disabled + :statuscode 404: certificate not found + :statuscode 405: certificate deletion is disabled """ if not current_app.config.get('ALLOW_CERT_DELETION', False): From 63de8047ce51f2da0ff181035afb330097017355 Mon Sep 17 00:00:00 2001 From: Ronald Moesbergen Date: Wed, 27 Feb 2019 09:38:25 +0100 Subject: [PATCH 3/3] Return 'already deleted' instead of 'not found' when cert has already been deleted --- lemur/certificates/views.py | 5 ++++- lemur/tests/test_certificates.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index b464b3ed..e77160b2 100644 --- a/lemur/certificates/views.py +++ b/lemur/certificates/views.py @@ -691,9 +691,12 @@ class Certificates(AuthenticatedResource): cert = service.get(certificate_id) - if not cert or cert.deleted: + if not cert: return dict(message="Cannot find specified certificate"), 404 + if cert.deleted: + return dict(message="Certificate is already deleted"), 412 + # allow creators if g.current_user != cert.user: owner_role = role_service.get_by_name(cert.owner) diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index 75a29e16..a020ac6b 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -738,7 +738,7 @@ def test_certificate_put_with_data(client, certificate, issuer_plugin): @pytest.mark.parametrize("token,status", [ (VALID_USER_HEADER_TOKEN, 403), (VALID_ADMIN_HEADER_TOKEN, 204), - (VALID_ADMIN_API_TOKEN, 404), + (VALID_ADMIN_API_TOKEN, 412), ('', 401) ]) def test_certificate_delete(client, token, status):