(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
This commit is contained in:
@@ -45,9 +45,21 @@
|
|||||||
{% if credential %}
|
{% if credential %}
|
||||||
<section class="panel">
|
<section class="panel">
|
||||||
<h2>Delete SSH Key</h2>
|
<h2>Delete SSH Key</h2>
|
||||||
|
{% if credential.hosts.exists or credential.global_configs.exists %}
|
||||||
|
<p class="muted">
|
||||||
|
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.
|
||||||
|
</p>
|
||||||
|
{% else %}
|
||||||
|
<p class="muted">Type <strong>{{ credential.name }}</strong> to confirm deletion.</p>
|
||||||
|
{% endif %}
|
||||||
<form method="post" action="{% url 'delete_ssh_credential' credential.id %}">
|
<form method="post" action="{% url 'delete_ssh_credential' credential.id %}">
|
||||||
{% csrf_token %}
|
{% csrf_token %}
|
||||||
<button type="submit" class="danger">Delete SSH key</button>
|
<div class="field">
|
||||||
|
<label for="confirm_name">Confirm key name</label>
|
||||||
|
<input id="confirm_name" name="confirm_name" type="text" {% if credential.hosts.exists or credential.global_configs.exists %}disabled{% endif %}>
|
||||||
|
</div>
|
||||||
|
<button type="submit" class="danger" {% if credential.hosts.exists or credential.global_configs.exists %}disabled{% endif %}>Delete SSH key</button>
|
||||||
</form>
|
</form>
|
||||||
</section>
|
</section>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
<th>Known hosts</th>
|
<th>Known hosts</th>
|
||||||
<th>Hosts</th>
|
<th>Hosts</th>
|
||||||
<th>Updated</th>
|
<th>Updated</th>
|
||||||
|
<th>Actions</th>
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
<tbody>
|
<tbody>
|
||||||
@@ -35,9 +36,10 @@
|
|||||||
<td>{{ credential.known_hosts|yesno:"yes,no" }}</td>
|
<td>{{ credential.known_hosts|yesno:"yes,no" }}</td>
|
||||||
<td>{{ credential.hosts.count }}</td>
|
<td>{{ credential.hosts.count }}</td>
|
||||||
<td>{{ credential.updated_at }}</td>
|
<td>{{ credential.updated_at }}</td>
|
||||||
|
<td><a class="button-link secondary" href="{% url 'edit_ssh_credential' credential.id %}">Edit</a></td>
|
||||||
</tr>
|
</tr>
|
||||||
{% empty %}
|
{% empty %}
|
||||||
<tr><td colspan="7" class="muted">No SSH credentials configured yet.</td></tr>
|
<tr><td colspan="8" class="muted">No SSH credentials configured yet.</td></tr>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
</tbody>
|
</tbody>
|
||||||
</table>
|
</table>
|
||||||
|
|||||||
@@ -406,13 +406,46 @@ class ViewTests(TestCase):
|
|||||||
generate_ssh_key(credential)
|
generate_ssh_key(credential)
|
||||||
key_path = Path(credential.key_path)
|
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.assertRedirects(response, reverse("ssh_credentials"))
|
||||||
self.assertContains(response, "SSH key deleted: generated-key.")
|
self.assertContains(response, "SSH key deleted: generated-key.")
|
||||||
self.assertFalse(SshCredential.objects.exists())
|
self.assertFalse(SshCredential.objects.exists())
|
||||||
self.assertFalse(key_path.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:
|
def test_ssh_credentials_view_rejects_invalid_key(self) -> None:
|
||||||
self.client.force_login(self.staff_user)
|
self.client.force_login(self.staff_user)
|
||||||
|
|
||||||
@@ -476,7 +509,7 @@ class ViewTests(TestCase):
|
|||||||
response = self.client.post(
|
response = self.client.post(
|
||||||
reverse("edit_ssh_credential", args=[credential.id]),
|
reverse("edit_ssh_credential", args=[credential.id]),
|
||||||
{
|
{
|
||||||
"name": "backup-key",
|
"name": "renamed-backup-key",
|
||||||
"private_key": "UPDATED KEY",
|
"private_key": "UPDATED KEY",
|
||||||
"public_key": "",
|
"public_key": "",
|
||||||
"known_hosts": "",
|
"known_hosts": "",
|
||||||
@@ -487,6 +520,7 @@ class ViewTests(TestCase):
|
|||||||
|
|
||||||
self.assertRedirects(response, reverse("ssh_credentials"))
|
self.assertRedirects(response, reverse("ssh_credentials"))
|
||||||
credential.refresh_from_db()
|
credential.refresh_from_db()
|
||||||
|
self.assertEqual(credential.name, "renamed-backup-key")
|
||||||
self.assertEqual(credential.private_key, "UPDATED KEY\n")
|
self.assertEqual(credential.private_key, "UPDATED KEY\n")
|
||||||
self.assertEqual(credential.public_key, "UPDATED PUBLIC KEY")
|
self.assertEqual(credential.public_key, "UPDATED PUBLIC KEY")
|
||||||
self.assertEqual(credential.notes, "rotated")
|
self.assertEqual(credential.notes, "rotated")
|
||||||
|
|||||||
@@ -239,6 +239,9 @@ def delete_ssh_credential(request, credential_id: int):
|
|||||||
if credential.hosts.exists() or credential.global_configs.exists():
|
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.")
|
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)
|
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
|
name = credential.name
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user