commit 9e6f791a6580f2a4b5ceff7fc9f1cc438eae3de1
Author: Jan Pašek <janpasek97@gmail.com>
Date:   Thu Apr 15 18:19:47 2021 +0200

    Re #8571 - Fixed problems during walk-through code review

diff --git a/src/controllers/certificates_controller.py b/src/controllers/certificates_controller.py
index 1306fd9..932c745 100644
--- a/src/controllers/certificates_controller.py
+++ b/src/controllers/certificates_controller.py
@@ -12,7 +12,7 @@ from src.constants import CA_ID, \
 from src.exceptions.database_exception import DatabaseException
 from src.model.subject import Subject
 from src.services.certificate_service import CertificateService, RevocationReasonInvalidException, \
-    CertificateStatusInvalidException, CertificateNotFoundException
+    CertificateStatusInvalidException, CertificateNotFoundException, CertificateAlreadyRevokedException
 #  responsibility.
 from src.services.key_service import KeyService
 
@@ -35,6 +35,7 @@ REASON_UNDEFINED = "unspecified"
 
 E_NO_ISSUER_FOUND = {"success": False, "data": "No certificate authority with such unique ID exists."}
 E_NO_CERTIFICATES_FOUND = {"success": False, "data": "No such certificate found."}
+E_NO_CERTIFICATE_ALREADY_REVOKED = {"success": False, "data": "Certificate is already revoked."}
 E_NO_CERT_PRIVATE_KEY_FOUND = {"success": False,
                                "data": "Internal server error (certificate's private key cannot be found)."}
 E_NOT_JSON_FORMAT = {"success": False, "data": "The request must be JSON-formatted."}
@@ -340,9 +341,13 @@ class CertController:
             try:
                 # set certificate status using certificate_service
                 self.certificate_service.set_certificate_revocation_status(identifier, status, reason)
-            except (RevocationReasonInvalidException, CertificateStatusInvalidException, CertificateNotFoundException):
+            except (RevocationReasonInvalidException, CertificateStatusInvalidException):
                 # these exceptions are thrown in case invalid status or revocation reason is passed to the controller
                 return E_WRONG_PARAMETERS, C_BAD_REQUEST
+            except CertificateAlreadyRevokedException:
+                return E_NO_CERTIFICATE_ALREADY_REVOKED, C_BAD_REQUEST
+            except CertificateNotFoundException:
+                return E_NO_CERTIFICATES_FOUND, C_NOT_FOUND
             return {"success": True,
                     "data": "Certificate status updated successfully."}, C_SUCCESS
         # throw an error in case the request does not contain a json body
diff --git a/src/exceptions/unknown_exception.py b/src/exceptions/unknown_exception.py
new file mode 100644
index 0000000..bb051bc
--- /dev/null
+++ b/src/exceptions/unknown_exception.py
@@ -0,0 +1,6 @@
+class UnknownException(Exception):
+    def __init__(self, message):
+        self.message = message
+
+    def __str__(self):
+        return self.message
diff --git a/src/services/certificate_service.py b/src/services/certificate_service.py
index 4d27786..a609443 100644
--- a/src/services/certificate_service.py
+++ b/src/services/certificate_service.py
@@ -6,6 +6,7 @@ from src.config.configuration import Configuration
 from src.constants import ROOT_CA_ID, INTERMEDIATE_CA_ID, CA_ID, CERTIFICATE_ID, CERTIFICATE_STATES, \
     CERTIFICATE_REVOCATION_REASONS
 from src.dao.certificate_repository import CertificateRepository
+from src.exceptions.unknown_exception import UnknownException
 from src.model.certificate import Certificate
 from src.model.private_key import PrivateKey
 from src.model.subject import Subject
@@ -279,15 +280,27 @@ class CertificateService:
         if reason not in CERTIFICATE_REVOCATION_REASONS:
             raise RevocationReasonInvalidException(reason)
 
+        # check whether the certificate exists
+        certificate = self.certificate_repository.read(id)
+        if certificate is None:
+            raise CertificateNotFoundException(id)
+
         updated = False
         if status == STATUS_VALID:
             updated = self.certificate_repository.clear_certificate_revocation(id)
         elif status == STATUS_REVOKED:
+            # check if the certificate is not revoked already
+            revoked = self.certificate_repository.get_all_revoked_by(certificate.parent_id)
+            if certificate.certificate_id in [x.certificate_id for x in revoked]:
+                raise CertificateAlreadyRevokedException(id)
+
             revocation_timestamp = int(time.time())
             updated = self.certificate_repository.set_certificate_revoked(id, str(revocation_timestamp), reason)
 
         if not updated:
-            raise CertificateNotFoundException(id)
+            # TODO log this
+            raise UnknownException("Repository returned 'false' from clear_certificate_revocation() "
+                                   "or set_certificate_revoked().")
 
     def get_subject_from_certificate(self, certificate: Certificate) -> Subject:
         """
@@ -357,7 +370,7 @@ class CertificateStatusInvalidException(Exception):
 class CertificateNotFoundException(Exception):
     """
     Exception that denotes that the caller was trying to set
-    a certificate to an invalid state
+    work with non-existing certificate
     """
 
     def __init__(self, id):
@@ -365,3 +378,16 @@ class CertificateNotFoundException(Exception):
 
     def __str__(self):
         return f"Certificate id '{self.id}' does not exist."
+
+
+class CertificateAlreadyRevokedException(Exception):
+    """
+    Exception that denotes that the caller was trying to revoke
+    a certificate that is already revoked
+    """
+
+    def __init__(self, id):
+        self.id = id
+
+    def __str__(self):
+        return f"Certificate id '{self.id}' is already revoked."
diff --git a/src/services/cryptography.py b/src/services/cryptography.py
index c7593f4..886577e 100644
--- a/src/services/cryptography.py
+++ b/src/services/cryptography.py
@@ -126,7 +126,7 @@ class CryptographyService:
         # reference in openssl req command using -extensions option.
         extensions += "\n"+CA_EXTENSIONS
         if len(config) == 0:
-            config += MINIMAL_CONFIG_FILE+"[ " + SSCRT_SECTION + " ]"+"\n"+extensions
+            config += MINIMAL_CONFIG_FILE
         config += "\n[ " + SSCRT_SECTION + " ]" + "\n" + extensions
 
         with TemporaryFile("openssl.conf", config) as conf_path:
diff --git a/tests/integration_tests/rest_api/certificates_test.py b/tests/integration_tests/rest_api/certificates_test.py
index 9e6f751..a63dad1 100644
--- a/tests/integration_tests/rest_api/certificates_test.py
+++ b/tests/integration_tests/rest_api/certificates_test.py
@@ -634,6 +634,13 @@ def test_set_certificate_status(server):
     assert "success" in revoke_ret.json
     assert revoke_ret.json["success"]
 
+    revoke_ret = server.patch(f"/api/certificates/{cert_id}", content_type="application/json", json=revocation_body)
+
+    assert revoke_ret.status_code == 400
+    assert "data" in revoke_ret.json
+    assert "success" in revoke_ret.json
+    assert not revoke_ret.json["success"]
+
     # set to valid again
     valid_body = {
         "status":  "valid"
@@ -687,7 +694,7 @@ def test_set_certificate_status(server):
     }
     revoke_ret = server.patch(f"/api/certificates/54791", content_type="application/json", json=revocation_body)
 
-    assert revoke_ret.status_code == 400
+    assert revoke_ret.status_code == 404
     assert "data" in revoke_ret.json
     assert "success" in revoke_ret.json
     assert not revoke_ret.json["success"]
diff --git a/tests/integration_tests/services/certificate_service_test.py b/tests/integration_tests/services/certificate_service_test.py
index f11be08..08e022f 100644
--- a/tests/integration_tests/services/certificate_service_test.py
+++ b/tests/integration_tests/services/certificate_service_test.py
@@ -5,7 +5,7 @@ import pytest
 from src.constants import SSL_ID, CA_ID, AUTHENTICATION_ID, INTERMEDIATE_CA_ID, ROOT_CA_ID, CERTIFICATE_ID, SIGNATURE_ID
 from src.model.subject import Subject
 from src.services.certificate_service import RevocationReasonInvalidException, CertificateStatusInvalidException, \
-    CertificateNotFoundException
+    CertificateNotFoundException, CertificateAlreadyRevokedException
 
 
 def export_crt(crt):
@@ -321,3 +321,10 @@ def test_set_certificate_revoked_errors(certificate_service_unique, private_key_
 
     with pytest.raises(CertificateNotFoundException) as e:
         certificate_service_unique.set_certificate_revocation_status(5974, "revoked", "unspecified")
+
+    certificate_service_unique.set_certificate_revocation_status(root_ca_cert.certificate_id, "revoked", "unspecified")
+
+    with pytest.raises(CertificateAlreadyRevokedException) as e:
+        certificate_service_unique.set_certificate_revocation_status(root_ca_cert.certificate_id, "revoked",
+                                                                     "unspecified")
+
