From 3d2e02a16e38894282ebf2f5f79eab405a61c784 Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Mon, 12 Aug 2019 15:52:31 +0100
Subject: [PATCH] Prevent users becoming someone they shouldn't be
This was previously only enforced by gating the frontend component, this PR
tidies things up by making sure the backend prevent it happening as well
---
opentech/apply/users/tests/factories.py | 5 ++-
opentech/apply/users/tests/test_views.py | 51 +++++++++++++++++++++++-
opentech/apply/users/views.py | 10 ++++-
3 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/opentech/apply/users/tests/factories.py b/opentech/apply/users/tests/factories.py
index 8db74b40f..ea11d35ec 100644
--- a/opentech/apply/users/tests/factories.py
+++ b/opentech/apply/users/tests/factories.py
@@ -1,5 +1,8 @@
+import uuid
+
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Group
+from django.utils.text import slugify
import factory
@@ -18,7 +21,7 @@ class UserFactory(factory.DjangoModelFactory):
class Meta:
model = get_user_model()
- email = factory.Sequence('email{}@email.com'.format)
+ email = factory.LazyAttribute(lambda o: '{}+{}@email.com'.format(slugify(o.full_name), uuid.uuid4()))
full_name = factory.Faker('name')
password = factory.PostGenerationMethodCall('set_password', 'defaultpassword')
diff --git a/opentech/apply/users/tests/test_views.py b/opentech/apply/users/tests/test_views.py
index 044a9566d..a62923d8a 100644
--- a/opentech/apply/users/tests/test_views.py
+++ b/opentech/apply/users/tests/test_views.py
@@ -3,7 +3,12 @@ from django.test import override_settings, TestCase
from django.urls import reverse
from opentech.apply.utils.testing.tests import BaseViewTestCase
-from .factories import OAuthUserFactory, StaffFactory, UserFactory
+from .factories import (
+ OAuthUserFactory,
+ StaffFactory,
+ SuperUserFactory,
+ UserFactory,
+)
@override_settings(ROOT_URLCONF='opentech.apply.urls')
@@ -58,3 +63,47 @@ class TestPasswordReset(BaseViewTestCase):
self.assertRedirects(response, self.url(None, view_name='password_reset_done'))
self.assertEqual(len(mail.outbox), 1)
self.assertIn('https://testserver/account/password/reset/confirm', mail.outbox[0].body)
+
+
+@override_settings(ROOT_URLCONF='opentech.apply.urls')
+class TestBecome(TestCase):
+ def setUp(self):
+ self.staff = StaffFactory()
+ self.user = UserFactory()
+ self.superuser = SuperUserFactory()
+
+ def become_request(self, user, target):
+ self.client.force_login(user)
+ url = reverse('users:become')
+ response = self.client.post(url, {'user': target.pk}, follow=True, secure=True)
+ return response
+
+ def test_staff_can_become_user(self):
+ response = self.become_request(self.staff, self.user)
+ self.assertEqual(response.status_code, 200)
+
+ def test_staff_cannot_become_superuser(self):
+ response = self.become_request(self.staff, self.superuser)
+ self.assertEqual(response.status_code, 403)
+
+ def test_superuser_can_become_staff(self):
+ response = self.become_request(self.superuser, self.staff)
+ self.assertEqual(response.status_code, 200)
+
+ def test_superuser_cannot_become_superuser(self):
+ other_superuser = SuperUserFactory()
+ response = self.become_request(self.superuser, other_superuser)
+ self.assertEqual(response.status_code, 403)
+
+ def test_user_cannot_become_staff(self):
+ response = self.become_request(self.user, self.staff)
+ self.assertEqual(response.status_code, 403)
+
+ def test_user_cannot_become_other_user(self):
+ other_user = UserFactory()
+ response = self.become_request(self.user, other_user)
+ self.assertEqual(response.status_code, 403)
+
+ def test_user_cannot_become_superuser(self):
+ response = self.become_request(self.user, self.superuser)
+ self.assertEqual(response.status_code, 403)
diff --git a/opentech/apply/users/views.py b/opentech/apply/users/views.py
index 86b54c1bd..fdc63e60a 100644
--- a/opentech/apply/users/views.py
+++ b/opentech/apply/users/views.py
@@ -5,6 +5,7 @@ from django.contrib.auth.decorators import login_required
from django.contrib.auth.forms import AdminPasswordChangeForm
from django.contrib.auth.tokens import PasswordResetTokenGenerator
from django.contrib.auth.views import SuccessURLAllowedHostsMixin
+from django.core.exceptions import PermissionDenied
from django.http import HttpResponseRedirect
from django.shortcuts import redirect, render, resolve_url
from django.template.response import TemplateResponse
@@ -93,8 +94,15 @@ class AccountView(UpdateView):
@login_required()
def become(request):
- id = request.POST['user']
+ if not request.user.is_apply_staff:
+ raise PermissionDenied()
+
+ id = request.POST.get('user')
if request.POST and id:
+ target_user = User.objects.get(pk=id)
+ if target_user.is_superuser:
+ raise PermissionDenied()
+
return login_with_id(request, id)
return redirect('users:account')
--
GitLab