From ffd55cf164d9d628b30a047f668498413e7a30d2 Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Mon, 22 Jul 2024 05:21:59 +0200 Subject: [PATCH] [PUI] Add permissions to groups (#7621) * Add permissions to group API * factor out permission formatting * add group permission details to UI * add nicer accordions with permissions * add group to models * Add Admin button to change permissions * add missing instance renderer * turn off default view permission to everything * add migration * fix rule assigment * Add now missing view permissions * Adjust test for the now new default permission count * add missing view permission * fix permissions for search test * adjust search testing to also account for missing permissions * adjust to new defaults * expand role testing --------- Co-authored-by: Oliver --- src/backend/InvenTree/InvenTree/test_api.py | 25 ++++++++- .../InvenTree/InvenTree/test_middleware.py | 1 + src/backend/InvenTree/InvenTree/unit_test.py | 3 +- src/backend/InvenTree/company/test_api.py | 4 +- src/backend/InvenTree/order/test_api.py | 1 + src/backend/InvenTree/part/test_api.py | 5 +- src/backend/InvenTree/users/api.py | 21 ++++++- .../migrations/0012_alter_ruleset_can_view.py | 20 +++++++ src/backend/InvenTree/users/models.py | 2 +- src/backend/InvenTree/users/serializers.py | 40 ++++++++++--- src/backend/InvenTree/users/tests.py | 4 +- .../src/components/render/Instance.tsx | 3 +- .../src/components/render/ModelType.tsx | 8 +++ src/frontend/src/components/render/User.tsx | 6 ++ src/frontend/src/enums/ModelType.tsx | 1 + .../src/tables/settings/GroupTable.tsx | 56 ++++++++++++++++--- 16 files changed, 170 insertions(+), 30 deletions(-) create mode 100644 src/backend/InvenTree/users/migrations/0012_alter_ruleset_can_view.py diff --git a/src/backend/InvenTree/InvenTree/test_api.py b/src/backend/InvenTree/InvenTree/test_api.py index ea1a6c5b4a..e462af3b50 100644 --- a/src/backend/InvenTree/InvenTree/test_api.py +++ b/src/backend/InvenTree/InvenTree/test_api.py @@ -62,6 +62,7 @@ class APITests(InvenTreeAPITestCase): """Tests for the InvenTree API.""" fixtures = ['location', 'category', 'part', 'stock'] + roles = ['part.view'] token = None auto_login = False @@ -132,6 +133,7 @@ class APITests(InvenTreeAPITestCase): # Now log in! self.basicAuth() + self.assignRole('part.view') response = self.get(url) @@ -147,12 +149,17 @@ class APITests(InvenTreeAPITestCase): role_names = roles.keys() - # By default, 'view' permissions are provided + # By default, no permissions are provided for rule in RuleSet.RULESET_NAMES: self.assertIn(rule, role_names) - self.assertIn('view', roles[rule]) + if roles[rule] is None: + continue + if rule == 'part': + self.assertIn('view', roles[rule]) + else: + self.assertNotIn('view', roles[rule]) self.assertNotIn('add', roles[rule]) self.assertNotIn('change', roles[rule]) self.assertNotIn('delete', roles[rule]) @@ -297,6 +304,7 @@ class SearchTests(InvenTreeAPITestCase): 'order', 'sales_order', ] + roles = ['build.view', 'part.view'] def test_empty(self): """Test empty request.""" @@ -331,6 +339,19 @@ class SearchTests(InvenTreeAPITestCase): {'search': '01', 'limit': 2, 'purchaseorder': {}, 'salesorder': {}}, expected_code=200, ) + self.assertEqual( + response.data['purchaseorder'], + {'error': 'User does not have permission to view this model'}, + ) + + # Add permissions and try again + self.assignRole('purchase_order.view') + self.assignRole('sales_order.view') + response = self.post( + reverse('api-search'), + {'search': '01', 'limit': 2, 'purchaseorder': {}, 'salesorder': {}}, + expected_code=200, + ) self.assertEqual(response.data['purchaseorder']['count'], 1) self.assertEqual(response.data['salesorder']['count'], 0) diff --git a/src/backend/InvenTree/InvenTree/test_middleware.py b/src/backend/InvenTree/InvenTree/test_middleware.py index c4fcc52858..e00ea8a00f 100644 --- a/src/backend/InvenTree/InvenTree/test_middleware.py +++ b/src/backend/InvenTree/InvenTree/test_middleware.py @@ -71,6 +71,7 @@ class MiddlewareTests(InvenTreeTestCase): def test_error_exceptions(self): """Test that ignored errors are not logged.""" + self.assignRole('part.view') def check(excpected_nbr=0): # Check that errors are empty diff --git a/src/backend/InvenTree/InvenTree/unit_test.py b/src/backend/InvenTree/InvenTree/unit_test.py index 3f64fe1d9c..2b525b9aa5 100644 --- a/src/backend/InvenTree/InvenTree/unit_test.py +++ b/src/backend/InvenTree/InvenTree/unit_test.py @@ -204,7 +204,8 @@ class UserMixin: ruleset.can_add = True ruleset.save() - break + if not assign_all: + break class PluginMixin: diff --git a/src/backend/InvenTree/company/test_api.py b/src/backend/InvenTree/company/test_api.py index afe04e6653..78064a3bc5 100644 --- a/src/backend/InvenTree/company/test_api.py +++ b/src/backend/InvenTree/company/test_api.py @@ -160,7 +160,7 @@ class CompanyTest(InvenTreeAPITestCase): class ContactTest(InvenTreeAPITestCase): """Tests for the Contact models.""" - roles = [] + roles = ['purchase_order.view'] @classmethod def setUpTestData(cls): @@ -266,7 +266,7 @@ class ContactTest(InvenTreeAPITestCase): class AddressTest(InvenTreeAPITestCase): """Test cases for Address API endpoints.""" - roles = [] + roles = ['purchase_order.view'] @classmethod def setUpTestData(cls): diff --git a/src/backend/InvenTree/order/test_api.py b/src/backend/InvenTree/order/test_api.py index 7ebabf97b5..4d8a78de8a 100644 --- a/src/backend/InvenTree/order/test_api.py +++ b/src/backend/InvenTree/order/test_api.py @@ -2010,6 +2010,7 @@ class ReturnOrderTests(InvenTreeAPITestCase): 'supplier_part', 'stock', ] + roles = ['return_order.view'] def test_options(self): """Test the OPTIONS endpoint.""" diff --git a/src/backend/InvenTree/part/test_api.py b/src/backend/InvenTree/part/test_api.py index 078c53f586..3d4e4909d9 100644 --- a/src/backend/InvenTree/part/test_api.py +++ b/src/backend/InvenTree/part/test_api.py @@ -512,7 +512,7 @@ class PartOptionsAPITest(InvenTreeAPITestCase): Ensure that the required field details are provided! """ - roles = ['part.add'] + roles = ['part.add', 'part_category.view'] def test_part(self): """Test the Part API OPTIONS.""" @@ -2149,7 +2149,7 @@ class BomItemTest(InvenTreeAPITestCase): fixtures = ['category', 'part', 'location', 'stock', 'bom', 'company'] - roles = ['part.add', 'part.change', 'part.delete'] + roles = ['part.add', 'part.change', 'part.delete', 'stock.view'] def setUp(self): """Set up the test case.""" @@ -2642,6 +2642,7 @@ class PartStocktakeTest(InvenTreeAPITestCase): superuser = False is_staff = False + roles = ['stocktake.view'] fixtures = ['category', 'part', 'location', 'stock'] diff --git a/src/backend/InvenTree/users/api.py b/src/backend/InvenTree/users/api.py index bbdc3be975..fd6400b6bc 100644 --- a/src/backend/InvenTree/users/api.py +++ b/src/backend/InvenTree/users/api.py @@ -162,7 +162,24 @@ class UserList(ListCreateAPI): filterset_fields = ['is_staff', 'is_active', 'is_superuser'] -class GroupDetail(RetrieveUpdateDestroyAPI): +class GroupMixin: + """Mixin for Group API endpoints to add permissions filter.""" + + def get_serializer(self, *args, **kwargs): + """Return serializer instance for this endpoint.""" + # Do we wish to include extra detail? + try: + params = self.request.query_params + kwargs['permission_detail'] = InvenTree.helpers.str2bool( + params.get('permission_detail', None) + ) + except AttributeError: + pass + kwargs['context'] = self.get_serializer_context() + return self.serializer_class(*args, **kwargs) + + +class GroupDetail(GroupMixin, RetrieveUpdateDestroyAPI): """Detail endpoint for a particular auth group.""" queryset = Group.objects.all() @@ -170,7 +187,7 @@ class GroupDetail(RetrieveUpdateDestroyAPI): permission_classes = [permissions.IsAuthenticated] -class GroupList(ListCreateAPI): +class GroupList(GroupMixin, ListCreateAPI): """List endpoint for all auth groups.""" queryset = Group.objects.all() diff --git a/src/backend/InvenTree/users/migrations/0012_alter_ruleset_can_view.py b/src/backend/InvenTree/users/migrations/0012_alter_ruleset_can_view.py new file mode 100644 index 0000000000..fbe0a7bebd --- /dev/null +++ b/src/backend/InvenTree/users/migrations/0012_alter_ruleset_can_view.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.12 on 2024-07-18 21:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0011_auto_20240523_1640"), + ] + + operations = [ + migrations.AlterField( + model_name="ruleset", + name="can_view", + field=models.BooleanField( + default=False, help_text="Permission to view items", verbose_name="View" + ), + ), + ] diff --git a/src/backend/InvenTree/users/models.py b/src/backend/InvenTree/users/models.py index 9b8a72b292..7a0cd636aa 100644 --- a/src/backend/InvenTree/users/models.py +++ b/src/backend/InvenTree/users/models.py @@ -389,7 +389,7 @@ class RuleSet(models.Model): ) can_view = models.BooleanField( - verbose_name=_('View'), default=True, help_text=_('Permission to view items') + verbose_name=_('View'), default=False, help_text=_('Permission to view items') ) can_add = models.BooleanField( diff --git a/src/backend/InvenTree/users/serializers.py b/src/backend/InvenTree/users/serializers.py index 583172d51c..c6c34fde3b 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, Permission, User +from django.core.exceptions import AppRegistryNotReady from django.db.models import Q from rest_framework import serializers @@ -31,7 +32,25 @@ class GroupSerializer(InvenTreeModelSerializer): """Metaclass defines serializer fields.""" model = Group - fields = ['pk', 'name'] + fields = ['pk', 'name', 'permissions'] + + def __init__(self, *args, **kwargs): + """Initialize this serializer with extra fields as required.""" + permission_detail = kwargs.pop('permission_detail', False) + + super().__init__(*args, **kwargs) + + try: + if not permission_detail: + self.fields.pop('permissions', None) + except AppRegistryNotReady: + pass + + permissions = serializers.SerializerMethodField() + + def get_permissions(self, group: Group): + """Return a list of permissions associated with the group.""" + return generate_permission_dict(group.permissions.all()) class RoleSerializer(InvenTreeModelSerializer): @@ -83,14 +102,19 @@ class RoleSerializer(InvenTreeModelSerializer): Q(user=user) | Q(group__user=user) ).distinct() - perms = {} + return generate_permission_dict(permissions) - for permission in permissions: - perm, model = permission.codename.split('_') - if model not in perms: - perms[model] = [] +def generate_permission_dict(permissions): + """Generate a dictionary of permissions for a given set of permissions.""" + perms = {} - perms[model].append(perm) + for permission in permissions: + perm, model = permission.codename.split('_') - return perms + if model not in perms: + perms[model] = [] + + perms[model].append(perm) + + return perms diff --git a/src/backend/InvenTree/users/tests.py b/src/backend/InvenTree/users/tests.py index 968d9024b3..1d5d9686c0 100644 --- a/src/backend/InvenTree/users/tests.py +++ b/src/backend/InvenTree/users/tests.py @@ -123,8 +123,8 @@ class RuleSetModelTest(TestCase): for model in models: permission_set.add(model) - # Every ruleset by default sets one permission, the "view" permission set - self.assertEqual(group.permissions.count(), len(permission_set)) + # By default no permissions should be assigned + self.assertEqual(group.permissions.count(), 0) # Add some more rules for rule in rulesets: diff --git a/src/frontend/src/components/render/Instance.tsx b/src/frontend/src/components/render/Instance.tsx index ab9cdd80e8..96a91d3a0e 100644 --- a/src/frontend/src/components/render/Instance.tsx +++ b/src/frontend/src/components/render/Instance.tsx @@ -37,7 +37,7 @@ import { RenderStockLocation, RenderStockLocationType } from './Stock'; -import { RenderOwner, RenderUser } from './User'; +import { RenderGroup, RenderOwner, RenderUser } from './User'; type EnumDictionary = { [K in T]: U; @@ -81,6 +81,7 @@ const RendererLookup: EnumDictionary< [ModelType.stockhistory]: RenderStockItem, [ModelType.supplierpart]: RenderSupplierPart, [ModelType.user]: RenderUser, + [ModelType.group]: RenderGroup, [ModelType.importsession]: RenderImportSession, [ModelType.reporttemplate]: RenderReportTemplate, [ModelType.labeltemplate]: RenderLabelTemplate, diff --git a/src/frontend/src/components/render/ModelType.tsx b/src/frontend/src/components/render/ModelType.tsx index 50a22c82ba..5b4c1133cd 100644 --- a/src/frontend/src/components/render/ModelType.tsx +++ b/src/frontend/src/components/render/ModelType.tsx @@ -201,6 +201,14 @@ export const ModelInformationDict: ModelDict = { url_detail: '/user/:pk/', api_endpoint: ApiEndpoints.user_list }, + group: { + label: t`Group`, + label_multiple: t`Groups`, + url_overview: '/user/group', + url_detail: '/user/group-:pk', + api_endpoint: ApiEndpoints.group_list, + admin_url: '/auth/group/' + }, importsession: { label: t`Import Session`, label_multiple: t`Import Sessions`, diff --git a/src/frontend/src/components/render/User.tsx b/src/frontend/src/components/render/User.tsx index 005351c0d6..16fdb9710e 100644 --- a/src/frontend/src/components/render/User.tsx +++ b/src/frontend/src/components/render/User.tsx @@ -28,3 +28,9 @@ export function RenderUser({ ) ); } + +export function RenderGroup({ + instance +}: Readonly): ReactNode { + return instance && ; +} diff --git a/src/frontend/src/enums/ModelType.tsx b/src/frontend/src/enums/ModelType.tsx index 570c382c25..6a1a1bb212 100644 --- a/src/frontend/src/enums/ModelType.tsx +++ b/src/frontend/src/enums/ModelType.tsx @@ -27,6 +27,7 @@ export enum ModelType { contact = 'contact', owner = 'owner', user = 'user', + group = 'group', reporttemplate = 'reporttemplate', labeltemplate = 'labeltemplate', pluginconfig = 'pluginconfig' diff --git a/src/frontend/src/tables/settings/GroupTable.tsx b/src/frontend/src/tables/settings/GroupTable.tsx index bbbd05a3c3..429e26e80f 100644 --- a/src/frontend/src/tables/settings/GroupTable.tsx +++ b/src/frontend/src/tables/settings/GroupTable.tsx @@ -1,13 +1,23 @@ import { Trans, t } from '@lingui/macro'; -import { Group, LoadingOverlay, Stack, Text, Title } from '@mantine/core'; +import { + Accordion, + Group, + LoadingOverlay, + Pill, + PillGroup, + Stack, + Text, + Title +} from '@mantine/core'; import { useCallback, useMemo, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import { AddItemButton } from '../../components/buttons/AddItemButton'; +import AdminButton from '../../components/buttons/AdminButton'; import { EditApiForm } from '../../components/forms/ApiForm'; -import { PlaceholderPill } from '../../components/items/Placeholder'; import { DetailDrawer } from '../../components/nav/DetailDrawer'; import { ApiEndpoints } from '../../enums/ApiEndpoints'; +import { ModelType } from '../../enums/ModelType'; import { useCreateApiFormModal, useDeleteApiFormModal @@ -32,14 +42,42 @@ export function GroupDrawer({ refreshTable: () => void; }) { const { + instance, refreshInstance, instanceQuery: { isFetching, error } } = useInstance({ endpoint: ApiEndpoints.group_list, pk: id, - throwError: true + throwError: true, + params: { + permission_detail: true + } }); + const permissionsAccordion = useMemo(() => { + if (!instance?.permissions) return null; + + const data = instance.permissions; + return ( + + {Object.keys(data).map((key) => ( + + + {instance.permissions[key].length} {key} + + + + {data[key].map((perm: string) => ( + {perm} + ))} + + + + ))} + + ); + }, [instance]); + if (isFetching) { return ; } @@ -72,13 +110,13 @@ export function GroupDrawer({ }} id={`group-detail-drawer-${id}`} /> - - - <Trans>Permission set</Trans> - - - + + + <Trans>Permission set</Trans> + + + {permissionsAccordion} ); }