From 816fedb78dccf49c311a1bec1926d19f127a7091 Mon Sep 17 00:00:00 2001 From: Arthur Hanson Date: Wed, 2 Nov 2022 09:45:00 -0700 Subject: [PATCH] 8853 Prevent the retrieval of API tokens after creation (#10645) * 8853 hide api token * 8853 hide key on edit * 8853 add key display * 8853 cleanup html * 8853 make token view accessible only once on POST * Clean up display of tokens in views * Honor ALLOW_TOKEN_RETRIEVAL in API serializer * Add docs & tweak default setting * Include token key when provisioning with user credentials Co-authored-by: jeremystretch --- docs/configuration/security.md | 8 ++++ docs/integrations/rest-api.md | 3 ++ docs/release-notes/version-3.4.md | 1 + netbox/netbox/configuration_example.py | 3 ++ netbox/netbox/settings.py | 1 + netbox/templates/users/api_token.html | 60 ++++++++++++++++++++++++++ netbox/users/api/serializers.py | 9 +++- netbox/users/api/views.py | 2 + netbox/users/forms.py | 8 ++++ netbox/users/models.py | 11 ++--- netbox/users/tables.py | 12 +++--- netbox/users/views.py | 10 ++++- 12 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 netbox/templates/users/api_token.html diff --git a/docs/configuration/security.md b/docs/configuration/security.md index 6aa363b1a..b8c2b1e11 100644 --- a/docs/configuration/security.md +++ b/docs/configuration/security.md @@ -1,5 +1,13 @@ # Security & Authentication Parameters +## ALLOW_TOKEN_RETRIEVAL + +Default: True + +If disabled, the values of API tokens will not be displayed after each token's initial creation. A user **must** record the value of a token immediately upon its creation, or it will be lost. Note that this affects _all_ users, regardless of assigned permissions. + +--- + ## ALLOWED_URL_SCHEMES !!! tip "Dynamic Configuration Parameter" diff --git a/docs/integrations/rest-api.md b/docs/integrations/rest-api.md index 3a5aed055..6f54a8cb0 100644 --- a/docs/integrations/rest-api.md +++ b/docs/integrations/rest-api.md @@ -579,6 +579,9 @@ By default, a token can be used to perform all actions via the API that a user w Additionally, a token can be set to expire at a specific time. This can be useful if an external client needs to be granted temporary access to NetBox. +!!! warning "Restricting Token Retrieval" + The ability to retrieve the key value of a previously-created API token can be restricted by disabling the [`ALLOW_TOKEN_RETRIEVAL`](../configuration/security.md#allow_token_retrieval) configuration parameter. + #### Client IP Restriction !!! note diff --git a/docs/release-notes/version-3.4.md b/docs/release-notes/version-3.4.md index 3783cc967..b6e30f2a8 100644 --- a/docs/release-notes/version-3.4.md +++ b/docs/release-notes/version-3.4.md @@ -28,6 +28,7 @@ A new `PluginMenu` class has been introduced, which enables a plugin to inject a * [#8245](https://github.com/netbox-community/netbox/issues/8245) - Enable GraphQL filtering of related objects * [#8274](https://github.com/netbox-community/netbox/issues/8274) - Enable associating a custom link with multiple object types +* [#8853](https://github.com/netbox-community/netbox/issues/8853) - Introduce the `ALLOW_TOKEN_RETRIEVAL` config parameter to restrict the display of API tokens * [#9249](https://github.com/netbox-community/netbox/issues/9249) - Device and virtual machine names are no longer case-sensitive * [#9478](https://github.com/netbox-community/netbox/issues/9478) - Add `link_peers` field to GraphQL types for cabled objects * [#9654](https://github.com/netbox-community/netbox/issues/9654) - Add `weight` field to racks, device types, and module types diff --git a/netbox/netbox/configuration_example.py b/netbox/netbox/configuration_example.py index ad0dcc7c3..b3b6fbb6c 100644 --- a/netbox/netbox/configuration_example.py +++ b/netbox/netbox/configuration_example.py @@ -72,6 +72,9 @@ ADMINS = [ # ('John Doe', 'jdoe@example.com'), ] +# Permit the retrieval of API tokens after their creation. +ALLOW_TOKEN_RETRIEVAL = False + # Enable any desired validators for local account passwords below. For a list of included validators, please see the # Django documentation at https://docs.djangoproject.com/en/stable/topics/auth/passwords/#password-validation. AUTH_PASSWORD_VALIDATORS = [ diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index 2898fbd75..4e93eb149 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -71,6 +71,7 @@ DEPLOYMENT_ID = hashlib.sha256(SECRET_KEY.encode('utf-8')).hexdigest()[:16] # Set static config parameters ADMINS = getattr(configuration, 'ADMINS', []) +ALLOW_TOKEN_RETRIEVAL = getattr(configuration, 'ALLOW_TOKEN_RETRIEVAL', True) AUTH_PASSWORD_VALIDATORS = getattr(configuration, 'AUTH_PASSWORD_VALIDATORS', []) BASE_PATH = getattr(configuration, 'BASE_PATH', '') if BASE_PATH: diff --git a/netbox/templates/users/api_token.html b/netbox/templates/users/api_token.html new file mode 100644 index 000000000..1a9296704 --- /dev/null +++ b/netbox/templates/users/api_token.html @@ -0,0 +1,60 @@ +{% extends 'generic/object.html' %} +{% load form_helpers %} +{% load helpers %} +{% load plugins %} + +{% block content %} +
+
+ {% if not settings.ALLOW_TOKEN_RETRIEVAL %} + + {% endif %} +
+
Token
+
+ + + + + + + + + + + + + + + + + + + + + +
Key +
+ + + +
+
{{ key }}
+
Description{{ object.description|placeholder }}
User{{ object.user }}
Created{{ object.created|annotated_date }}
Expires + {% if object.expires %} + {{ object.expires|annotated_date }} + {% else %} + Never + {% endif %} +
+
+
+ +
+
+{% endblock %} diff --git a/netbox/users/api/serializers.py b/netbox/users/api/serializers.py index 1ec3528f7..f1f1fc975 100644 --- a/netbox/users/api/serializers.py +++ b/netbox/users/api/serializers.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.contrib.auth.models import Group, User from django.contrib.contenttypes.models import ContentType from rest_framework import serializers @@ -63,7 +64,13 @@ class GroupSerializer(ValidatedModelSerializer): class TokenSerializer(ValidatedModelSerializer): url = serializers.HyperlinkedIdentityField(view_name='users-api:token-detail') - key = serializers.CharField(min_length=40, max_length=40, allow_blank=True, required=False) + key = serializers.CharField( + min_length=40, + max_length=40, + allow_blank=True, + required=False, + write_only=not settings.ALLOW_TOKEN_RETRIEVAL + ) user = NestedUserSerializer() allowed_ips = serializers.ListField( child=IPNetworkSerializer(), diff --git a/netbox/users/api/views.py b/netbox/users/api/views.py index 66ef92ab7..86a66a01f 100644 --- a/netbox/users/api/views.py +++ b/netbox/users/api/views.py @@ -88,6 +88,8 @@ class TokenProvisionView(APIView): token = Token(user=user) token.save() data = serializers.TokenSerializer(token, context={'request': request}).data + # Manually append the token key, which is normally write-only + data['key'] = token.key return Response(data, status=HTTP_201_CREATED) diff --git a/netbox/users/forms.py b/netbox/users/forms.py index b4e86461d..048005f13 100644 --- a/netbox/users/forms.py +++ b/netbox/users/forms.py @@ -1,4 +1,5 @@ from django import forms +from django.conf import settings from django.contrib.auth.forms import AuthenticationForm, PasswordChangeForm as DjangoPasswordChangeForm from django.contrib.postgres.forms import SimpleArrayField from django.utils.html import mark_safe @@ -117,3 +118,10 @@ class TokenForm(BootstrapMixin, forms.ModelForm): widgets = { 'expires': DateTimePicker(), } + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Omit the key field if token retrieval is not permitted + if self.instance.pk and not settings.ALLOW_TOKEN_RETRIEVAL: + del self.fields['key'] diff --git a/netbox/users/models.py b/netbox/users/models.py index 4ee4dce6b..441ed2eee 100644 --- a/netbox/users/models.py +++ b/netbox/users/models.py @@ -1,6 +1,7 @@ import binascii import os +from django.conf import settings from django.contrib.auth.models import Group, User from django.contrib.contenttypes.models import ContentType from django.contrib.postgres.fields import ArrayField @@ -230,12 +231,12 @@ class Token(models.Model): 'Ex: "10.1.1.0/24, 192.168.10.16/32, 2001:DB8:1::/64"', ) - class Meta: - pass - def __str__(self): - # Only display the last 24 bits of the token to avoid accidental exposure. - return f"{self.key[-6:]} ({self.user})" + return self.key if settings.ALLOW_TOKEN_RETRIEVAL else self.partial + + @property + def partial(self): + return f'**********************************{self.key[-6:]}' if self.key else '' def save(self, *args, **kwargs): if not self.key: diff --git a/netbox/users/tables.py b/netbox/users/tables.py index 27547b955..8fbe9e8b3 100644 --- a/netbox/users/tables.py +++ b/netbox/users/tables.py @@ -6,14 +6,16 @@ __all__ = ( ) -TOKEN = """{{ value }}""" +TOKEN = """{{ record }}""" ALLOWED_IPS = """{{ value|join:", " }}""" COPY_BUTTON = """ - - - +{% if settings.ALLOW_TOKEN_RETRIEVAL %} + + + +{% endif %} """ @@ -38,5 +40,5 @@ class TokenTable(NetBoxTable): class Meta(NetBoxTable.Meta): model = Token fields = ( - 'pk', 'key', 'write_enabled', 'created', 'expires', 'last_used', 'allowed_ips', 'description', + 'pk', 'description', 'key', 'write_enabled', 'created', 'expires', 'last_used', 'allowed_ips', ) diff --git a/netbox/users/views.py b/netbox/users/views.py index 33ef3fadd..fe1181fc1 100644 --- a/netbox/users/views.py +++ b/netbox/users/views.py @@ -273,6 +273,7 @@ class TokenEditView(LoginRequiredMixin, View): form = TokenForm(request.POST) if form.is_valid(): + token = form.save(commit=False) token.user = request.user token.save() @@ -280,7 +281,13 @@ class TokenEditView(LoginRequiredMixin, View): msg = f"Modified token {token}" if pk else f"Created token {token}" messages.success(request, msg) - if '_addanother' in request.POST: + if not pk and not settings.ALLOW_TOKEN_RETRIEVAL: + return render(request, 'users/api_token.html', { + 'object': token, + 'key': token.key, + 'return_url': reverse('users:token_list'), + }) + elif '_addanother' in request.POST: return redirect(request.path) else: return redirect('users:token_list') @@ -289,6 +296,7 @@ class TokenEditView(LoginRequiredMixin, View): 'object': token, 'form': form, 'return_url': reverse('users:token_list'), + 'disable_addanother': not settings.ALLOW_TOKEN_RETRIEVAL })