Handle existing Boefje name

boefjes/boefjes/dependencies/plugins.py

@@ -3,6 +3,7 @@
 from pathlib import Path
 from typing import Literal
 
+import psycopg2
 import structlog
 from fastapi import Query
 from jsonschema.exceptions import ValidationError
@@ -115,7 +116,10 @@ def create_boefje(self, boefje: Boefje) -> None:
                 if plugin.type == "boefje":
                     raise ExistingPluginName(boefje.name)
                 else:
-                    self.plugin_storage.create_boefje(boefje)
+                    try:
+                        self.plugin_storage.create_boefje(boefje)
+                    except psycopg2.errors.UniqueViolation:
+                        raise ExistingPluginName(boefje.name)
             except KeyError:
                 self.plugin_storage.create_boefje(boefje)
 

boefjes/boefjes/katalogus/plugins.py

@@ -17,7 +17,7 @@
 from boefjes.katalogus.organisations import check_organisation_exists
 from boefjes.models import FilterParameters, PaginationParameters, PluginType
 from boefjes.sql.plugin_storage import get_plugin_storage
-from boefjes.storage.interfaces import PluginStorage
+from boefjes.storage.interfaces import ExistingPluginId, ExistingPluginName, NotAllowed, PluginStorage
 
 router = APIRouter(
     prefix="/organisations/{organisation_id}",
@@ -96,14 +96,19 @@ def get_plugin(
 
 @router.post("/plugins", status_code=status.HTTP_201_CREATED)
 def add_plugin(plugin: PluginType, plugin_service: PluginService = Depends(get_plugin_service)):
-    with plugin_service as service:
-        plugin.static = False  # Creation through the API implies that these cannot be static
+    try:
+        with plugin_service as service:
+            plugin.static = False  # Creation through the API implies that these cannot be static
 
-        if plugin.type == "boefje":
-            return service.create_boefje(plugin)
+            if plugin.type == "boefje":
+                return service.create_boefje(plugin)
 
-        if plugin.type == "normalizer":
-            return service.create_normalizer(plugin)
+            if plugin.type == "normalizer":
+                return service.create_normalizer(plugin)
+    except ExistingPluginId:
+        raise HTTPException(status.HTTP_400_BAD_REQUEST, "Duplicate plugin id")
+    except ExistingPluginName:
+        raise HTTPException(status.HTTP_400_BAD_REQUEST, "Duplicate plugin name")
 
     raise HTTPException(status.HTTP_400_BAD_REQUEST, "Creation of Bits is not supported")
 
@@ -165,7 +170,10 @@ def update_boefje(
     storage: PluginStorage = Depends(get_plugin_storage),
 ):
     with storage as p:
-        p.update_boefje(boefje_id, boefje.model_dump(exclude_unset=True))
+        try:
+            p.update_boefje(boefje_id, boefje.model_dump(exclude_unset=True))
+        except NotAllowed:
+            raise HTTPException(status.HTTP_403_FORBIDDEN, "Updating a static plugin is not allowed")
 
 
 @router.delete("/boefjes/{boefje_id}", status_code=status.HTTP_204_NO_CONTENT)

boefjes/boefjes/sql/db.py

@@ -43,7 +43,7 @@ def session_managed_iterator(service_factory: Callable[[Session], Any]) -> Itera
     try:
         yield service
     except Exception as error:
-        logger.exception("An error occurred: %s. Rolling back session", error)
+        logger.error("An error occurred: %s. Rolling back session", error)
         session.rollback()
         raise error
     finally:

boefjes/boefjes/sql/session.py

@@ -30,7 +30,7 @@ def __enter__(self) -> Self:
 
     def __exit__(self, exc_type: type[Exception], exc_value: str, exc_traceback: str) -> None:  # noqa: F841
         if exc_type is not None:
-            logger.error("An error occurred: %s. Rolling back session", exc_value, exc_info=True)
+            logger.error("An error occurred: %s. Rolling back session", exc_value)
             self.session.rollback()
 
             return
@@ -45,5 +45,6 @@ def __exit__(self, exc_type: type[Exception], exc_value: str, exc_traceback: str
         except exc.DatabaseError as e:
             raise StorageError("A storage error occurred") from e
         finally:
-            logger.exception("Committing failed, rolling back")
-            self.session.rollback()
+            if exc_type is not None or self.session.is_active:
+                logger.debug("Committing failed, rolling back")
+                self.session.rollback()

boefjes/tests/integration/test_api.py

@@ -45,12 +45,12 @@ def test_cannot_add_plugin_reserved_id(test_client, organisation):
     boefje = Boefje(id="dns-records", name="My test boefje", static=False)
     response = test_client.post(f"/v1/organisations/{organisation.id}/plugins", content=boefje.model_dump_json())
     assert response.status_code == 400
-    assert response.json() == {"message": "Plugin id 'dns-records' is already used"}
+    assert response.json() == {"detail": "Duplicate plugin id"}
 
     normalizer = Normalizer(id="kat_nmap_normalize", name="My test normalizer")
     response = test_client.post(f"/v1/organisations/{organisation.id}/plugins", content=normalizer.model_dump_json())
     assert response.status_code == 400
-    assert response.json() == {"message": "Plugin id 'kat_nmap_normalize' is already used"}
+    assert response.json() == {"detail": "Duplicate plugin id"}
 
 
 def test_add_boefje(test_client, organisation):
@@ -80,7 +80,7 @@ def test_cannot_add_static_plugin_with_duplicate_name(test_client, organisation)
     boefje = Boefje(id="test_plugin", name="DNS records", static=False)
     response = test_client.post(f"/v1/organisations/{organisation.id}/plugins", content=boefje.model_dump_json())
     assert response.status_code == 400
-    assert response.json() == {"message": "Plugin name 'DNS records' is already used"}
+    assert response.json() == {"detail": "Duplicate plugin name"}
 
 
 def test_cannot_add_plugin_with_duplicate_name(test_client, organisation):
@@ -238,7 +238,7 @@ def test_cannot_update_static_plugins(test_client, organisation):
     assert response.json()["enabled"] is True
 
     response = test_client.patch(f"/v1/organisations/{organisation.id}/boefjes/dns-records", json={"version": "v1.2"})
-    assert response.status_code == 400
+    assert response.status_code == 403
 
     response = test_client.get(f"/v1/organisations/{organisation.id}/plugins/dns-records")
     assert response.json()["version"] != "v1.2"

rocky/katalogus/client.py

@@ -73,6 +73,22 @@ def __str__(self):
         return str(self.message)
 
 
+class DuplicateNameError(KATalogusError):
+    def __init__(self, *args: object) -> None:
+        super().__init__(*args)
+        status_message = ""
+        self.message = status_message + _("Boefje with this name already exists.")
+
+
+class KATalogusNotAllowedError(KATalogusError):
+    def __init__(self, *args: object, status_code: str | None = None, status_message: str | None = None) -> None:
+        super().__init__(*args)
+        status_message = ""
+        if status_code is not None:
+            status_message = f"{status_code}: "
+        self.message = status_message + _("Editing this boefje is not allowed because it is static.")
+
+
 class KATalogusHTTPStatusError(KATalogusError):
     def __init__(self, *args: object, status_code: str | None = None) -> None:
         super().__init__(*args)
@@ -209,29 +225,41 @@ def get_cover(self, boefje_id: str) -> BytesIO:
         return BytesIO(response.content)
 
     def create_plugin(self, plugin: Plugin) -> None:
-        response = self.session.post(
-            f"{self.organization_uri}/plugins",
-            headers={"Content-Type": "application/json"},
-            content=plugin.model_dump_json(exclude_none=True),
-        )
-        response.raise_for_status()
-
-        if response.status_code == codes.CREATED:
-            logger.info("Plugin %s", plugin.name)
-        else:
-            logger.info("Plugin %s could not be created", plugin.name)
+        try:
+            response = self.session.post(
+                f"{self.organization_uri}/plugins",
+                headers={"Content-Type": "application/json"},
+                content=plugin.model_dump_json(exclude_none=True),
+            )
+            response.raise_for_status()
+        except httpx.HTTPStatusError as error:
+            if response.status_code == codes.CREATED:
+                logger.info("Plugin %s created", plugin.name)
+            else:
+                logger.info("Plugin %s could not be created", plugin.name)
+            if response.status_code == codes.BAD_REQUEST and "Duplicate plugin name" in error.response.text:
+                raise DuplicateNameError
+            else:
+                raise error
 
     def edit_plugin(self, plugin: Plugin) -> None:
-        response = self.session.patch(
-            f"{self.organization_uri}/boefjes/{plugin.id}",
-            content=plugin.model_dump_json(exclude_none=True),
-        )
-        response.raise_for_status()
-
-        if response.status_code == codes.CREATED:
-            logger.info("Plugin %s", plugin.name)
-        else:
-            logger.info("Plugin %s could not be created", plugin.name)
+        try:
+            response = self.session.patch(
+                f"{self.organization_uri}/boefjes/{plugin.id}",
+                content=plugin.model_dump_json(exclude_none=True),
+            )
+            response.raise_for_status()
+        except httpx.HTTPStatusError as error:
+            if response.status_code == codes.CREATED:
+                logger.info("Plugin %s updated", plugin.name)
+            else:
+                logger.info("Plugin %s could not be updated", plugin.name)
+            if response.status_code == codes.BAD_REQUEST:
+                raise DuplicateNameError
+            if response.status_code in [codes.FORBIDDEN, codes.NOT_FOUND]:
+                raise KATalogusNotAllowedError
+            else:
+                raise error
 
 
 def parse_boefje(boefje: dict) -> Boefje: