From 7c575e6d572723009b9c9f9f26dc4a8da3ed9ecf Mon Sep 17 00:00:00 2001 From: Terje Kvernes Date: Fri, 17 Mar 2023 21:56:59 +0100 Subject: [PATCH] feat: Increase coverage. Fixes #483 First pass: - Add pragma to exempt lines from coverage to .coveragerc - Add test for stringifying HostPolicyComponent-based objects (see https://github.com/unioslo/mreg/issues/482) - Mock host_permissions/user_in_required_groups as its not supported by default. - Ensure that IsSuperOrAdminOrReadOnly is tested for unauthenticated users (test_permissions/test_get_at_different_privilege_levels) - Exclude exceptions from coverage that are impossible to reach without creating a broken API. - Test that IsSuperOrNetworkAdminMember returns 401 for unauthenticated users (test_networks/test_networks_get_401_unauthorized). --- .coveragerc | 4 ++- hostpolicy/tests.py | 10 +++++++ mreg/api/permissions.py | 22 +++++++++------ mreg/api/v1/tests/test_host_permissions.py | 33 +++++++++++++++++++++- mreg/api/v1/tests/test_networks.py | 6 +++- mreg/api/v1/tests/test_permissions.py | 11 ++++++++ 6 files changed, 75 insertions(+), 11 deletions(-) diff --git a/.coveragerc b/.coveragerc index 07a355b1..9f2953d8 100644 --- a/.coveragerc +++ b/.coveragerc @@ -11,7 +11,9 @@ omit = [report] -exclude_lines = def __repr__ +exclude_lines = + def __repr__ + pragma: no cover ignore_errors = True diff --git a/hostpolicy/tests.py b/hostpolicy/tests.py index 8128c847..8ddc8b05 100644 --- a/hostpolicy/tests.py +++ b/hostpolicy/tests.py @@ -13,6 +13,16 @@ def clean_and_save(entity): entity.save() +class Internals(TestCase): + """Test internal data structures.""" + + def test_str(self): + """Test that __str__ returns obj.name.""" + name = "test1" + atom = HostPolicyAtom(name=name, description='test') + self.assertEqual(str(atom), '"' + name + '"') + + class UniqueNamespace(TestCase): """Atoms and Roles must jointly have unique names, so test that.""" diff --git a/mreg/api/permissions.py b/mreg/api/permissions.py index f22fd7bf..8f308978 100644 --- a/mreg/api/permissions.py +++ b/mreg/api/permissions.py @@ -1,7 +1,6 @@ from django.conf import settings - from rest_framework import exceptions -from rest_framework.permissions import IsAuthenticated, SAFE_METHODS +from rest_framework.permissions import SAFE_METHODS, IsAuthenticated from mreg.api.v1.serializers import HostSerializer from mreg.models import HostGroup, NetGroupRegexPermission, Network @@ -35,9 +34,10 @@ def _list_in_list(list_a, list_b): return any(i in list_b for i in list_a) -def user_in_required_group(user): - return _list_in_list(get_settings_groups('REQUIRED_USER_GROUPS'), - user.group_list) +def user_in_required_group(user, required_groups=[]): + if not required_groups: + required_groups = get_settings_groups('REQUIRED_USER_GROUPS') + return _list_in_list(required_groups, user.group_list) def user_is_superuser(user): @@ -257,7 +257,9 @@ def has_create_permission(self, request, view, validated_serializer): hostname = data['host'].name elif 'host' in data: hostname, ips = self._get_hostname_and_ips(data['host']) - else: + else: # pragma: no cover + # Testing these kinds of should-never-happen codepaths is hard. + # We have to basically mock a complete API call and then break it. raise exceptions.PermissionDenied(f"Unhandled view: {view}") if ips and hostname: @@ -274,7 +276,9 @@ def has_destroy_permission(self, request, view, validated_serializer): pass elif hasattr(obj, 'host'): obj = obj.host - else: + else: # pragma: no cover + # Testing these kinds of should-never-happen codepaths is hard. + # We have to basically mock a complete API call and then break it. raise exceptions.PermissionDenied(f"Unhandled view: {view}") if _deny_superuser_only_names(name=obj.name, view=view, request=request): return False @@ -313,7 +317,9 @@ def has_update_permission(self, request, view, validated_serializer): if not self.has_obj_perm(request.user, data['host']): return False return self.has_obj_perm(request.user, obj.host) - raise exceptions.PermissionDenied(f"Unhandled view: {view}") + # Testing these kinds of should-never-happen codepaths is hard. + # We have to basically mock a complete API call and then break it. + raise exceptions.PermissionDenied(f"Unhandled view: {view}") # pragma: no cover def _get_hostname_and_ips(self, hostobject): ips = [] diff --git a/mreg/api/v1/tests/test_host_permissions.py b/mreg/api/v1/tests/test_host_permissions.py index a806e1c8..17b060d0 100644 --- a/mreg/api/v1/tests/test_host_permissions.py +++ b/mreg/api/v1/tests/test_host_permissions.py @@ -1,10 +1,41 @@ from django.contrib.auth.models import Group +from rest_framework import exceptions + +from mreg.api.permissions import get_settings_groups, user_in_required_group +from mreg.models import ForwardZone, Host, Ipaddress, NetGroupRegexPermission, Network, PtrOverride, User -from mreg.models import ForwardZone, Host, Ipaddress, NetGroupRegexPermission, Network, PtrOverride from .tests import MregAPITestCase +class Internals(MregAPITestCase): + """Test internal structures in permissions.""" + def test_missing_group_settings(self): + """Ensure that missing group settings are caught if requested.""" + with self.assertRaises(exceptions.APIException): + get_settings_groups("NO_SUCH_SETTINGS_GROUP") + + def test_user_in_required_groups(self): + """Ensure that user_in_required_groups works.""" + testuser, _ = User.objects.get_or_create(username="testuser") + testgroup, _ = Group.objects.get_or_create(name="testgroup") + + with self.assertRaises(exceptions.APIException): + user_in_required_group(testuser) + + # Note that required groups contains the names of the groups, not objects. + # See the User definition in mreg/models_auth.py + self.assertFalse(user_in_required_group(testuser, required_groups=[testgroup.name])) + # The caching of group list requires us to manually expunge the cache. + # This operation has no API, so right no we do it the hard way. + testuser._group_list = None + testuser.groups.add(testgroup) + self.assertTrue(user_in_required_group(testuser, required_groups=[testgroup.name])) + + testuser.delete() + testgroup.delete() + + class HostsNoRights(MregAPITestCase): def setUp(self): diff --git a/mreg/api/v1/tests/test_networks.py b/mreg/api/v1/tests/test_networks.py index 68ee1dbf..d0d3636b 100644 --- a/mreg/api/v1/tests/test_networks.py +++ b/mreg/api/v1/tests/test_networks.py @@ -164,6 +164,11 @@ def test_networks_list_200_ok(self): self.assertEqual(response.data['count'], 4) self.assertEqual(len(response.data['results']), 4) + def test_networks_get_401_unauthorized(self): + """GET on an existing ip-network while unauthorized should return 401 Unauthorized.""" + self.client.logout() + self.assert_get_and_401('/networks/%s' % self.network_sample.network) + def test_networks_patch_204_no_content(self): """Patching an existing and valid entry should return 204 and Location""" response = self.assert_patch('/networks/%s' % self.network_sample.network, @@ -452,7 +457,6 @@ def test_client_must_be_logged_in(self): self.client.logout() self.assert_get_and_401('/networks/') - class NetworkExcludedRanges(MregAPITestCase): """Tests for NetworkExcludedRange objects and that they are enforced for Ipaddress and PtrOverride""" diff --git a/mreg/api/v1/tests/test_permissions.py b/mreg/api/v1/tests/test_permissions.py index 56ff54f9..7fbaee00 100644 --- a/mreg/api/v1/tests/test_permissions.py +++ b/mreg/api/v1/tests/test_permissions.py @@ -1,3 +1,5 @@ +from rest_framework.test import APIClient + from .tests import MregAPITestCase @@ -14,6 +16,15 @@ def test_get(self): ret2 = self.assert_get('/permissions/netgroupregex/{}'.format(ret1.json()['id'])) self.assertEqual(ret1.json(), ret2.json()) + def test_get_at_different_privilege_levels(self): + """Verify get at different privilege levels.""" + ret1 = self.assert_post('/permissions/netgroupregex/', self.data) + self.client = self.get_token_client(superuser=False, adminuser=False) + ret2 = self.assert_get('/permissions/netgroupregex/{}'.format(ret1.json()['id'])) + self.assertEqual(ret1.json(), ret2.json()) + self.client = APIClient() + self.assert_get_and_401('/permissions/netgroupregex/{}'.format(ret1.json()['id'])) + def test_list(self): ret1 = self.assert_post('/permissions/netgroupregex/', self.data) data = self.assert_get('/permissions/netgroupregex/').json()