From 935243c2d5f187175c1faf8b3648d0198e4d25ac Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Fri, 21 Jun 2024 15:49:52 +0200 Subject: [PATCH] More tests for `users` app (#7479) * disable patch coverage * also test for is_active flag * ignore edge case regarding coverage * add tests for api logout * test login redirects * style fixes * fully utilise serializer for /api/user/roles api * ignore logout mig from cov * bump api version as we are now documenting the roles endpoint * ignore on migration runs for coverage * remove dead code * ignore potential caching errors for coverage * test default dj_rest_auth token endpoint * move pragma * just ignore whole block * move ignore back * test for token based token revocation --- codecov.yml | 1 + .../InvenTree/InvenTree/api_version.py | 5 +- src/backend/InvenTree/users/api.py | 69 +++---------------- src/backend/InvenTree/users/apps.py | 2 +- .../migrations/0011_auto_20240523_1640.py | 3 +- src/backend/InvenTree/users/models.py | 23 +------ src/backend/InvenTree/users/serializers.py | 39 +++++++++-- src/backend/InvenTree/users/test_api.py | 29 ++++++++ src/backend/InvenTree/users/tests.py | 4 ++ 9 files changed, 86 insertions(+), 89 deletions(-) diff --git a/codecov.yml b/codecov.yml index 2edad7bb46..79d066e93b 100644 --- a/codecov.yml +++ b/codecov.yml @@ -3,6 +3,7 @@ coverage: project: default: target: 82% + patch: off github_checks: annotations: true diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index 04b82cf536..c579d8d072 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,11 +1,14 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 207 +INVENTREE_API_VERSION = 208 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v208 - 2024-06-19 : https://github.com/inventree/InvenTree/pull/7479 + - Adds documentation for the user roles API endpoint (no functional changes) + v207 - 2024-06-09 : https://github.com/inventree/InvenTree/pull/7420 - Moves all "Attachment" models into a single table - All "Attachment" operations are now performed at /api/attachment/ diff --git a/src/backend/InvenTree/users/api.py b/src/backend/InvenTree/users/api.py index e61de9a153..bbdc3be975 100644 --- a/src/backend/InvenTree/users/api.py +++ b/src/backend/InvenTree/users/api.py @@ -4,8 +4,7 @@ import datetime import logging from django.contrib.auth import authenticate, get_user, login, logout -from django.contrib.auth.models import Group, Permission, User -from django.db.models import Q +from django.contrib.auth.models import Group, User from django.http.response import HttpResponse from django.shortcuts import redirect from django.urls import include, path, re_path, reverse @@ -34,8 +33,8 @@ from InvenTree.mixins import ( ) from InvenTree.serializers import ExendedUserSerializer, UserCreateSerializer from InvenTree.settings import FRONTEND_URL_BASE -from users.models import ApiToken, Owner, RuleSet, check_user_role -from users.serializers import GroupSerializer, OwnerSerializer +from users.models import ApiToken, Owner +from users.serializers import GroupSerializer, OwnerSerializer, RoleSerializer logger = logging.getLogger('inventree') @@ -113,63 +112,15 @@ class OwnerDetail(RetrieveAPI): serializer_class = OwnerSerializer -class RoleDetails(APIView): - """API endpoint which lists the available role permissions for the current user. - - (Requires authentication) - """ +class RoleDetails(RetrieveAPI): + """API endpoint which lists the available role permissions for the current user.""" permission_classes = [permissions.IsAuthenticated] - serializer_class = None + serializer_class = RoleSerializer - def get(self, request, *args, **kwargs): - """Return the list of roles / permissions available to the current user.""" - user = request.user - - roles = {} - - for ruleset in RuleSet.RULESET_CHOICES: - role, _text = ruleset - - permissions = [] - - for permission in RuleSet.RULESET_PERMISSIONS: - if check_user_role(user, role, permission): - permissions.append(permission) - - if len(permissions) > 0: - roles[role] = permissions - else: - roles[role] = None # pragma: no cover - - # Extract individual permissions for the user - if user.is_superuser: - permissions = Permission.objects.all() - else: - permissions = Permission.objects.filter( - Q(user=user) | Q(group__user=user) - ).distinct() - - perms = {} - - for permission in permissions: - perm, model = permission.codename.split('_') - - if model not in perms: - perms[model] = [] - - perms[model].append(perm) - - data = { - 'user': user.pk, - 'username': user.username, - 'roles': roles, - 'permissions': perms, - 'is_staff': user.is_staff, - 'is_superuser': user.is_superuser, - } - - return Response(data) + def get_object(self): + """Overwritten to always return current user.""" + return self.request.user class UserDetail(RetrieveUpdateDestroyAPI): @@ -312,7 +263,7 @@ class Logout(LogoutView): try: token = ApiToken.objects.get(key=token_key, user=request.user) token.delete() - except ApiToken.DoesNotExist: + except ApiToken.DoesNotExist: # pragma: no cover pass return super().logout(request) diff --git a/src/backend/InvenTree/users/apps.py b/src/backend/InvenTree/users/apps.py index a4c52ee777..fdf96ca0ec 100644 --- a/src/backend/InvenTree/users/apps.py +++ b/src/backend/InvenTree/users/apps.py @@ -26,7 +26,7 @@ class UsersConfig(AppConfig): # Skip if running migrations if InvenTree.ready.isRunningMigrations(): - return + return # pragma: no cover if InvenTree.ready.canAppAccessDatabase(allow_test=True): try: diff --git a/src/backend/InvenTree/users/migrations/0011_auto_20240523_1640.py b/src/backend/InvenTree/users/migrations/0011_auto_20240523_1640.py index a736de6f72..31d606fab2 100644 --- a/src/backend/InvenTree/users/migrations/0011_auto_20240523_1640.py +++ b/src/backend/InvenTree/users/migrations/0011_auto_20240523_1640.py @@ -6,7 +6,7 @@ from django.conf import settings from django.db import migrations -def clear_sessions(apps, schema_editor): +def clear_sessions(apps, schema_editor): # pragma: no cover """Clear all user sessions.""" # Ignore in test mode @@ -20,6 +20,7 @@ def clear_sessions(apps, schema_editor): except Exception: # Database may not be ready yet, so this does not matter anyhow pass + class Migration(migrations.Migration): atomic = False diff --git a/src/backend/InvenTree/users/models.py b/src/backend/InvenTree/users/models.py index df237c3416..48bb915b6a 100644 --- a/src/backend/InvenTree/users/models.py +++ b/src/backend/InvenTree/users/models.py @@ -678,25 +678,6 @@ def clear_user_role_cache(user): cache.delete(key) -def get_user_roles(user): - """Return all roles available to a given user.""" - roles = set() - - for group in user.groups.all(): - for rule in group.rule_sets.all(): - name = rule.name - if rule.can_view: - roles.add(f'{name}.view') - if rule.can_add: - roles.add(f'{name}.add') - if rule.can_change: - roles.add(f'{name}.change') - if rule.can_delete: - roles.add(f'{name}.delete') - - return roles - - def check_user_role(user, role, permission): """Check if a user has a particular role:permission combination. @@ -710,7 +691,7 @@ def check_user_role(user, role, permission): try: result = cache.get(key) - except Exception: + except Exception: # pragma: no cover result = None if result is not None: @@ -741,7 +722,7 @@ def check_user_role(user, role, permission): # Save result to cache try: cache.set(key, result, timeout=3600) - except Exception: + except Exception: # pragma: no cover pass return result diff --git a/src/backend/InvenTree/users/serializers.py b/src/backend/InvenTree/users/serializers.py index b4e3dbb7da..583172d51c 100644 --- a/src/backend/InvenTree/users/serializers.py +++ b/src/backend/InvenTree/users/serializers.py @@ -1,6 +1,7 @@ """DRF API serializers for the 'users' app.""" -from django.contrib.auth.models import Group, User +from django.contrib.auth.models import Group, Permission, User +from django.db.models import Q from rest_framework import serializers @@ -40,16 +41,21 @@ class RoleSerializer(InvenTreeModelSerializer): """Metaclass options.""" model = User - fields = ['user', 'username', 'is_staff', 'is_superuser', 'roles'] + fields = [ + 'user', + 'username', + 'roles', + 'permissions', + 'is_staff', + 'is_superuser', + ] user = serializers.IntegerField(source='pk') - username = serializers.CharField() - is_staff = serializers.BooleanField() - is_superuser = serializers.BooleanField() roles = serializers.SerializerMethodField() + permissions = serializers.SerializerMethodField() def get_roles(self, user: User) -> dict: - """Return roles associated with the specified User.""" + """Roles associated with the user.""" roles = {} for ruleset in RuleSet.RULESET_CHOICES: @@ -67,3 +73,24 @@ class RoleSerializer(InvenTreeModelSerializer): roles[role] = None # pragma: no cover return roles + + def get_permissions(self, user: User) -> dict: + """Permissions associated with the user.""" + if user.is_superuser: + permissions = Permission.objects.all() + else: + permissions = Permission.objects.filter( + Q(user=user) | Q(group__user=user) + ).distinct() + + perms = {} + + for permission in permissions: + perm, model = permission.codename.split('_') + + if model not in perms: + perms[model] = [] + + perms[model].append(perm) + + return perms diff --git a/src/backend/InvenTree/users/test_api.py b/src/backend/InvenTree/users/test_api.py index c25b9acbff..6e9805c601 100644 --- a/src/backend/InvenTree/users/test_api.py +++ b/src/backend/InvenTree/users/test_api.py @@ -48,6 +48,25 @@ class UserAPITests(InvenTreeAPITestCase): self.assertIn('name', response.data) + def test_logout(self): + """Test api logout endpoint.""" + token_key = self.get(url=reverse('api-token')).data['token'] + self.client.logout() + self.client.credentials(HTTP_AUTHORIZATION='Token ' + token_key) + + self.post(reverse('api-logout'), expected_code=200) + self.get(reverse('api-token'), expected_code=401) + + def test_login_redirect(self): + """Test login redirect endpoint.""" + response = self.get(reverse('api-login-redirect'), expected_code=302) + self.assertEqual(response.url, '/index/') + + # PUI + self.put(reverse('api-ui-preference'), {'preferred_method': 'pui'}) + response = self.get(reverse('api-login-redirect'), expected_code=302) + self.assertEqual(response.url, '/platform/logged-in/') + class UserTokenTests(InvenTreeAPITestCase): """Tests for user token functionality.""" @@ -156,3 +175,13 @@ class UserTokenTests(InvenTreeAPITestCase): token.save() self.client.get(me, expected_code=200) + + def test_buildin_token(self): + """Test the built-in token authentication.""" + response = self.post( + reverse('rest_login'), + {'username': self.username, 'password': self.password}, + expected_code=200, + ) + self.assertIn('key', response.data) + self.assertTrue(response.data['key'].startswith('inv-')) diff --git a/src/backend/InvenTree/users/tests.py b/src/backend/InvenTree/users/tests.py index 786ab142a0..968d9024b3 100644 --- a/src/backend/InvenTree/users/tests.py +++ b/src/backend/InvenTree/users/tests.py @@ -221,6 +221,10 @@ class OwnerModelTest(InvenTreeTestCase): self.client.login(username=self.username, password=self.password) # user list self.do_request(reverse('api-owner-list'), {}) + + # user list with 'is_active' filter + self.do_request(reverse('api-owner-list'), {'is_active': False}) + # user list with search self.do_request(reverse('api-owner-list'), {'search': 'user'})