Преглед изворни кода

Fixes #22307: Ensure consistent treatment of grant_token in web UI

Jeremy Stretch пре 9 часа
родитељ
комит
54199977eb

+ 8 - 0
netbox/users/forms/bulk_import.py

@@ -54,6 +54,14 @@ class TokenImportForm(CSVModelForm):
         model = Token
         fields = ('user', 'version', 'token', 'enabled', 'write_enabled', 'expires', 'description',)
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # Disable the user field when updating an existing Token, so a Token's owner cannot be reassigned via
+        # bulk import (consistent with the REST API & UI edit form).
+        if self.instance.pk:
+            self.fields['user'].disabled = True
+
 
 class OwnerGroupImportForm(CSVModelForm):
 

+ 17 - 0
netbox/users/forms/model_forms.py

@@ -185,6 +185,23 @@ class TokenForm(UserTokenForm):
         if self.instance and not self.instance._state.adding:
             self.fields['user'].disabled = True
 
+    def clean(self):
+        super().clean()
+
+        # Creating a Token on behalf of another user requires the grant_token permission. This is enforced only on
+        # creation; the user field is disabled when editing an existing Token.
+        if self.instance._state.adding and (token_user := self.cleaned_data.get('user')):
+            request = getattr(self.instance, '_request', None)
+            if request is None:
+                # Fail closed: we cannot verify that the acting user is authorized.
+                raise forms.ValidationError(_("Unable to verify permission to create tokens."))
+            if token_user != request.user and not request.user.has_perm('users.grant_token'):
+                raise forms.ValidationError(
+                    _("This user does not have permission to create tokens for other users.")
+                )
+
+        return self.cleaned_data
+
 
 class UserForm(forms.ModelForm):
     password = forms.CharField(

+ 133 - 1
netbox/users/tests/test_views.py

@@ -1,6 +1,7 @@
 from django.urls import reverse
 
 from core.models import ObjectType
+from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
 from users.constants import TOKEN_PREFIX
 from users.models import *
 from utilities.testing import TestCase, ViewTestCases, create_test_user
@@ -219,6 +220,8 @@ class TokenTestCase(
     model = Token
     maxDiff = None
     validation_excluded_fields = ['token', 'user']
+    # Tokens in form_data/csv_data are created for other users, which requires the grant_token permission
+    user_permissions = ('users.grant_token',)
 
     @classmethod
     def setUpTestData(cls):
@@ -265,7 +268,7 @@ class TokenOneTimeAuthStringTestCase(TestCase):
     Verify that the plaintext value of a newly-created Token is surfaced exactly once via the detail view, and
     that it is never persisted in the database.
     """
-    user_permissions = ('users.add_token', 'users.view_token', 'users.view_user')
+    user_permissions = ('users.add_token', 'users.view_token', 'users.view_user', 'users.grant_token')
 
     def test_create_stashes_plaintext_and_detail_view_renders_it_once(self):
         target_user = create_test_user('token_owner')
@@ -332,6 +335,135 @@ class TokenOneTimeAuthStringTestCase(TestCase):
         self.assertTrue(token.validate(plaintext))
 
 
+class TokenGrantPermissionTestCase(TestCase):
+    """
+    Verify that creating a Token for another user via the UI requires the grant_token permission, consistent
+    with REST API enforcement.
+    """
+    user_permissions = ('users.add_token', 'users.change_token', 'users.view_token', 'users.view_user')
+
+    def test_create_token_for_self_without_grant(self):
+        # Creating a Token for oneself should not require the grant_token permission.
+        response = self.client.post(reverse('users:token_add'), data={
+            'version': 2,
+            'user': self.user.pk,
+            'description': 'self token',
+            'enabled': 'on',
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertTrue(Token.objects.filter(description='self token', user=self.user).exists())
+
+    def test_create_token_for_other_user_without_grant(self):
+        # Creating a Token for another user without the grant_token permission should be prohibited.
+        other_user = create_test_user('other_user')
+        response = self.client.post(reverse('users:token_add'), data={
+            'version': 2,
+            'user': other_user.pk,
+            'description': 'other token',
+            'enabled': 'on',
+        })
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(Token.objects.filter(user=other_user).exists())
+
+    def test_create_token_for_other_user_with_grant(self):
+        # Creating a Token for another user with the grant_token permission should succeed.
+        self.add_permissions('users.grant_token')
+        other_user = create_test_user('other_user')
+        response = self.client.post(reverse('users:token_add'), data={
+            'version': 2,
+            'user': other_user.pk,
+            'description': 'other token',
+            'enabled': 'on',
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertTrue(Token.objects.filter(description='other token', user=other_user).exists())
+
+    def test_edit_token_for_other_user_without_grant(self):
+        # Editing an existing Token owned by another user should not require the grant_token permission, since
+        # the user field is read-only on edits (consistent with the REST API).
+        other_user = create_test_user('other_user')
+        token = Token.objects.create(user=other_user, description='original')
+        response = self.client.post(reverse('users:token_edit', kwargs={'pk': token.pk}), data={
+            'version': token.version,
+            'user': other_user.pk,
+            'description': 'updated',
+            'enabled': 'on',
+        })
+        self.assertEqual(response.status_code, 302)
+        token.refresh_from_db()
+        self.assertEqual(token.description, 'updated')
+
+    def test_bulk_import_token_for_other_user_without_grant(self):
+        # Bulk-importing a Token for another user without the grant_token permission should be prohibited.
+        other_user = create_test_user('other_user')
+        csv_data = '\n'.join((
+            "user,description",
+            f"{other_user.pk},imported token",
+        ))
+        response = self.client.post(reverse('users:token_bulk_import'), data={
+            'data': csv_data,
+            'format': ImportFormatChoices.CSV,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(Token.objects.filter(user=other_user).exists())
+
+    def test_bulk_import_token_for_other_user_with_grant(self):
+        # Bulk-importing a Token for another user with the grant_token permission should succeed.
+        self.add_permissions('users.grant_token')
+        other_user = create_test_user('other_user')
+        csv_data = '\n'.join((
+            "user,description",
+            f"{other_user.pk},imported token",
+        ))
+        response = self.client.post(reverse('users:token_bulk_import'), data={
+            'data': csv_data,
+            'format': ImportFormatChoices.CSV,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertTrue(Token.objects.filter(description='imported token', user=other_user).exists())
+
+    def test_bulk_update_token_for_other_user_without_grant(self):
+        # Bulk-updating an existing Token owned by another user should not require the grant_token permission,
+        # even when the CSV includes the (unchanged) user column. Consistent with the UI edit path.
+        other_user = create_test_user('other_user')
+        token = Token.objects.create(user=other_user, description='original')
+        csv_data = '\n'.join((
+            "id,user,description",
+            f"{token.pk},{other_user.pk},updated",
+        ))
+        response = self.client.post(reverse('users:token_bulk_import'), data={
+            'data': csv_data,
+            'format': ImportFormatChoices.CSV,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+        self.assertEqual(response.status_code, 302)
+        token.refresh_from_db()
+        self.assertEqual(token.description, 'updated')
+
+    def test_bulk_update_cannot_reassign_token_owner(self):
+        # A bulk update must not be able to reassign a Token's owner, regardless of the grant_token permission
+        # (the user field is read-only on update, consistent with the REST API).
+        self.add_permissions('users.grant_token')
+        owner = create_test_user('owner')
+        new_owner = create_test_user('new_owner')
+        token = Token.objects.create(user=owner, description='original')
+        csv_data = '\n'.join((
+            "id,user,description",
+            f"{token.pk},{new_owner.pk},updated",
+        ))
+        response = self.client.post(reverse('users:token_bulk_import'), data={
+            'data': csv_data,
+            'format': ImportFormatChoices.CSV,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+        self.assertEqual(response.status_code, 302)
+        token.refresh_from_db()
+        self.assertEqual(token.user, owner)
+        self.assertEqual(token.description, 'updated')
+
+
 class OwnerGroupTestCase(ViewTestCases.AdminModelViewTestCase):
     model = OwnerGroup
 

+ 10 - 0
netbox/users/views.py

@@ -1,3 +1,4 @@
+from django.core.exceptions import ValidationError
 from django.db.models import Count
 from django.utils.translation import gettext_lazy as _
 
@@ -80,6 +81,15 @@ class TokenBulkImportView(generic.BulkImportView):
     queryset = Token.objects.all()
     model_form = forms.TokenImportForm
 
+    def save_object(self, object_form, request):
+        # Creating a Token on behalf of another user requires the grant_token permission. This is enforced only on
+        # creation; TokenImportForm disables the user field on update, so an existing Token's owner cannot change.
+        token_user = object_form.cleaned_data.get('user')
+        if object_form.instance._state.adding and token_user and token_user != request.user \
+                and not request.user.has_perm('users.grant_token'):
+            raise ValidationError(_("This user does not have permission to create tokens for other users."))
+        return super().save_object(object_form, request)
+
 
 @register_model_view(Token, 'bulk_edit', path='edit', detail=False)
 class TokenBulkEditView(generic.BulkEditView):