Skip to content

Commit

Permalink
feat: Increase coverage.
Browse files Browse the repository at this point in the history
Fixes unioslo#483

First pass:
  - Add pragma to exempt lines from coverage to .coveragerc
  - Add test for stringifying HostPolicyComponent-based objects (see unioslo#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).
  • Loading branch information
terjekv committed Mar 17, 2023
1 parent 66c74af commit 7c575e6
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 11 deletions.
4 changes: 3 additions & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ omit =


[report]
exclude_lines = def __repr__
exclude_lines =
def __repr__
pragma: no cover

ignore_errors = True

Expand Down
10 changes: 10 additions & 0 deletions hostpolicy/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
22 changes: 14 additions & 8 deletions mreg/api/permissions.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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 = []
Expand Down
33 changes: 32 additions & 1 deletion mreg/api/v1/tests/test_host_permissions.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down
6 changes: 5 additions & 1 deletion mreg/api/v1/tests/test_networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"""
Expand Down
11 changes: 11 additions & 0 deletions mreg/api/v1/tests/test_permissions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from rest_framework.test import APIClient

from .tests import MregAPITestCase


Expand All @@ -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()
Expand Down

0 comments on commit 7c575e6

Please sign in to comment.