From 5b5a5bc637b7a1056a47d8c8c87471f181ea45c3 Mon Sep 17 00:00:00 2001 From: Peter van Arkel Date: Thu, 21 May 2026 03:38:55 +0200 Subject: [PATCH] (release) Harden SSH key edit and delete flow Make SSH credential management more explicit by adding an edit action in the key overview and requiring name confirmation before deletion. Keep deletion blocked while a key is still selected by hosts or global config, and cover rename, delete confirmation, and in-use protection in view tests. Refs #20 Refs #8 --- .../pobsync_backend/ssh_credential_form.html | 14 ++++++- .../pobsync_backend/ssh_credentials.html | 4 +- src/pobsync_backend/tests/test_views.py | 38 ++++++++++++++++++- src/pobsync_backend/views.py | 3 ++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/pobsync_backend/templates/pobsync_backend/ssh_credential_form.html b/src/pobsync_backend/templates/pobsync_backend/ssh_credential_form.html index ee061bb..543915d 100644 --- a/src/pobsync_backend/templates/pobsync_backend/ssh_credential_form.html +++ b/src/pobsync_backend/templates/pobsync_backend/ssh_credential_form.html @@ -45,9 +45,21 @@ {% if credential %}

Delete SSH Key

+ {% if credential.hosts.exists or credential.global_configs.exists %} +

+ This SSH key is still selected by {{ credential.hosts.count }} host(s) or + {{ credential.global_configs.count }} global config(s). Select another key there before deleting it. +

+ {% else %} +

Type {{ credential.name }} to confirm deletion.

+ {% endif %}
{% csrf_token %} - +
+ + +
+
{% endif %} diff --git a/src/pobsync_backend/templates/pobsync_backend/ssh_credentials.html b/src/pobsync_backend/templates/pobsync_backend/ssh_credentials.html index 71ec49e..cc031a6 100644 --- a/src/pobsync_backend/templates/pobsync_backend/ssh_credentials.html +++ b/src/pobsync_backend/templates/pobsync_backend/ssh_credentials.html @@ -23,6 +23,7 @@ Known hosts Hosts Updated + Actions @@ -35,9 +36,10 @@ {{ credential.known_hosts|yesno:"yes,no" }} {{ credential.hosts.count }} {{ credential.updated_at }} + Edit {% empty %} - No SSH credentials configured yet. + No SSH credentials configured yet. {% endfor %} diff --git a/src/pobsync_backend/tests/test_views.py b/src/pobsync_backend/tests/test_views.py index 6538c01..131603d 100644 --- a/src/pobsync_backend/tests/test_views.py +++ b/src/pobsync_backend/tests/test_views.py @@ -406,13 +406,46 @@ class ViewTests(TestCase): generate_ssh_key(credential) key_path = Path(credential.key_path) - response = self.client.post(reverse("delete_ssh_credential", args=[credential.id]), follow=True) + response = self.client.post( + reverse("delete_ssh_credential", args=[credential.id]), + {"confirm_name": credential.name}, + follow=True, + ) self.assertRedirects(response, reverse("ssh_credentials")) self.assertContains(response, "SSH key deleted: generated-key.") self.assertFalse(SshCredential.objects.exists()) self.assertFalse(key_path.exists()) + def test_ssh_credentials_view_requires_delete_confirmation(self) -> None: + self.client.force_login(self.staff_user) + credential = SshCredential.objects.create(name="backup-key") + + response = self.client.post( + reverse("delete_ssh_credential", args=[credential.id]), + {"confirm_name": "wrong"}, + follow=True, + ) + + self.assertRedirects(response, reverse("edit_ssh_credential", args=[credential.id])) + self.assertContains(response, "Type backup-key to confirm SSH key deletion.") + self.assertTrue(SshCredential.objects.filter(pk=credential.pk).exists()) + + def test_ssh_credentials_view_blocks_delete_when_key_is_in_use(self) -> None: + self.client.force_login(self.staff_user) + credential = SshCredential.objects.create(name="backup-key") + HostConfig.objects.create(host="web-01", address="web-01.example.test", ssh_credential=credential) + + response = self.client.post( + reverse("delete_ssh_credential", args=[credential.id]), + {"confirm_name": credential.name}, + follow=True, + ) + + self.assertRedirects(response, reverse("edit_ssh_credential", args=[credential.id])) + self.assertContains(response, "SSH key backup-key is still in use and cannot be deleted.") + self.assertTrue(SshCredential.objects.filter(pk=credential.pk).exists()) + def test_ssh_credentials_view_rejects_invalid_key(self) -> None: self.client.force_login(self.staff_user) @@ -476,7 +509,7 @@ class ViewTests(TestCase): response = self.client.post( reverse("edit_ssh_credential", args=[credential.id]), { - "name": "backup-key", + "name": "renamed-backup-key", "private_key": "UPDATED KEY", "public_key": "", "known_hosts": "", @@ -487,6 +520,7 @@ class ViewTests(TestCase): self.assertRedirects(response, reverse("ssh_credentials")) credential.refresh_from_db() + self.assertEqual(credential.name, "renamed-backup-key") self.assertEqual(credential.private_key, "UPDATED KEY\n") self.assertEqual(credential.public_key, "UPDATED PUBLIC KEY") self.assertEqual(credential.notes, "rotated") diff --git a/src/pobsync_backend/views.py b/src/pobsync_backend/views.py index a6d7414..f0ef91c 100644 --- a/src/pobsync_backend/views.py +++ b/src/pobsync_backend/views.py @@ -239,6 +239,9 @@ def delete_ssh_credential(request, credential_id: int): if credential.hosts.exists() or credential.global_configs.exists(): messages.error(request, f"SSH key {credential.name} is still in use and cannot be deleted.") return redirect("edit_ssh_credential", credential_id=credential.id) + if request.POST.get("confirm_name", "").strip() != credential.name: + messages.error(request, f"Type {credential.name} to confirm SSH key deletion.") + return redirect("edit_ssh_credential", credential_id=credential.id) name = credential.name try: