Skip to content

Commit

Permalink
[fix] Fixed authorization bug in get_template_default_values view
Browse files Browse the repository at this point in the history
  • Loading branch information
pandafy authored and nemesifier committed Apr 8, 2021
1 parent 7e8ee35 commit 7fa17d6
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 6 deletions.
2 changes: 1 addition & 1 deletion openwisp_controller/config/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ def get_urls(self):
),
url(
r'^get-template-default-values/$',
get_template_default_values,
self.admin_site.admin_view(get_template_default_values),
name='get_template_default_values',
),
] + super().get_urls()
Expand Down
4 changes: 2 additions & 2 deletions openwisp_controller/config/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -986,15 +986,15 @@ def test_get_template_default_values(self):
path = reverse('admin:get_template_default_values')

with self.subTest('get default values for one template'):
with self.assertNumQueries(1):
with self.assertNumQueries(3):
r = self.client.get(path, {'pks': f'{t1.pk}'})
self.assertEqual(r.status_code, 200)
expected = {'default_values': {'name1': 'test1'}}
self.assertEqual(r.json(), expected)

with self.subTest('get default values for multiple templates'):
t2 = self._create_template(name='t2', default_values={'name2': 'test2'})
with self.assertNumQueries(1):
with self.assertNumQueries(3):
r = self.client.get(path, {'pks': f'{t1.pk},{t2.pk}'})
self.assertEqual(r.status_code, 200)
expected = {'default_values': {'name1': 'test1', 'name2': 'test2'}}
Expand Down
54 changes: 54 additions & 0 deletions openwisp_controller/config/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,57 @@ def test_get_default_templates_400(self):
reverse('admin:get_default_templates', args=['wrong'])
)
self.assertEqual(response.status_code, 404)

def get_template_default_values_authorization(self):
org1 = self._get_org()
org1_template = self._create_template(
organization=org1, default_values={'org1': 'secret1'}
)
org2 = self._create_org(name='org2')
org2_template = self._create_template(
organization=org2, default_values={'org2': 'secret2'}
)
shared_template = self._create_template(
name='shared-template', default_values={'key': 'value'}
)
url = (
reverse('admin:get_template_default_values')
+ f'?pks={org1_template.pk},{org2_template.pk},{shared_template.pk}'
)

with self.subTest('Unauthenticated user'):
# Unauthenticated users will be redirected to login page
response = self.client.get(url)
self.assertEqual(response.status_code, 302)

with self.subTest('Authenticated non-staff user'):
# Non-staff users will be redirected to login page of admin
# and will be asked to login with a staff account
user = self._create_user()
self.client.force_login(user)
response = self.client.get(url)
self.assertEqual(response.status_code, 302)
response = self.client.get(
reverse('admin:get_default_templates', args=[org1.pk]), follow=True
)
self.assertContains(response, 'not authorized')

with self.subTest('Org admin requests data of other organization'):
org1_admin = self._create_org_user(organization=org1, is_admin=True)
org1_user = org1_admin.user
org1_user.is_staff = True
org1_user.save()
self.client.force_login(org1_user)
expected_response = {'default_values': {'org1': 'secret1', 'key': 'value'}}
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertJSONEqual(response.content, expected_response)

with self.subTest('Superuser requests data for any organization'):
self._login()
response = self.client.get(url)
expected_response = {
'default_values': {'org1': 'secret1', 'org2': 'secret2', 'key': 'value'}
}
self.assertEqual(response.status_code, 200)
self.assertJSONEqual(response.content, expected_response)
12 changes: 9 additions & 3 deletions openwisp_controller/config/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from copy import deepcopy
from uuid import UUID

from django.db.models import Q
from django.http import HttpResponse, JsonResponse
from django.utils import timezone
from django.utils.module_loading import import_string
Expand Down Expand Up @@ -81,6 +82,7 @@ def get_template_default_values(request):
"""
returns default_values for one or more templates
"""
user = request.user
pk_list = []
for pk in request.GET.get('pks', '').split(','):
try:
Expand All @@ -91,9 +93,13 @@ def get_template_default_values(request):
)
else:
pk_list.append(pk)
values = Template.objects.filter(pk__in=pk_list).values_list(
'default_values', flat=True
)
where = Q(organization=None)
if not user.is_superuser:
where |= Q(organization__in=user.organizations_managed)
if user.is_superuser:
where = Q()
where &= Q(pk__in=pk_list)
values = Template.objects.filter(where).values_list('default_values', flat=True)
default_values = {}
for item in values:
default_values.update(item)
Expand Down

0 comments on commit 7fa17d6

Please sign in to comment.