From 30ec52683b96b481a097a128b54fdb74e88096ac Mon Sep 17 00:00:00 2001 From: Johanna England Date: Sat, 27 Sep 2025 09:52:26 +0200 Subject: [PATCH 1/2] Catch general exceptions for manual detention And improve the read-write profile exception message --- changelog.d/3383.fixed.md | 1 + python/nav/arnold.py | 12 +++++++++--- python/nav/web/arnold/views.py | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 changelog.d/3383.fixed.md diff --git a/changelog.d/3383.fixed.md b/changelog.d/3383.fixed.md new file mode 100644 index 0000000000..77770e2d22 --- /dev/null +++ b/changelog.d/3383.fixed.md @@ -0,0 +1 @@ +Catch and show no read-write profile error in arnold diff --git a/python/nav/arnold.py b/python/nav/arnold.py index b621c88d67..03b93bf5ed 100644 --- a/python/nav/arnold.py +++ b/python/nav/arnold.py @@ -295,7 +295,9 @@ def disable(candidate, justification, username, comment="", autoenablestep=0): if not candidate.interface.netbox.get_preferred_snmp_management_profile( require_write=True ): - raise NoReadWriteManagementProfileError(candidate.interface.netbox) + raise NoReadWriteManagementProfileError( + "%s has no read-write management profile" % candidate.interface.netbox + ) identity = check_identity(candidate) change_port_status('disable', identity) identity.status = 'disabled' @@ -317,7 +319,9 @@ def quarantine(candidate, qvlan, justification, username, comment="", autoenable if not candidate.interface.netbox.get_preferred_snmp_management_profile( require_write=True ): - raise NoReadWriteManagementProfileError(candidate.interface.netbox) + raise NoReadWriteManagementProfileError( + "%s has no read-write management profile" % candidate.interface.netbox + ) identity = check_identity(candidate) identity.fromvlan = change_port_vlan(identity, qvlan.vlan) identity.tovlan = qvlan @@ -461,7 +465,9 @@ def change_port_status( profile = netbox.get_preferred_snmp_management_profile(require_write=True) if not profile: - raise NoReadWriteManagementProfileError + raise NoReadWriteManagementProfileError( + "%s has no read-write management profile" % netbox + ) agent = agent_getter(profile)(host=netbox.ip) diff --git a/python/nav/web/arnold/views.py b/python/nav/web/arnold/views.py index 72f3d8951d..3e0a5d041a 100644 --- a/python/nav/web/arnold/views.py +++ b/python/nav/web/arnold/views.py @@ -322,7 +322,7 @@ def process_manual_detention_form(form, account): disable( identity, justification, username, comment=comment, autoenablestep=days ) - except GeneralException as error: + except Exception as error: # noqa return error elif form.cleaned_data['method'] == 'quarantine': qvlan = QuarantineVlan.objects.get(pk=form.cleaned_data['qvlan']) @@ -335,7 +335,7 @@ def process_manual_detention_form(form, account): comment=comment, autoenablestep=days, ) - except GeneralException as error: + except Exception as error: # noqa return error From eee531fb60234cb91999bf3cd223867b0cac4cd6 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Sat, 27 Sep 2025 11:36:03 +0200 Subject: [PATCH 2/2] Add tests --- tests/integration/web/arnold_test.py | 152 ++++++++++++++++++++++----- 1 file changed, 125 insertions(+), 27 deletions(-) diff --git a/tests/integration/web/arnold_test.py b/tests/integration/web/arnold_test.py index 4f76e5c859..e16af36ea2 100644 --- a/tests/integration/web/arnold_test.py +++ b/tests/integration/web/arnold_test.py @@ -1,40 +1,138 @@ +import pytest from django.urls import reverse from django.utils.encoding import smart_str -from nav.models.arnold import QuarantineVlan +from nav.models.arnold import Justification, QuarantineVlan +from nav.models.fields import INFINITY +from nav.models.manage import Cam, Device, Interface, Module, Netbox -def test_arnold_manualdetention_should_not_crash(client): - url = reverse('arnold-manual-detention') +class TestManualDetention: + def test_search_for_valid_ip_should_not_crash(self, client): + url = reverse('arnold-manual-detention') - response = client.post( - url, - follow=True, - data={ - 'submit': 'Find', - 'target': '10.0.0.1', - }, # any address will do - ) + response = client.post( + url, + follow=True, + data={ + 'submit': 'Find', + 'target': '10.0.0.1', + }, # any address will do + ) + + assert response.status_code == 200 + + def test_given_netbox_without_read_write_profile_blocking_should_show_error( + self, client, cam_entry + ): + qvlan = QuarantineVlan.objects.create(vlan=1, description='') + justification = Justification.objects.create(name='justification') + + url = reverse("arnold-manual-detention-step-two", args=(cam_entry.mac,)) + + response = client.post( + url, + follow=True, + data={ + "camtuple": str(cam_entry.pk), + "target": cam_entry.mac, + "method": "disable", + "qvlan": str(qvlan.pk), + "justification": str(justification.pk), + "submit": "Detain", + }, + ) + + assert response.status_code == 200 + assert ( + str(response.context.get("error")) + == "example-sw.example.org has no read-write management profile" + ) + + def test_given_netbox_without_read_write_profile_quarantining_should_show_error( + self, client, cam_entry + ): + qvlan = QuarantineVlan.objects.create(vlan=1, description='') + justification = Justification.objects.create(name='justification') + + url = reverse("arnold-manual-detention-step-two", args=(cam_entry.mac,)) + + response = client.post( + url, + follow=True, + data={ + "camtuple": str(cam_entry.pk), + "target": cam_entry.mac, + "method": "quarantine", + "qvlan": str(qvlan.pk), + "justification": str(justification.pk), + "submit": "Detain", + }, + ) - assert response.status_code == 200 + assert response.status_code == 200 + assert ( + str(response.context.get("error")) + == "example-sw.example.org has no read-write management profile" + ) -def test_arnold_quarantine_vlan_twice_should_show_error_message(client): - url = reverse('arnold-quarantinevlans') +class TestQuarantineVlan: + def test_quarantine_vlan_twice_should_show_error_message(self, client): + url = reverse('arnold-quarantinevlans') - vlan_id = 1 - QuarantineVlan.objects.create(vlan=vlan_id, description='') + vlan_id = 1 + QuarantineVlan.objects.create(vlan=vlan_id, description='') - response = client.post( - url, - follow=True, - data={ - 'vlan': vlan_id, - 'description': '', - 'qid': '', - 'submit': 'Add+vlan', - }, + response = client.post( + url, + follow=True, + data={ + 'vlan': vlan_id, + 'description': '', + 'qid': '', + 'submit': 'Add+vlan', + }, + ) + + assert response.status_code == 200 + assert "This vlan is already quarantined." in smart_str(response.content) + + +@pytest.fixture() +def cam_entry(db): + box = Netbox( + ip='10.254.254.254', + sysname='example-sw.example.org', + organization_id='myorg', + room_id='myroom', + category_id='SW', ) + box.save() + + device = Device(serial="1234test") + device.save() + module = Module(device=device, netbox=box, name='Module 1', model='') + module.save() + + interface = Interface( + netbox=box, + module=module, + ifindex=1, + ) + interface.save() + + cam = Cam.objects.create( + netbox_id=box.id, + mac='aa:aa:aa:aa:aa:aa', + ifindex=interface.ifindex, + end_time=INFINITY, + ) + + yield cam - assert response.status_code == 200 - assert "This vlan is already quarantined." in smart_str(response.content) + cam.delete() + interface.delete() + module.delete() + device.delete() + box.delete()