From 6624fc607602502bd40f7124cc98bb050d96c01c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 8 May 2020 17:30:25 -0400 Subject: [PATCH] Initial work on #554 (WIP) --- netbox/netbox/settings.py | 1 + netbox/users/admin.py | 9 ++- .../users/migrations/0007_objectpermission.py | 36 +++++++++++ netbox/users/models.py | 55 +++++++++++++++- netbox/users/tests/test_permissions.py | 62 +++++++++++++++++++ netbox/utilities/auth_backends.py | 42 +++++++++++++ 6 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 netbox/users/migrations/0007_objectpermission.py create mode 100644 netbox/users/tests/test_permissions.py diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index bf660696b..5c48ee620 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -335,6 +335,7 @@ TEMPLATES = [ AUTHENTICATION_BACKENDS = [ REMOTE_AUTH_BACKEND, 'utilities.auth_backends.ViewExemptModelBackend', + 'utilities.auth_backends.ObjectPermissionBackend', ] # Internationalization diff --git a/netbox/users/admin.py b/netbox/users/admin.py index 42e651712..fcaeb4ef0 100644 --- a/netbox/users/admin.py +++ b/netbox/users/admin.py @@ -3,7 +3,7 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin as UserAdmin_ from django.contrib.auth.models import User -from .models import Token, UserConfig +from .models import ObjectPermission, Token, UserConfig # Unregister the built-in UserAdmin so that we can use our custom admin view below admin.site.unregister(User) @@ -43,3 +43,10 @@ class TokenAdmin(admin.ModelAdmin): list_display = [ 'key', 'user', 'created', 'expires', 'write_enabled', 'description' ] + + +@admin.register(ObjectPermission) +class ObjectPermissionAdmin(admin.ModelAdmin): + list_display = [ + 'model', 'can_view', 'can_add', 'can_change', 'can_delete' + ] diff --git a/netbox/users/migrations/0007_objectpermission.py b/netbox/users/migrations/0007_objectpermission.py new file mode 100644 index 000000000..d805c3379 --- /dev/null +++ b/netbox/users/migrations/0007_objectpermission.py @@ -0,0 +1,36 @@ +# Generated by Django 3.0.6 on 2020-05-08 20:18 + +from django.conf import settings +import django.contrib.postgres.fields.jsonb +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('auth', '0011_update_proxy_permissions'), + ('contenttypes', '0002_remove_content_type_name'), + ('users', '0006_create_userconfigs'), + ] + + operations = [ + migrations.CreateModel( + name='ObjectPermission', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)), + ('attrs', django.contrib.postgres.fields.jsonb.JSONField()), + ('can_view', models.BooleanField(default=False)), + ('can_add', models.BooleanField(default=False)), + ('can_change', models.BooleanField(default=False)), + ('can_delete', models.BooleanField(default=False)), + ('groups', models.ManyToManyField(blank=True, related_name='object_permissions', to='auth.Group')), + ('model', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')), + ('users', models.ManyToManyField(blank=True, related_name='object_permissions', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'unique_together': {('model', 'attrs')}, + }, + ), + ] diff --git a/netbox/users/models.py b/netbox/users/models.py index ea5762232..f2002ae95 100644 --- a/netbox/users/models.py +++ b/netbox/users/models.py @@ -1,8 +1,10 @@ import binascii import os -from django.contrib.auth.models import User +from django.contrib.auth.models import Group, User +from django.contrib.contenttypes.models import ContentType from django.contrib.postgres.fields import JSONField +from django.core.exceptions import FieldError, ValidationError from django.core.validators import MinLengthValidator from django.db import models from django.db.models.signals import post_save @@ -190,3 +192,54 @@ class Token(models.Model): if self.expires is None or timezone.now() < self.expires: return False return True + + +class ObjectPermission(models.Model): + """ + A mapping of view, add, change, and/or delete permission for users and/or groups to an arbitrary set of objects + identified by ORM query parameters. + """ + users = models.ManyToManyField( + to=User, + blank=True, + related_name='object_permissions' + ) + groups = models.ManyToManyField( + to=Group, + blank=True, + related_name='object_permissions' + ) + model = models.ForeignKey( + to=ContentType, + on_delete=models.CASCADE + ) + attrs = JSONField( + verbose_name='Attributes' + ) + can_view = models.BooleanField( + default=False + ) + can_add = models.BooleanField( + default=False + ) + can_change = models.BooleanField( + default=False + ) + can_delete = models.BooleanField( + default=False + ) + + class Meta: + unique_together = ('model', 'attrs') + + def clean(self): + + # Validate the specified model attributes by attempting to execute a query. We don't care whether the query + # returns anything; we just want to make sure the specified attributes are valid. + model = self.model.model_class() + try: + model.objects.filter(**self.attrs).exists() + except FieldError as e: + raise ValidationError({ + 'attrs': f'Invalid attributes for {model}: {e}' + }) diff --git a/netbox/users/tests/test_permissions.py b/netbox/users/tests/test_permissions.py new file mode 100644 index 000000000..f73fd8f43 --- /dev/null +++ b/netbox/users/tests/test_permissions.py @@ -0,0 +1,62 @@ +from django.contrib.contenttypes.models import ContentType +from django.contrib.auth.models import Permission, User +from django.test import TestCase, override_settings + +from dcim.models import Site +from tenancy.models import Tenant +from users.models import ObjectPermission + + +class UserConfigTest(TestCase): + + def setUp(self): + + self.user = User.objects.create_user(username='testuser') + + @classmethod + def setUpTestData(cls): + + tenant = Tenant.objects.create(name='Tenant 1', slug='tenant-1') + Site.objects.bulk_create(( + Site(name='Site 1', slug='site-1'), + Site(name='Site 2', slug='site-2', tenant=tenant), + Site(name='Site 3', slug='site-3'), + )) + + @override_settings(EXEMPT_VIEW_PERMISSIONS=[]) + def test_permission_view_object(self): + + # Sanity check to ensure the user has no model-level permission + self.assertFalse(self.user.has_perm('dcim.view_site')) + + # The permission check for a specific object should fail. + sites = Site.objects.all() + self.assertFalse(self.user.has_perm('dcim.view_site', sites[0])) + + # Create and assign a new ObjectPermission specifying the first site by name. + ct = ContentType.objects.get_for_model(sites[0]) + object_perm = ObjectPermission( + model=ct, + attrs={'name': 'Site 1'}, + can_view=True + ) + object_perm.save() + self.user.object_permissions.add(object_perm) + + # The test user should have permission to view only the first site. + self.assertTrue(self.user.has_perm('dcim.view_site', sites[0])) + self.assertFalse(self.user.has_perm('dcim.view_site', sites[1])) + + # Create a second ObjectPermission matching sites by assigned tenant. + object_perm = ObjectPermission( + model=ct, + attrs={'tenant__name': 'Tenant 1'}, + can_view=True + ) + object_perm.save() + self.user.object_permissions.add(object_perm) + + # The user should now able to view the first two sites, but not the third. + self.assertTrue(self.user.has_perm('dcim.view_site', sites[0])) + self.assertTrue(self.user.has_perm('dcim.view_site', sites[1])) + self.assertFalse(self.user.has_perm('dcim.view_site', sites[2])) diff --git a/netbox/utilities/auth_backends.py b/netbox/utilities/auth_backends.py index 6342bad2b..0d20fe02f 100644 --- a/netbox/utilities/auth_backends.py +++ b/netbox/utilities/auth_backends.py @@ -3,6 +3,10 @@ import logging from django.conf import settings from django.contrib.auth.backends import ModelBackend, RemoteUserBackend as RemoteUserBackend_ from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType +from django.db.models import Q + +from users.models import ObjectPermission class ViewExemptModelBackend(ModelBackend): @@ -31,6 +35,44 @@ class ViewExemptModelBackend(ModelBackend): return super().has_perm(user_obj, perm, obj) +class ObjectPermissionBackend(ModelBackend): + """ + Evaluates permission of a user to access or modify a specific object based on the assignment of ObjectPermissions + either to the user directly or to a group of which the user is a member. Model-level permissions supersede this + check: For example, if a user has the dcim.view_site model-level permission assigned, the ViewExemptModelBackend + will grant permission before this backend is evaluated for permission to view a specific site. + """ + def has_perm(self, user_obj, perm, obj=None): + + # This backend only checks for permissions on specific objects + if obj is None: + return + + app, codename = perm.split('.') + action, model_name = codename.split('_') + model = obj._meta.model + + # Check that the requested permission applies to the specified object + if model._meta.model_name != model_name: + raise ValueError(f"Invalid permission {perm} for model {model}") + + # Retrieve user's permissions for this model + # This can probably be cached + obj_permissions = ObjectPermission.objects.filter( + Q(users=user_obj) | Q(groups__user=user_obj), + model=ContentType.objects.get_for_model(obj), + **{f'can_{action}': True} + ) + + for perm in obj_permissions: + + # Attempt to retrieve the model from the database using the + # attributes defined in the ObjectPermission. If we have a + # match, assert that the user has permission. + if model.objects.filter(pk=obj.pk, **perm.attrs).exists(): + return True + + class RemoteUserBackend(ViewExemptModelBackend, RemoteUserBackend_): """ Custom implementation of Django's RemoteUserBackend which provides configuration hooks for basic customization.