From e6bc55af858c061616eb2ff610e374e43bf03639 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 7 Aug 2020 16:19:18 -0400 Subject: [PATCH] #4969: Remove user and group assignment from SecretRole --- docs/models/secrets/secretrole.md | 2 - netbox/secrets/api/views.py | 10 ++-- netbox/secrets/forms.py | 8 +-- .../0009_secretrole_drop_users_groups.py | 20 +++++++ netbox/secrets/models.py | 27 ---------- netbox/secrets/tables.py | 2 +- netbox/secrets/templatetags/__init__.py | 0 netbox/secrets/templatetags/secret_helpers.py | 12 ----- netbox/secrets/tests/test_views.py | 2 - netbox/templates/secrets/inc/secret_tr.html | 25 ++++----- netbox/templates/secrets/secret.html | 52 ++++++++----------- netbox/templates/secrets/secret_edit.html | 3 +- .../migrations/0009_replicate_permissions.py | 47 ++++++++++++----- 13 files changed, 93 insertions(+), 117 deletions(-) create mode 100644 netbox/secrets/migrations/0009_secretrole_drop_users_groups.py delete mode 100644 netbox/secrets/templatetags/__init__.py delete mode 100644 netbox/secrets/templatetags/secret_helpers.py diff --git a/docs/models/secrets/secretrole.md b/docs/models/secrets/secretrole.md index 8997ed52a..23f68912b 100644 --- a/docs/models/secrets/secretrole.md +++ b/docs/models/secrets/secretrole.md @@ -7,5 +7,3 @@ Each secret is assigned a functional role which indicates what it is used for. S * RADIUS/TACACS+ keys * IKE key strings * Routing protocol shared secrets - -Roles are also used to control access to secrets. Each role is assigned an arbitrary number of groups and/or users. Only the users associated with a role have permission to decrypt the secrets assigned to that role. (A superuser has permission to decrypt all secrets, provided they have an active user key.) diff --git a/netbox/secrets/api/views.py b/netbox/secrets/api/views.py index 3ad87a8ff..4c88e30ea 100644 --- a/netbox/secrets/api/views.py +++ b/netbox/secrets/api/views.py @@ -38,7 +38,7 @@ class SecretRoleViewSet(ModelViewSet): class SecretViewSet(ModelViewSet): queryset = Secret.objects.prefetch_related( - 'device__primary_ip4', 'device__primary_ip6', 'role', 'role__users', 'role__groups', 'tags', + 'device__primary_ip4', 'device__primary_ip6', 'role', 'tags', ) serializer_class = serializers.SecretSerializer filterset_class = filters.SecretFilterSet @@ -84,8 +84,8 @@ class SecretViewSet(ModelViewSet): secret = self.get_object() - # Attempt to decrypt the secret if the user is permitted and the master key is known - if secret.decryptable_by(request.user) and self.master_key is not None: + # Attempt to decrypt the secret if the master key is known + if self.master_key is not None: secret.decrypt(self.master_key) serializer = self.get_serializer(secret) @@ -102,9 +102,7 @@ class SecretViewSet(ModelViewSet): if self.master_key is not None: secrets = [] for secret in page: - # Enforce role permissions - if secret.decryptable_by(request.user): - secret.decrypt(self.master_key) + secret.decrypt(self.master_key) secrets.append(secret) serializer = self.get_serializer(secrets, many=True) else: diff --git a/netbox/secrets/forms.py b/netbox/secrets/forms.py index e59819246..54178f46b 100644 --- a/netbox/secrets/forms.py +++ b/netbox/secrets/forms.py @@ -46,13 +46,7 @@ class SecretRoleForm(BootstrapMixin, forms.ModelForm): class Meta: model = SecretRole - fields = [ - 'name', 'slug', 'description', 'users', 'groups', - ] - widgets = { - 'users': StaticSelect2Multiple(), - 'groups': StaticSelect2Multiple(), - } + fields = ('name', 'slug', 'description') class SecretRoleCSVForm(CSVModelForm): diff --git a/netbox/secrets/migrations/0009_secretrole_drop_users_groups.py b/netbox/secrets/migrations/0009_secretrole_drop_users_groups.py new file mode 100644 index 000000000..e4110b505 --- /dev/null +++ b/netbox/secrets/migrations/0009_secretrole_drop_users_groups.py @@ -0,0 +1,20 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('secrets', '0008_standardize_description'), + ('users', '0009_replicate_permissions'), + ] + + operations = [ + migrations.RemoveField( + model_name='secretrole', + name='groups', + ), + migrations.RemoveField( + model_name='secretrole', + name='users', + ), + ] diff --git a/netbox/secrets/models.py b/netbox/secrets/models.py index e5e53399c..6209b5700 100644 --- a/netbox/secrets/models.py +++ b/netbox/secrets/models.py @@ -239,9 +239,6 @@ class SecretRole(ChangeLoggedModel): """ A SecretRole represents an arbitrary functional classification of Secrets. For example, a user might define roles such as "Login Credentials" or "SNMP Communities." - - By default, only superusers will have access to decrypt Secrets. To allow other users to decrypt Secrets, grant them - access to the appropriate SecretRoles either individually or by group. """ name = models.CharField( max_length=50, @@ -254,16 +251,6 @@ class SecretRole(ChangeLoggedModel): max_length=200, blank=True, ) - users = models.ManyToManyField( - to=User, - related_name='secretroles', - blank=True - ) - groups = models.ManyToManyField( - to=Group, - related_name='secretroles', - blank=True - ) objects = RestrictedQuerySet.as_manager() @@ -285,14 +272,6 @@ class SecretRole(ChangeLoggedModel): self.description, ) - def has_member(self, user): - """ - Check whether the given user has belongs to this SecretRole. Note that superusers belong to all roles. - """ - if user.is_superuser: - return True - return user in self.users.all() or user.groups.filter(pk__in=self.groups.all()).exists() - @extras_features('custom_fields', 'custom_links', 'export_templates', 'webhooks') class Secret(ChangeLoggedModel, CustomFieldModel): @@ -453,9 +432,3 @@ class Secret(ChangeLoggedModel, CustomFieldModel): if not self.hash: raise Exception("Hash has not been generated for this secret.") return check_password(plaintext, self.hash, preferred=SecretValidationHasher()) - - def decryptable_by(self, user): - """ - Check whether the given user has permission to decrypt this Secret. - """ - return self.role.has_member(user) diff --git a/netbox/secrets/tables.py b/netbox/secrets/tables.py index f773a278f..5e8c5a8b4 100644 --- a/netbox/secrets/tables.py +++ b/netbox/secrets/tables.py @@ -18,7 +18,7 @@ class SecretRoleTable(BaseTable): class Meta(BaseTable.Meta): model = SecretRole - fields = ('pk', 'name', 'secret_count', 'description', 'slug', 'users', 'groups', 'actions') + fields = ('pk', 'name', 'secret_count', 'description', 'slug', 'actions') default_columns = ('pk', 'name', 'secret_count', 'description', 'actions') diff --git a/netbox/secrets/templatetags/__init__.py b/netbox/secrets/templatetags/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/netbox/secrets/templatetags/secret_helpers.py b/netbox/secrets/templatetags/secret_helpers.py deleted file mode 100644 index 142c0d2cb..000000000 --- a/netbox/secrets/templatetags/secret_helpers.py +++ /dev/null @@ -1,12 +0,0 @@ -from django import template - - -register = template.Library() - - -@register.filter() -def decryptable_by(secret, user): - """ - Determine whether a given User is permitted to decrypt a Secret. - """ - return secret.decryptable_by(user) diff --git a/netbox/secrets/tests/test_views.py b/netbox/secrets/tests/test_views.py index ea8e6ed48..3b7519b7b 100644 --- a/netbox/secrets/tests/test_views.py +++ b/netbox/secrets/tests/test_views.py @@ -25,8 +25,6 @@ class SecretRoleTestCase(ViewTestCases.OrganizationalObjectViewTestCase): 'name': 'Secret Role X', 'slug': 'secret-role-x', 'description': 'A secret role', - 'users': [], - 'groups': [], } cls.csv_data = ( diff --git a/netbox/templates/secrets/inc/secret_tr.html b/netbox/templates/secrets/inc/secret_tr.html index 3c60928ae..2af609289 100644 --- a/netbox/templates/secrets/inc/secret_tr.html +++ b/netbox/templates/secrets/inc/secret_tr.html @@ -1,23 +1,16 @@ -{% load secret_helpers %} {{ secret.role }} {{ secret.name }} ******** - {% if secret|decryptable_by:request.user %} - - - - {% else %} - - {% endif %} + + + diff --git a/netbox/templates/secrets/secret.html b/netbox/templates/secrets/secret.html index 3ddb2fe98..841d9843a 100644 --- a/netbox/templates/secrets/secret.html +++ b/netbox/templates/secrets/secret.html @@ -2,7 +2,6 @@ {% load buttons %} {% load custom_links %} {% load helpers %} -{% load secret_helpers %} {% load static %} {% load plugins %} @@ -70,38 +69,31 @@ {% plugin_left_page secret %}
- {% if secret|decryptable_by:request.user %} -
-
- Secret Data -
-
-
- {% csrf_token %} -
-
-
Secret
-
********
-
- - - -
+
+
+ Secret Data +
+
+
+ {% csrf_token %} +
+
+
Secret
+
********
+
+ + +
- {% else %} -
- - You do not have permission to decrypt this secret. -
- {% endif %} +
{% include 'extras/inc/tags_panel.html' with tags=secret.tags.all url='secrets:secret_list' %} {% plugin_right_page secret %}
diff --git a/netbox/templates/secrets/secret_edit.html b/netbox/templates/secrets/secret_edit.html index 6893e2d14..0cb1eefef 100644 --- a/netbox/templates/secrets/secret_edit.html +++ b/netbox/templates/secrets/secret_edit.html @@ -1,7 +1,6 @@ {% extends 'base.html' %} {% load static %} {% load form_helpers %} -{% load secret_helpers %} {% block content %}
@@ -30,7 +29,7 @@
Secret Data
- {% if obj.pk and obj|decryptable_by:request.user %} + {% if obj.pk %}
diff --git a/netbox/users/migrations/0009_replicate_permissions.py b/netbox/users/migrations/0009_replicate_permissions.py index 4583da302..eacb63964 100644 --- a/netbox/users/migrations/0009_replicate_permissions.py +++ b/netbox/users/migrations/0009_replicate_permissions.py @@ -1,5 +1,5 @@ from django.db import migrations - +from django.db.models import Q ACTIONS = ['view', 'add', 'change', 'delete'] @@ -10,6 +10,7 @@ def replicate_permissions(apps, schema_editor): """ Permission = apps.get_model('auth', 'Permission') ObjectPermission = apps.get_model('users', 'ObjectPermission') + SecretRole = apps.get_model('secrets', 'SecretRole') # TODO: Optimize this iteration so that ObjectPermissions with identical sets of users and groups # are combined into a single ObjectPermission instance. @@ -24,17 +25,39 @@ def replicate_permissions(apps, schema_editor): action = perm.codename if perm.group_set.exists() or perm.user_set.exists(): - obj_perm = ObjectPermission( - # Copy name from original Permission object - name=f'{perm.content_type.app_label}.{perm.codename}'[:100], - actions=[action] - ) - obj_perm.save() - obj_perm.object_types.add(perm.content_type) - if perm.group_set.exists(): - obj_perm.groups.add(*list(perm.group_set.all())) - if perm.user_set.exists(): - obj_perm.users.add(*list(perm.user_set.all())) + + # Handle replication of SecretRole user/group assignments for Secrets + if perm.codename == 'view_secret': + for secretrole in SecretRole.objects.prefetch_related('users', 'groups'): + obj_perm = ObjectPermission( + name=f'{perm.content_type.app_label}.{perm.codename} ({secretrole.name})'[:100], + actions=[action], + constraints={'role__name': secretrole.name} + ) + obj_perm.save() + obj_perm.object_types.add(perm.content_type) + # Assign only users/groups who both a) are assigned to the SecretRole and b) have the view_secret + # permission + obj_perm.groups.add( + *list(secretrole.groups.filter(permissions=perm)) + ) + obj_perm.users.add(*list(secretrole.users.filter( + Q(user_permissions=perm) | Q(groups__permissions=perm) + ))) + + else: + obj_perm = ObjectPermission( + # Copy name from original Permission object + name=f'{perm.content_type.app_label}.{perm.codename}'[:100], + actions=[action] + ) + obj_perm.save() + obj_perm.object_types.add(perm.content_type) + + if perm.group_set.exists(): + obj_perm.groups.add(*list(perm.group_set.all())) + if perm.user_set.exists(): + obj_perm.users.add(*list(perm.user_set.all())) class Migration(migrations.Migration):