From 18f61631c9aa0d9b935743b5226f7fcaa4186da7 Mon Sep 17 00:00:00 2001
From: Allan Eising <allan@eising.dk>
Date: Sat, 31 May 2025 10:55:33 +0200
Subject: [PATCH] Improve error handling and begin error test suite

---
 .../src/sshecret_admin/services/keepass.py    | 12 ++++++++-
 .../tests/test_password_context.py            |  9 -------
 .../tests/test_password_context_errors.py     | 26 +++++++++++++++++++
 3 files changed, 37 insertions(+), 10 deletions(-)
 create mode 100644 packages/sshecret-admin/tests/test_password_context_errors.py

diff --git a/packages/sshecret-admin/src/sshecret_admin/services/keepass.py b/packages/sshecret-admin/src/sshecret_admin/services/keepass.py
index 0367774..73a674a 100644
--- a/packages/sshecret-admin/src/sshecret_admin/services/keepass.py
+++ b/packages/sshecret-admin/src/sshecret_admin/services/keepass.py
@@ -7,6 +7,7 @@ from pathlib import Path
 from typing import cast
 
 import pykeepass
+import pykeepass.exceptions
 from sshecret_admin.core.settings import AdminServerSettings
 
 from .models import SecretGroup
@@ -20,6 +21,10 @@ NO_USERNAME = "NO_USERNAME"
 DEFAULT_LOCATION = "keepass.kdbx"
 
 
+class PasswordCredentialsError(Exception):
+    pass
+
+
 def create_password_db(location: Path, password: str) -> None:
     """Create the password database."""
     LOG.info("Creating password database at %s", location)
@@ -248,7 +253,12 @@ class PasswordContext:
 @contextmanager
 def _password_context(location: Path, password: str) -> Iterator[PasswordContext]:
     """Open the password context."""
-    database = pykeepass.PyKeePass(str(location.absolute()), password=password)
+    try:
+        database = pykeepass.PyKeePass(str(location.absolute()), password=password)
+    except pykeepass.exceptions.CredentialsError as e:
+        raise PasswordCredentialsError(
+            "Could not open password database. Invalid credentials."
+        ) from e
     context = PasswordContext(database)
     yield context
 
diff --git a/packages/sshecret-admin/tests/test_password_context.py b/packages/sshecret-admin/tests/test_password_context.py
index 7b834bf..6aa1494 100644
--- a/packages/sshecret-admin/tests/test_password_context.py
+++ b/packages/sshecret-admin/tests/test_password_context.py
@@ -7,9 +7,7 @@ those in the low level API.
 
 import random
 import string
-from pathlib import Path
 from typing import cast
-import pytest
 import pykeepass
 
 from sshecret_admin.services.keepass import PasswordContext
@@ -43,13 +41,6 @@ def create_random_entries(
     password_database.save()
 
 
-@pytest.fixture(name="password_database")
-def password_db_fixture(tmp_path: Path):
-    """Create a password database."""
-    filename = tmp_path / "kpdb.kdbx"
-    yield pykeepass.create_database(str(filename), password="test")
-
-
 def test_add_entry(password_database: pykeepass.PyKeePass) -> None:
     """Test add entry."""
     context = PasswordContext(password_database)
diff --git a/packages/sshecret-admin/tests/test_password_context_errors.py b/packages/sshecret-admin/tests/test_password_context_errors.py
new file mode 100644
index 0000000..ebc0af5
--- /dev/null
+++ b/packages/sshecret-admin/tests/test_password_context_errors.py
@@ -0,0 +1,26 @@
+"""Tests various error types."""
+
+from pathlib import Path
+import pykeepass
+import pykeepass.exceptions
+import pytest
+
+from sshecret_admin.services.keepass import PasswordContext, _password_context, PasswordCredentialsError
+
+def test_open_invalid_database() -> None:
+    """Test opening a non-existing database."""
+    bogus_path = Path("/tmp/non/existing/password/database.kdbx")
+
+    with pytest.raises(FileNotFoundError):
+        with _password_context(bogus_path, "foobar") as context:
+            assert context is not None
+
+
+def test_incorrect_password(tmp_path: Path) -> None:
+    """Test opening database with incorrect password."""
+    filename = tmp_path / "db.kdbx"
+    pykeepass.create_database(str(filename), password="correct")
+
+    with pytest.raises(PasswordCredentialsError):
+        with _password_context(filename, "incorrect") as context:
+            assert context is not None