From 7cf0e6034b45224cff78f70926e9e7d27227f6d3 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 16 Jun 2020 15:12:50 -0400 Subject: [PATCH 1/5] Optimize tag population under prepare_cloned_fields() --- netbox/utilities/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 351b1fd68..4c07f5520 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -213,9 +213,9 @@ def prepare_cloned_fields(instance): if field_value not in (None, ''): params[field_name] = field_value - # Copy tags - if is_taggable(instance): - params['tags'] = ','.join([t.name for t in instance.tags.all()]) + # Copy tags + if is_taggable(instance): + params['tags'] = ','.join([t.name for t in instance.tags.all()]) # Concatenate parameters into a URL query string param_string = '&'.join( From e23a5ad141bd91aaf1c2034b109b28611ea4f1f5 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 17 Jun 2020 09:15:03 -0400 Subject: [PATCH 2/5] Fixes #4766: Fix redirect after login when next is not specified --- docs/release-notes/version-2.8.md | 8 ++++++++ netbox/users/views.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/version-2.8.md b/docs/release-notes/version-2.8.md index 7bddcbd40..03a9a4f1b 100644 --- a/docs/release-notes/version-2.8.md +++ b/docs/release-notes/version-2.8.md @@ -1,5 +1,13 @@ # NetBox v2.8 +## v2.8.7 (FUTURE) + +### Bug Fixes + +* [#4766](https://github.com/netbox-community/netbox/issues/4766) - Fix redirect after login when `next` is not specified + +--- + ## v2.8.6 (2020-06-15) ### Enhancements diff --git a/netbox/users/views.py b/netbox/users/views.py index c3e366542..9053d7b70 100644 --- a/netbox/users/views.py +++ b/netbox/users/views.py @@ -50,7 +50,7 @@ class LoginView(View): logger.debug("Login form validation was successful") # Determine where to direct user after successful login - redirect_to = request.POST.get('next') + redirect_to = request.POST.get('next', reverse('home')) if redirect_to and not is_safe_url(url=redirect_to, allowed_hosts=request.get_host()): logger.warning(f"Ignoring unsafe 'next' URL passed to login form: {redirect_to}") redirect_to = reverse('home') From b0c24de5967d04e82f1691f0f33799f6f11148b4 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 17 Jun 2020 14:22:55 -0400 Subject: [PATCH 3/5] Fixes #4772: Fix "brief" format for the secrets REST API endpoint --- docs/release-notes/version-2.8.md | 1 + netbox/secrets/api/nested_serializers.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/version-2.8.md b/docs/release-notes/version-2.8.md index 03a9a4f1b..366241390 100644 --- a/docs/release-notes/version-2.8.md +++ b/docs/release-notes/version-2.8.md @@ -5,6 +5,7 @@ ### Bug Fixes * [#4766](https://github.com/netbox-community/netbox/issues/4766) - Fix redirect after login when `next` is not specified +* [#4772](https://github.com/netbox-community/netbox/issues/4772) - Fix "brief" format for the secrets REST API endpoint --- diff --git a/netbox/secrets/api/nested_serializers.py b/netbox/secrets/api/nested_serializers.py index 7aa8087da..13c016c18 100644 --- a/netbox/secrets/api/nested_serializers.py +++ b/netbox/secrets/api/nested_serializers.py @@ -1,13 +1,22 @@ from rest_framework import serializers -from secrets.models import SecretRole +from secrets.models import Secret, SecretRole from utilities.api import WritableNestedSerializer __all__ = [ - 'NestedSecretRoleSerializer' + 'NestedSecretRoleSerializer', + 'NestedSecretSerializer', ] +class NestedSecretSerializer(WritableNestedSerializer): + url = serializers.HyperlinkedIdentityField(view_name='secrets-api:secret-detail') + + class Meta: + model = Secret + fields = ['id', 'url', 'name'] + + class NestedSecretRoleSerializer(WritableNestedSerializer): url = serializers.HyperlinkedIdentityField(view_name='secrets-api:secretrole-detail') secret_count = serializers.IntegerField(read_only=True) From 26770515e13c0981e5fdb970a49946cdfae3173f Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 17 Jun 2020 15:36:56 -0400 Subject: [PATCH 4/5] Refactor TestCase to provide model_to_dict(), prepare_instance() --- netbox/utilities/testing/testcases.py | 77 +++++++++++++++++---------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/netbox/utilities/testing/testcases.py b/netbox/utilities/testing/testcases.py index 4f33c747e..82e88312e 100644 --- a/netbox/utilities/testing/testcases.py +++ b/netbox/utilities/testing/testcases.py @@ -26,6 +26,54 @@ class TestCase(_TestCase): self.client = Client() self.client.force_login(self.user) + def prepare_instance(self, instance): + """ + Test cases can override this method to perform any necessary manipulation of an instance prior to its evaluation + against test data. For example, it can be used to decrypt a Secret's plaintext attribute. + """ + return instance + + def model_to_dict(self, instance, fields, api=False): + """ + Return a dictionary representation of an instance. + """ + # Prepare the instance and call Django's model_to_dict() to extract all fields + model_dict = model_to_dict(self.prepare_instance(instance), fields=fields) + + # Map any additional (non-field) instance attributes that were specified + for attr in fields: + if hasattr(instance, attr) and attr not in model_dict: + model_dict[attr] = getattr(instance, attr) + + for key, value in list(model_dict.items()): + + # TODO: Differentiate between tags assigned to the instance and a M2M field for tags (ex: ConfigContext) + if key == 'tags': + model_dict[key] = ','.join(sorted([tag.name for tag in value])) + + # Convert ManyToManyField to list of instance PKs + elif model_dict[key] and type(value) in (list, tuple) and hasattr(value[0], 'pk'): + model_dict[key] = [obj.pk for obj in value] + + if api: + + # Replace ContentType numeric IDs with . + if type(getattr(instance, key)) is ContentType: + ct = ContentType.objects.get(pk=value) + model_dict[key] = f'{ct.app_label}.{ct.model}' + + # Convert IPNetwork instances to strings + if type(value) is IPNetwork: + model_dict[key] = str(value) + + else: + + # Convert ArrayFields to CSV strings + if type(instance._meta.get_field(key)) is ArrayField: + model_dict[key] = ','.join([str(v) for v in value]) + + return model_dict + # # Permissions management # @@ -70,34 +118,7 @@ class TestCase(_TestCase): :data: Dictionary of test data used to define the instance :api: Set to True is the data is a JSON representation of the instance """ - model_dict = model_to_dict(instance, fields=data.keys()) - - for key, value in list(model_dict.items()): - - # TODO: Differentiate between tags assigned to the instance and a M2M field for tags (ex: ConfigContext) - if key == 'tags': - model_dict[key] = ','.join(sorted([tag.name for tag in value])) - - # Convert ManyToManyField to list of instance PKs - elif model_dict[key] and type(value) in (list, tuple) and hasattr(value[0], 'pk'): - model_dict[key] = [obj.pk for obj in value] - - if api: - - # Replace ContentType numeric IDs with . - if type(getattr(instance, key)) is ContentType: - ct = ContentType.objects.get(pk=value) - model_dict[key] = f'{ct.app_label}.{ct.model}' - - # Convert IPNetwork instances to strings - if type(value) is IPNetwork: - model_dict[key] = str(value) - - else: - - # Convert ArrayFields to CSV strings - if type(instance._meta.get_field(key)) is ArrayField: - model_dict[key] = ','.join([str(v) for v in value]) + model_dict = self.model_to_dict(instance, fields=data.keys(), api=api) # Omit any dictionary keys which are not instance attributes relevant_data = { From 0800279325f984eda149a38afbb0991a8ce704c7 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 17 Jun 2020 15:37:28 -0400 Subject: [PATCH 5/5] Standardize SecretTest --- netbox/secrets/tests/test_api.py | 230 ++++++------------------------- 1 file changed, 43 insertions(+), 187 deletions(-) diff --git a/netbox/secrets/tests/test_api.py b/netbox/secrets/tests/test_api.py index e3078c81f..136912773 100644 --- a/netbox/secrets/tests/test_api.py +++ b/netbox/secrets/tests/test_api.py @@ -49,212 +49,68 @@ class SecretRoleTest(APIViewTestCases.APIViewTestCase): SecretRole.objects.bulk_create(secret_roles) -# TODO: Standardize SecretTest -class SecretTest(APITestCase): +class SecretTest(APIViewTestCases.APIViewTestCase): + model = Secret + brief_fields = ['id', 'name', 'url'] def setUp(self): + super().setUp() - # Create a non-superuser test user - self.user = create_test_user('testuser', permissions=( - 'secrets.add_secret', - 'secrets.change_secret', - 'secrets.delete_secret', - 'secrets.view_secret', - )) - self.token = Token.objects.create(user=self.user) - self.header = {'HTTP_AUTHORIZATION': 'Token {}'.format(self.token.key)} - + # Create a UserKey for the test user userkey = UserKey(user=self.user, public_key=PUBLIC_KEY) userkey.save() + + # Create a SessionKey for the user self.master_key = userkey.get_master_key(PRIVATE_KEY) session_key = SessionKey(userkey=userkey) session_key.save(self.master_key) - self.header = { - 'HTTP_AUTHORIZATION': 'Token {}'.format(self.token.key), - 'HTTP_X_SESSION_KEY': base64.b64encode(session_key.key), - } + # Append the session key to the test client's request header + self.header['HTTP_X_SESSION_KEY'] = base64.b64encode(session_key.key) - self.plaintexts = ( - 'Secret #1 Plaintext', - 'Secret #2 Plaintext', - 'Secret #3 Plaintext', + site = Site.objects.create(name='Site 1', slug='site-1') + manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + devicetype = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type 1') + devicerole = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1') + device = Device.objects.create(name='Device 1', site=site, device_type=devicetype, device_role=devicerole) + + secret_roles = ( + SecretRole(name='Secret Role 1', slug='secret-role-1'), + SecretRole(name='Secret Role 2', slug='secret-role-2'), ) + SecretRole.objects.bulk_create(secret_roles) - site = Site.objects.create(name='Test Site 1', slug='test-site-1') - manufacturer = Manufacturer.objects.create(name='Test Manufacturer 1', slug='test-manufacturer-1') - devicetype = DeviceType.objects.create(manufacturer=manufacturer, model='Test Device Type 1') - devicerole = DeviceRole.objects.create(name='Test Device Role 1', slug='test-device-role-1') - self.device = Device.objects.create( - name='Test Device 1', site=site, device_type=devicetype, device_role=devicerole + secrets = ( + Secret(device=device, role=secret_roles[0], name='Secret 1', plaintext='ABC'), + Secret(device=device, role=secret_roles[0], name='Secret 2', plaintext='DEF'), + Secret(device=device, role=secret_roles[0], name='Secret 3', plaintext='GHI'), ) - self.secretrole1 = SecretRole.objects.create(name='Test Secret Role 1', slug='test-secret-role-1') - self.secretrole2 = SecretRole.objects.create(name='Test Secret Role 2', slug='test-secret-role-2') - self.secret1 = Secret( - device=self.device, role=self.secretrole1, name='Test Secret 1', plaintext=self.plaintexts[0] - ) - self.secret1.encrypt(self.master_key) - self.secret1.save() - self.secret2 = Secret( - device=self.device, role=self.secretrole1, name='Test Secret 2', plaintext=self.plaintexts[1] - ) - self.secret2.encrypt(self.master_key) - self.secret2.save() - self.secret3 = Secret( - device=self.device, role=self.secretrole1, name='Test Secret 3', plaintext=self.plaintexts[2] - ) - self.secret3.encrypt(self.master_key) - self.secret3.save() + for secret in secrets: + secret.encrypt(self.master_key) + secret.save() - def test_get_secret(self): - - url = reverse('secrets-api:secret-detail', kwargs={'pk': self.secret1.pk}) - - # Secret plaintext not be decrypted as the user has not been assigned to the role - response = self.client.get(url, **self.header) - self.assertIsNone(response.data['plaintext']) - - # The plaintext should be present once the user has been assigned to the role - self.secretrole1.users.add(self.user) - response = self.client.get(url, **self.header) - self.assertEqual(response.data['plaintext'], self.plaintexts[0]) - - def test_list_secrets(self): - - url = reverse('secrets-api:secret-list') - - # Secret plaintext not be decrypted as the user has not been assigned to the role - response = self.client.get(url, **self.header) - self.assertEqual(response.data['count'], 3) - for secret in response.data['results']: - self.assertIsNone(secret['plaintext']) - - # The plaintext should be present once the user has been assigned to the role - self.secretrole1.users.add(self.user) - response = self.client.get(url, **self.header) - self.assertEqual(response.data['count'], 3) - for i, secret in enumerate(response.data['results']): - self.assertEqual(secret['plaintext'], self.plaintexts[i]) - - def test_create_secret(self): - - data = { - 'device': self.device.pk, - 'role': self.secretrole1.pk, - 'name': 'Test Secret 4', - 'plaintext': 'Secret #4 Plaintext', - } - - url = reverse('secrets-api:secret-list') - response = self.client.post(url, data, format='json', **self.header) - - self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(response.data['plaintext'], data['plaintext']) - self.assertEqual(Secret.objects.count(), 4) - secret4 = Secret.objects.get(pk=response.data['id']) - secret4.decrypt(self.master_key) - self.assertEqual(secret4.role_id, data['role']) - self.assertEqual(secret4.plaintext, data['plaintext']) - - def test_create_secret_bulk(self): - - data = [ + self.create_data = [ { - 'device': self.device.pk, - 'role': self.secretrole1.pk, - 'name': 'Test Secret 4', - 'plaintext': 'Secret #4 Plaintext', + 'device': device.pk, + 'role': secret_roles[1].pk, + 'name': 'Secret 4', + 'plaintext': 'JKL', }, { - 'device': self.device.pk, - 'role': self.secretrole1.pk, - 'name': 'Test Secret 5', - 'plaintext': 'Secret #5 Plaintext', + 'device': device.pk, + 'role': secret_roles[1].pk, + 'name': 'Secret 5', + 'plaintext': 'MNO', }, { - 'device': self.device.pk, - 'role': self.secretrole1.pk, - 'name': 'Test Secret 6', - 'plaintext': 'Secret #6 Plaintext', + 'device': device.pk, + 'role': secret_roles[1].pk, + 'name': 'Secret 6', + 'plaintext': 'PQR', }, ] - url = reverse('secrets-api:secret-list') - response = self.client.post(url, data, format='json', **self.header) - - self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(Secret.objects.count(), 6) - self.assertEqual(response.data[0]['plaintext'], data[0]['plaintext']) - self.assertEqual(response.data[1]['plaintext'], data[1]['plaintext']) - self.assertEqual(response.data[2]['plaintext'], data[2]['plaintext']) - - def test_update_secret(self): - - data = { - 'device': self.device.pk, - 'role': self.secretrole2.pk, - 'plaintext': 'NewPlaintext', - } - - url = reverse('secrets-api:secret-detail', kwargs={'pk': self.secret1.pk}) - response = self.client.put(url, data, format='json', **self.header) - - self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(response.data['plaintext'], data['plaintext']) - self.assertEqual(Secret.objects.count(), 3) - secret1 = Secret.objects.get(pk=response.data['id']) - secret1.decrypt(self.master_key) - self.assertEqual(secret1.role_id, data['role']) - self.assertEqual(secret1.plaintext, data['plaintext']) - - def test_delete_secret(self): - - url = reverse('secrets-api:secret-detail', kwargs={'pk': self.secret1.pk}) - response = self.client.delete(url, **self.header) - - self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) - self.assertEqual(Secret.objects.count(), 2) - - -class GetSessionKeyTest(APITestCase): - - def setUp(self): - - super().setUp() - - userkey = UserKey(user=self.user, public_key=PUBLIC_KEY) - userkey.save() - master_key = userkey.get_master_key(PRIVATE_KEY) - self.session_key = SessionKey(userkey=userkey) - self.session_key.save(master_key) - - self.header = { - 'HTTP_AUTHORIZATION': 'Token {}'.format(self.token.key), - } - - def test_get_session_key(self): - - encoded_session_key = base64.b64encode(self.session_key.key).decode() - - url = reverse('secrets-api:get-session-key-list') - data = { - 'private_key': PRIVATE_KEY, - } - response = self.client.post(url, data, **self.header) - - self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertIsNotNone(response.data.get('session_key')) - self.assertNotEqual(response.data.get('session_key'), encoded_session_key) - - def test_get_session_key_preserved(self): - - encoded_session_key = base64.b64encode(self.session_key.key).decode() - - url = reverse('secrets-api:get-session-key-list') + '?preserve_key=True' - data = { - 'private_key': PRIVATE_KEY, - } - response = self.client.post(url, data, **self.header) - - self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(response.data.get('session_key'), encoded_session_key) + def prepare_instance(self, instance): + # Unlock the plaintext prior to evaluation of the instance + instance.decrypt(self.master_key) + return instance