Move user & group M2M assignments for ObjectPermission

netbox/users/api/serializers_/permissions.py

@@ -1,11 +1,9 @@
-from django.contrib.auth import get_user_model
 from rest_framework import serializers
 
 from core.models import ObjectType
-from netbox.api.fields import ContentTypeField, SerializedPKRelatedField
+from netbox.api.fields import ContentTypeField
 from netbox.api.serializers import ValidatedModelSerializer
-from users.models import Group, ObjectPermission
+from users.models import ObjectPermission
-from .users import GroupSerializer, UserSerializer
 
 __all__ = (
     'ObjectPermissionSerializer',
@@ -18,27 +16,12 @@ class ObjectPermissionSerializer(ValidatedModelSerializer):
         queryset=ObjectType.objects.all(),
         many=True
     )
-    groups = SerializedPKRelatedField(
-        queryset=Group.objects.all(),
-        serializer=GroupSerializer,
-        nested=True,
-        required=False,
-        many=True
-    )
-    users = SerializedPKRelatedField(
-        queryset=get_user_model().objects.all(),
-        serializer=UserSerializer,
-        nested=True,
-        required=False,
-        many=True
-    )
 
     class Meta:
         model = ObjectPermission
         fields = (
-            'id', 'url', 'display', 'name', 'description', 'enabled', 'object_types', 'groups', 'users', 'actions',
+            'id', 'url', 'display', 'name', 'description', 'enabled', 'object_types', 'actions', 'constraints',
-            'constraints',
         )
         brief_fields = (
-            'id', 'url', 'display', 'name', 'description', 'enabled', 'object_types', 'groups', 'users', 'actions',
+            'id', 'url', 'display', 'name', 'description', 'enabled', 'object_types', 'actions',
         )

netbox/users/api/serializers_/users.py

@@ -5,7 +5,8 @@ from rest_framework import serializers
 
 from netbox.api.fields import SerializedPKRelatedField
 from netbox.api.serializers import ValidatedModelSerializer
-from users.models import Group
+from users.models import Group, ObjectPermission
+from .permissions import ObjectPermissionSerializer
 
 __all__ = (
     'GroupSerializer',
@@ -16,10 +17,18 @@ __all__ = (
 class GroupSerializer(ValidatedModelSerializer):
     url = serializers.HyperlinkedIdentityField(view_name='users-api:group-detail')
     user_count = serializers.IntegerField(read_only=True)
+    permissions = SerializedPKRelatedField(
+        source='object_permissions',
+        queryset=ObjectPermission.objects.all(),
+        serializer=ObjectPermissionSerializer,
+        nested=True,
+        required=False,
+        many=True
+    )
 
     class Meta:
         model = Group
-        fields = ('id', 'url', 'display', 'name', 'user_count')
+        fields = ('id', 'url', 'display', 'name', 'permissions', 'user_count')
         brief_fields = ('id', 'url', 'display', 'name')
 
 
@@ -32,12 +41,20 @@ class UserSerializer(ValidatedModelSerializer):
         required=False,
         many=True
     )
+    permissions = SerializedPKRelatedField(
+        source='object_permissions',
+        queryset=ObjectPermission.objects.all(),
+        serializer=ObjectPermissionSerializer,
+        nested=True,
+        required=False,
+        many=True
+    )
 
     class Meta:
         model = get_user_model()
         fields = (
             'id', 'url', 'display', 'username', 'password', 'first_name', 'last_name', 'email', 'is_staff', 'is_active',
-            'date_joined', 'last_login', 'groups',
+            'date_joined', 'last_login', 'groups', 'permissions',
         )
         brief_fields = ('id', 'url', 'display', 'username')
         extra_kwargs = {

netbox/users/forms/model_forms.py

@@ -203,9 +203,6 @@ class UserForm(forms.ModelForm):
         super().__init__(*args, **kwargs)
 
         if self.instance.pk:
-            # Populate assigned permissions
-            self.fields['object_permissions'].initial = self.instance.object_permissions.values_list('id', flat=True)
-
             # Password fields are optional for existing Users
             self.fields['password'].required = False
             self.fields['confirm_password'].required = False
@@ -213,9 +210,6 @@ class UserForm(forms.ModelForm):
     def save(self, *args, **kwargs):
         instance = super().save(*args, **kwargs)
 
-        # Update assigned permissions
-        instance.object_permissions.set(self.cleaned_data['object_permissions'])
-
         # On edit, check if we have to save the password
         if self.cleaned_data.get('password'):
             instance.set_password(self.cleaned_data.get('password'))
@@ -260,14 +254,12 @@ class GroupForm(forms.ModelForm):
         # Populate assigned users and permissions
         if self.instance.pk:
             self.fields['users'].initial = self.instance.users.values_list('id', flat=True)
-            self.fields['object_permissions'].initial = self.instance.object_permissions.values_list('id', flat=True)
 
     def save(self, *args, **kwargs):
         instance = super().save(*args, **kwargs)
 
-        # Update assigned users and permissions
+        # Update assigned users
         instance.users.set(self.cleaned_data['users'])
-        instance.object_permissions.set(self.cleaned_data['object_permissions'])
 
         return instance
 
@@ -335,9 +327,10 @@ class ObjectPermissionForm(forms.ModelForm):
         # Make the actions field optional since the form uses it only for non-CRUD actions
         self.fields['actions'].required = False
 
-        # Order group and user fields
+        # Populate assigned users and groups
-        self.fields['groups'].queryset = self.fields['groups'].queryset.order_by('name')
+        if self.instance.pk:
-        self.fields['users'].queryset = self.fields['users'].queryset.order_by('username')
+            self.fields['groups'].initial = self.instance.groups.values_list('id', flat=True)
+            self.fields['users'].initial = self.instance.users.values_list('id', flat=True)
 
         # Check the appropriate checkboxes when editing an existing ObjectPermission
         if self.instance.pk:
@@ -381,3 +374,12 @@ class ObjectPermissionForm(forms.ModelForm):
                     raise forms.ValidationError({
                         'constraints': _('Invalid filter for {model}: {error}').format(model=model, error=e)
                     })
+
+    def save(self, *args, **kwargs):
+        instance = super().save(*args, **kwargs)
+
+        # Update assigned users and groups
+        instance.users.set(self.cleaned_data['users'])
+        instance.groups.set(self.cleaned_data['groups'])
+
+        return instance

netbox/users/migrations/0008_flip_objectpermission_assignments.py

@@ -0,0 +1,122 @@
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0007_objectpermission_update_object_types'),
+    ]
+
+    operations = [
+        # Flip M2M assignments for ObjectPermission to Groups
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.RemoveField(
+                    model_name='objectpermission',
+                    name='groups',
+                ),
+                migrations.AddField(
+                    model_name='group',
+                    name='object_permissions',
+                    field=models.ManyToManyField(blank=True, related_name='groups', to='users.objectpermission'),
+                ),
+            ],
+            database_operations=[
+                # Rename table
+                migrations.RunSQL(
+                    "ALTER TABLE users_objectpermission_groups"
+                    "  RENAME TO users_group_object_permissions"
+                ),
+                migrations.RunSQL(
+                    "ALTER TABLE users_objectpermission_groups_id_seq"
+                    "  RENAME TO users_group_object_permissions_id_seq"
+                ),
+
+                # Rename constraints
+                migrations.RunSQL(
+                    "ALTER TABLE users_group_object_permissions RENAME CONSTRAINT "
+                    "users_objectpermissi_group_id_fb7ba6e0_fk_users_gro TO "
+                    "users_group_object_p_group_id_90dd183a_fk_users_gro"
+                ),
+                migrations.RunSQL(
+                    "ALTER TABLE users_group_object_permissions RENAME CONSTRAINT "
+                    "users_objectpermissi_objectpermission_id_2f7cc117_fk_users_obj TO "
+                    "users_group_object_p_objectpermission_id_dd489dc4_fk_users_obj"
+                ),
+
+                # Rename indexes
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_groups_pkey "
+                    "  RENAME TO users_group_object_permissions_pkey"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_g_objectpermission_id_grou_3b62a39c_uniq "
+                    "  RENAME TO users_group_object_permi_group_id_objectpermissio_db1f8cbe_uniq"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_groups_group_id_fb7ba6e0"
+                    "  RENAME TO users_group_object_permissions_group_id_90dd183a"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_groups_objectpermission_id_2f7cc117"
+                    "  RENAME TO users_group_object_permissions_objectpermission_id_dd489dc4"
+                ),
+            ]
+        ),
+
+        # Flip M2M assignments for ObjectPermission to Users
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.RemoveField(
+                    model_name='objectpermission',
+                    name='users',
+                ),
+                migrations.AddField(
+                    model_name='user',
+                    name='object_permissions',
+                    field=models.ManyToManyField(blank=True, related_name='users', to='users.objectpermission'),
+                ),
+            ],
+            database_operations=[
+                # Rename table
+                migrations.RunSQL(
+                    "ALTER TABLE users_objectpermission_users"
+                    "  RENAME TO users_user_object_permissions"
+                ),
+                migrations.RunSQL(
+                    "ALTER TABLE users_objectpermission_users_id_seq"
+                    "  RENAME TO users_user_object_permissions_id_seq"
+                ),
+
+                # Rename constraints
+                migrations.RunSQL(
+                    "ALTER TABLE users_user_object_permissions RENAME CONSTRAINT "
+                    "users_objectpermissi_objectpermission_id_78a9c2e6_fk_users_obj TO "
+                    "users_user_object_pe_objectpermission_id_29b431b4_fk_users_obj"
+                ),
+                migrations.RunSQL(
+                    "ALTER TABLE users_user_object_permissions RENAME CONSTRAINT "
+                    "users_objectpermission_users_user_id_16c0905d_fk_auth_user_id TO "
+                    "users_user_object_permissions_user_id_9d647aac_fk_users_user_id"
+                ),
+
+                # Rename indexes
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_users_pkey "
+                    "  RENAME TO users_user_object_permissions_pkey"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_u_objectpermission_id_user_3a7db108_uniq "
+                    "  RENAME TO users_user_object_permis_user_id_objectpermission_0a98550e_uniq"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_users_user_id_16c0905d"
+                    "  RENAME TO users_user_object_permissions_user_id_9d647aac"
+                ),
+                migrations.RunSQL(
+                    "ALTER INDEX users_objectpermission_users_objectpermission_id_78a9c2e6"
+                    "  RENAME TO users_user_object_permissions_objectpermission_id_29b431b4"
+                ),
+            ]
+        ),
+    ]

netbox/users/models.py

@@ -53,6 +53,11 @@ class Group(models.Model):
         max_length=200,
         blank=True
     )
+    object_permissions = models.ManyToManyField(
+        to='users.ObjectPermission',
+        blank=True,
+        related_name='groups'
+    )
 
     # Replicate legacy Django permissions support from stock Group model
     # to ensure authentication backend compatibility
@@ -92,6 +97,11 @@ class User(AbstractUser):
         related_name='users',
         related_query_name='user'
     )
+    object_permissions = models.ManyToManyField(
+        to='users.ObjectPermission',
+        blank=True,
+        related_name='users'
+    )
 
     objects = UserManager()
 
@@ -387,16 +397,6 @@ class ObjectPermission(models.Model):
         limit_choices_to=OBJECTPERMISSION_OBJECT_TYPES,
         related_name='object_permissions'
     )
-    groups = models.ManyToManyField(
-        to='users.Group',
-        blank=True,
-        related_name='object_permissions'
-    )
-    users = models.ManyToManyField(
-        to=get_user_model(),
-        blank=True,
-        related_name='object_permissions'
-    )
     actions = ArrayField(
         base_field=models.CharField(max_length=30),
         help_text=_("The list of actions granted by this permission")

netbox/users/tests/test_api.py

@@ -16,29 +16,13 @@ class AppTest(APITestCase):
 
         url = reverse('users-api:api-root')
         response = self.client.get(f'{url}?format=api', **self.header)
-
         self.assertEqual(response.status_code, 200)
 
 
 class UserTest(APIViewTestCases.APIViewTestCase):
     model = User
-    view_namespace = 'users'
     brief_fields = ['display', 'id', 'url', 'username']
     validation_excluded_fields = ['password']
-    create_data = [
-        {
-            'username': 'User_4',
-            'password': 'password4',
-        },
-        {
-            'username': 'User_5',
-            'password': 'password5',
-        },
-        {
-            'username': 'User_6',
-            'password': 'password6',
-        },
-    ]
     bulk_update_data = {
         'email': 'test@example.com',
     }
@@ -46,6 +30,16 @@ class UserTest(APIViewTestCases.APIViewTestCase):
     @classmethod
     def setUpTestData(cls):
 
+        permissions = (
+            ObjectPermission(name='Permission 1', actions=['view']),
+            ObjectPermission(name='Permission 2', actions=['view']),
+            ObjectPermission(name='Permission 3', actions=['view']),
+        )
+        ObjectPermission.objects.bulk_create(permissions)
+        permissions[0].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'site'))
+        permissions[1].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'location'))
+        permissions[2].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'rack'))
+
         users = (
             User(username='User_1', password='password1'),
             User(username='User_2', password='password2'),
@@ -53,6 +47,24 @@ class UserTest(APIViewTestCases.APIViewTestCase):
         )
         User.objects.bulk_create(users)
 
+        cls.create_data = [
+            {
+                'username': 'User_4',
+                'password': 'password4',
+                'permissions': [permissions[0].pk],
+            },
+            {
+                'username': 'User_5',
+                'password': 'password5',
+                'permissions': [permissions[1].pk],
+            },
+            {
+                'username': 'User_6',
+                'password': 'password6',
+                'permissions': [permissions[2].pk],
+            },
+        ]
+
     def test_that_password_is_changed(self):
         """
         Test that password is changed
@@ -76,41 +88,56 @@ class UserTest(APIViewTestCases.APIViewTestCase):
             'password': 'newpassword'
         }
         url = reverse('users-api:user-detail', kwargs={'pk': user.id})
-
         response = self.client.patch(url, data, format='json', **self.header)
-
         self.assertEqual(response.status_code, 200)
-
+        user.refresh_from_db()
-        updated_user = User.objects.get(id=user.id)
+        self.assertTrue(user.check_password(data['password']))
-
-        self.assertTrue(updated_user.check_password(data['password']))
 
 
 class GroupTest(APIViewTestCases.APIViewTestCase):
     model = Group
-    view_namespace = 'users'
     brief_fields = ['display', 'id', 'name', 'url']
-    create_data = [
-        {
-            'name': 'Group 4',
-        },
-        {
-            'name': 'Group 5',
-        },
-        {
-            'name': 'Group 6',
-        },
-    ]
 
     @classmethod
     def setUpTestData(cls):
 
-        users = (
+        permissions = (
+            ObjectPermission(name='Permission 1', actions=['view']),
+            ObjectPermission(name='Permission 2', actions=['view']),
+            ObjectPermission(name='Permission 3', actions=['view']),
+        )
+        ObjectPermission.objects.bulk_create(permissions)
+        permissions[0].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'site'))
+        permissions[1].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'location'))
+        permissions[2].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'rack'))
+
+        groups = (
             Group(name='Group 1'),
             Group(name='Group 2'),
             Group(name='Group 3'),
         )
-        Group.objects.bulk_create(users)
+        Group.objects.bulk_create(groups)
+
+        cls.create_data = [
+            {
+                'name': 'Group 4',
+                'permissions': [permissions[0].pk],
+            },
+            {
+                'name': 'Group 5',
+                'permissions': [permissions[1].pk],
+            },
+            {
+                'name': 'Group 6',
+                'permissions': [permissions[2].pk],
+            },
+        ]
+
+    def model_to_dict(self, instance, *args, **kwargs):
+        # Overwrite permissions attr to work around the serializer field having a different name
+        data = super().model_to_dict(instance, *args, **kwargs)
+        data['permissions'] = list(instance.object_permissions.values_list('id', flat=True))
+        return data
 
     def test_bulk_update_objects(self):
         """
@@ -240,9 +267,7 @@ class ObjectPermissionTest(
     APIViewTestCases.DeleteObjectViewTestCase
 ):
     model = ObjectPermission
-    brief_fields = [
+    brief_fields = ['actions', 'description', 'display', 'enabled', 'id', 'name', 'object_types', 'url']
-        'actions', 'description', 'display', 'enabled', 'groups', 'id', 'name', 'object_types', 'url', 'users',
-    ]
 
     @classmethod
     def setUpTestData(cls):
@@ -278,24 +303,18 @@ class ObjectPermissionTest(
             {
                 'name': 'Permission 4',
                 'object_types': ['dcim.site'],
-                'groups': [groups[0].pk],
-                'users': [users[0].pk],
                 'actions': ['view', 'add', 'change', 'delete'],
                 'constraints': {'name': 'TEST4'},
             },
             {
                 'name': 'Permission 5',
                 'object_types': ['dcim.site'],
-                'groups': [groups[1].pk],
-                'users': [users[1].pk],
                 'actions': ['view', 'add', 'change', 'delete'],
                 'constraints': {'name': 'TEST5'},
             },
             {
                 'name': 'Permission 6',
                 'object_types': ['dcim.site'],
-                'groups': [groups[2].pk],
-                'users': [users[2].pk],
                 'actions': ['view', 'add', 'change', 'delete'],
                 'constraints': {'name': 'TEST6'},
             },