From d727a4826cc4c359f5ea074b0223b06748f47504 Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Tue, 12 Dec 2023 11:49:21 +0100 Subject: [PATCH 1/2] Allow admins to change VLANs on all ports It doesn't matter that a port is set to an invalid native VLAN, if the user has admin-level access, they should still be allowed to edit the port and fix the problem. --- python/nav/web/portadmin/utils.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/python/nav/web/portadmin/utils.py b/python/nav/web/portadmin/utils.py index 221b06e170..072054b6bd 100644 --- a/python/nav/web/portadmin/utils.py +++ b/python/nav/web/portadmin/utils.py @@ -15,7 +15,7 @@ # """Util functions for the PortAdmin""" from __future__ import unicode_literals -from typing import List, Sequence, Dict, Any +from typing import List, Sequence, Dict, Any, Optional import re import logging from operator import attrgetter @@ -24,6 +24,7 @@ from nav.models import manage, profiles from nav.django.utils import is_admin +from nav.models.profiles import Account from nav.portadmin.config import CONFIG from nav.portadmin.management import ManagementFactory from nav.portadmin.handlers import ManagementHandler @@ -76,7 +77,7 @@ def find_and_populate_allowed_vlans( ): """Finds allowed vlans and indicate which interfaces can be edited""" allowed_vlans = find_allowed_vlans_for_user_on_netbox(account, netbox, handler) - set_editable_flag_on_interfaces(interfaces, allowed_vlans) + set_editable_flag_on_interfaces(interfaces, allowed_vlans, account) return allowed_vlans @@ -126,7 +127,9 @@ def find_allowed_vlans_for_user(account): def set_editable_flag_on_interfaces( - interfaces: Sequence[manage.Interface], vlans: Sequence[FantasyVlan] + interfaces: Sequence[manage.Interface], + vlans: Sequence[FantasyVlan], + user: Optional[Account] = None, ): """Sets the pseudo-attribute `iseditable` on each interface in the interfaces list, indicating whether the PortAdmin UI should allow edits to it or not. @@ -136,8 +139,13 @@ def set_editable_flag_on_interfaces( an uplink, depending on how portadmin is configured. """ vlan_tags = {vlan.vlan for vlan in vlans} + allow_everything = not should_check_access_rights(account=user) if user else False for interface in interfaces: + if allow_everything: + interface.iseditable = True + continue + vlan_is_acceptable = interface.vlan in vlan_tags is_link = bool(interface.to_netbox) refuse_link_edit = not CONFIG.get_link_edit() and is_link From 2e73fd76dae2a66734b77144e0e1f4714acc9e9c Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Wed, 13 Dec 2023 13:51:53 +0100 Subject: [PATCH 2/2] Test set_editable_flag_on_interfaces() --- tests/unittests/web/portadmin/__init__.py | 0 tests/unittests/web/portadmin/utils_test.py | 39 +++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 tests/unittests/web/portadmin/__init__.py create mode 100644 tests/unittests/web/portadmin/utils_test.py diff --git a/tests/unittests/web/portadmin/__init__.py b/tests/unittests/web/portadmin/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unittests/web/portadmin/utils_test.py b/tests/unittests/web/portadmin/utils_test.py new file mode 100644 index 0000000000..84ccff0824 --- /dev/null +++ b/tests/unittests/web/portadmin/utils_test.py @@ -0,0 +1,39 @@ +from unittest.mock import patch, Mock + +from nav.web.portadmin.utils import set_editable_flag_on_interfaces + + +class TestSetEditableFlagOnInterfaces: + def test_when_user_is_admin_it_should_set_all_interfaces_to_editable(self): + with patch( + "nav.web.portadmin.utils.should_check_access_rights", return_value=False + ): + mock_admin = Mock() + mock_interfaces = [Mock(iseditable=False)] * 3 + set_editable_flag_on_interfaces(mock_interfaces, [], mock_admin) + + assert all(ifc.iseditable for ifc in mock_interfaces) + + def test_when_user_is_not_admin_it_should_set_only_matching_interfaces_to_editable( + self, + ): + with patch( + "nav.web.portadmin.utils.should_check_access_rights", return_value=True + ): + mock_user = Mock() + mock_vlans = [Mock(vlan=42), Mock(vlan=69), Mock(vlan=666)] + editable_interface = Mock(vlan=666, iseditable=False) + mock_interfaces = [ + Mock(vlan=99, iseditable=False), + editable_interface, + Mock(vlan=27, iseditable=False), + ] + + set_editable_flag_on_interfaces(mock_interfaces, mock_vlans, mock_user) + + assert editable_interface.iseditable + assert all( + not ifc.iseditable + for ifc in mock_interfaces + if ifc is not editable_interface + )