From ae8bc6e6a203086670c0d14927a482d06d8bf7da Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 28 May 2026 09:07:58 -0400 Subject: [PATCH] Fixes #22307: Ensure consistent treatment of grant_token in web UI (#22308) --- netbox/users/forms/bulk_import.py | 8 ++ netbox/users/forms/model_forms.py | 17 ++++ netbox/users/tests/test_views.py | 134 +++++++++++++++++++++++++++++- netbox/users/views.py | 10 +++ 4 files changed, 168 insertions(+), 1 deletion(-) diff --git a/netbox/users/forms/bulk_import.py b/netbox/users/forms/bulk_import.py index baabe5441..776fb6932 100644 --- a/netbox/users/forms/bulk_import.py +++ b/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): diff --git a/netbox/users/forms/model_forms.py b/netbox/users/forms/model_forms.py index abf492ae4..b86eb7234 100644 --- a/netbox/users/forms/model_forms.py +++ b/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( diff --git a/netbox/users/tests/test_views.py b/netbox/users/tests/test_views.py index c0930448d..97a91ab79 100644 --- a/netbox/users/tests/test_views.py +++ b/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 diff --git a/netbox/users/views.py b/netbox/users/views.py index c5b6117a4..6c0f963a7 100644 --- a/netbox/users/views.py +++ b/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):