#4969: Remove user and group assignment from SecretRole

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.)

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:

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):

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',
+        ),
+    ]

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)

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')
 
 

netbox/secrets/templatetags/secret_helpers.py

@@ -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)

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 = (

netbox/templates/secrets/inc/secret_tr.html

@@ -1,23 +1,16 @@
-{% load secret_helpers %}
 <tr>
     <td><a href="{% url 'secrets:secret' pk=secret.pk %}">{{ secret.role }}</a></td>
     <td>{{ secret.name }}</td>
     <td id="secret_{{ secret.pk }}">********</td>
     <td class="text-right noprint">
-        {% if secret|decryptable_by:request.user %}
-            <button class="btn btn-xs btn-success unlock-secret" secret-id="{{ secret.pk }}">
-                <i class="fa fa-lock"></i> Unlock
-            </button>
-            <button class="btn btn-xs btn-default copy-secret collapse" secret-id="{{ secret.pk }}" data-clipboard-target="#secret_{{ secret.pk }}">
-                <i class="fa fa-copy"></i> Copy
-            </button>
-            <button class="btn btn-xs btn-danger lock-secret collapse" secret-id="{{ secret.pk }}">
-                <i class="fa fa-unlock-alt"></i> Lock
-            </button>
-        {% else %}
-            <button class="btn btn-xs btn-default" disabled="disabled" title="Permission denied">
-                <i class="fa fa-lock"></i> Unlock
-            </button>
-        {% endif %}
+        <button class="btn btn-xs btn-success unlock-secret" secret-id="{{ secret.pk }}">
+            <i class="fa fa-lock"></i> Unlock
+        </button>
+        <button class="btn btn-xs btn-default copy-secret collapse" secret-id="{{ secret.pk }}" data-clipboard-target="#secret_{{ secret.pk }}">
+            <i class="fa fa-copy"></i> Copy
+        </button>
+        <button class="btn btn-xs btn-danger lock-secret collapse" secret-id="{{ secret.pk }}">
+            <i class="fa fa-unlock-alt"></i> Lock
+        </button>
     </td>
 </tr>

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 %}
 	</div>
 	<div class="col-md-6">
-        {% if secret|decryptable_by:request.user %}
-            <div class="panel panel-default">
-                <div class="panel-heading">
-                    <strong>Secret Data</strong>
-                </div>
-                <div class="panel-body">
-                    <form id="secret_form">
-                        {% csrf_token %}
-                    </form>
-                    <div class="row">
-                        <div class="col-md-2">Secret</div>
-                        <div class="col-md-6" id="secret_{{ secret.pk }}">********</div>
-                        <div class="col-md-4 text-right noprint">
-                            <button class="btn btn-xs btn-success unlock-secret" secret-id="{{ secret.pk }}">
-                                <i class="fa fa-lock"></i> Unlock
-                            </button>
-                            <button class="btn btn-xs btn-default copy-secret collapse" secret-id="{{ secret.pk }}" data-clipboard-target="#secret_{{ secret.pk }}">
-                                <i class="fa fa-copy"></i> Copy
-                            </button>
-                            <button class="btn btn-xs btn-danger lock-secret collapse" secret-id="{{ secret.pk }}">
-                                <i class="fa fa-unlock-alt"></i> Lock
-                            </button>
-                        </div>
+        <div class="panel panel-default">
+            <div class="panel-heading">
+                <strong>Secret Data</strong>
+            </div>
+            <div class="panel-body">
+                <form id="secret_form">
+                    {% csrf_token %}
+                </form>
+                <div class="row">
+                    <div class="col-md-2">Secret</div>
+                    <div class="col-md-6" id="secret_{{ secret.pk }}">********</div>
+                    <div class="col-md-4 text-right noprint">
+                        <button class="btn btn-xs btn-success unlock-secret" secret-id="{{ secret.pk }}">
+                            <i class="fa fa-lock"></i> Unlock
+                        </button>
+                        <button class="btn btn-xs btn-default copy-secret collapse" secret-id="{{ secret.pk }}" data-clipboard-target="#secret_{{ secret.pk }}">
+                            <i class="fa fa-copy"></i> Copy
+                        </button>
+                        <button class="btn btn-xs btn-danger lock-secret collapse" secret-id="{{ secret.pk }}">
+                            <i class="fa fa-unlock-alt"></i> Lock
+                        </button>
                     </div>
                 </div>
             </div>
-        {% else %}
-            <div class="alert alert-warning">
-                <i class="fa fa-warning" aria-hidden="true"></i>
-                You do not have permission to decrypt this secret.
-            </div>
-        {% endif %}
+        </div>
         {% include 'extras/inc/tags_panel.html' with tags=secret.tags.all url='secrets:secret_list' %}
         {% plugin_right_page secret %}
     </div>

netbox/templates/secrets/secret_edit.html

@@ -1,7 +1,6 @@
 {% extends 'base.html' %}
 {% load static %}
 {% load form_helpers %}
-{% load secret_helpers %}
 
 {% block content %}
 <form action="." method="post" class="form form-horizontal">
@@ -30,7 +29,7 @@
             <div class="panel panel-default">
                 <div class="panel-heading"><strong>Secret Data</strong></div>
                 <div class="panel-body">
-                    {% if obj.pk and obj|decryptable_by:request.user %}
+                    {% if obj.pk %}
                         <div class="form-group">
                             <label class="col-md-3 control-label required">Current Plaintext</label>
                             <div class="col-md-7">

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):