This commit is contained in:
parent
99b3e7873b
commit
ae8bc6e6a2
|
|
@ -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):
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue