diff --git a/openedx/core/djangoapps/user_api/verification_api/tests/test_views.py b/openedx/core/djangoapps/user_api/verification_api/tests/test_views.py index 34ff38e999..a9c49a99fb 100644 --- a/openedx/core/djangoapps/user_api/verification_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/verification_api/tests/test_views.py @@ -26,8 +26,8 @@ class PhotoVerificationStatusViewTests(TestCase): def setUp(self): super(PhotoVerificationStatusViewTests, self).setUp() - self.user = UserFactory.create(password=self.PASSWORD) - self.staff = UserFactory.create(is_staff=True, password=self.PASSWORD) + self.user = UserFactory(password=self.PASSWORD) + self.staff = UserFactory(is_staff=True, password=self.PASSWORD) self.verification = SoftwareSecurePhotoVerification.objects.create(user=self.user, status='submitted') self.path = reverse('verification_status', kwargs={'username': self.user.username}) self.client.login(username=self.staff.username, password=self.PASSWORD) @@ -57,7 +57,7 @@ class PhotoVerificationStatusViewTests(TestCase): def test_no_verifications(self): """ The endpoint should return HTTP 404 if the user has no verifications. """ - user = UserFactory.create() + user = UserFactory() path = reverse('verification_status', kwargs={'username': user.username}) self.assert_path_not_found(path) @@ -69,17 +69,19 @@ class PhotoVerificationStatusViewTests(TestCase): def test_staff_user(self): """ The endpoint should be accessible to staff users. """ + self.client.logout() self.client.login(username=self.staff.username, password=self.PASSWORD) self.assert_verification_returned() def test_owner(self): """ The endpoint should be accessible to the user who submitted the verification request. """ - self.client.login(username=self.user.username, password=self.user.password) + self.client.logout() + self.client.login(username=self.user.username, password=self.PASSWORD) self.assert_verification_returned() def test_non_owner_or_staff_user(self): """ The endpoint should NOT be accessible if the request is not made by the submitter or staff user. """ - user = UserFactory.create() + user = UserFactory() self.client.login(username=user.username, password=self.PASSWORD) response = self.client.get(self.path) self.assertEqual(response.status_code, 403) @@ -88,5 +90,6 @@ class PhotoVerificationStatusViewTests(TestCase): """ The endpoint should return that the user is verified if the user's verification is accepted. """ self.verification.status = 'approved' self.verification.save() - self.client.login(username=self.user.username, password=self.user.password) + self.client.logout() + self.client.login(username=self.user.username, password=self.PASSWORD) self.assert_verification_returned(verified=True) diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index 0e666f3e0c..9c70d3cdd4 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -156,4 +156,5 @@ class IsStaffOrOwner(permissions.BasePermission): user = request.user return user.is_staff \ or (user.username == request.GET.get('username')) \ - or (user.username == getattr(request, 'data', {}).get('username')) + or (user.username == getattr(request, 'data', {}).get('username')) \ + or (user.username == getattr(view, 'kwargs', {}).get('username')) diff --git a/openedx/core/lib/api/tests/test_permissions.py b/openedx/core/lib/api/tests/test_permissions.py index d0142c73f8..e9b3003815 100644 --- a/openedx/core/lib/api/tests/test_permissions.py +++ b/openedx/core/lib/api/tests/test_permissions.py @@ -5,6 +5,7 @@ from django.contrib.auth.models import AnonymousUser from django.http import Http404 from django.test import TestCase, RequestFactory from nose.plugins.attrib import attr +from rest_framework.generics import GenericAPIView from student.roles import CourseStaffRole, CourseInstructorRole from openedx.core.lib.api.permissions import ( @@ -37,8 +38,8 @@ class IsCourseStaffInstructorTests(TestCase): def setUp(self): super(IsCourseStaffInstructorTests, self).setUp() self.permission = IsCourseStaffInstructor() - self.coach = UserFactory.create() - self.user = UserFactory.create() + self.coach = UserFactory() + self.user = UserFactory() self.request = RequestFactory().get('/') self.request.user = self.user self.course_key = CourseKey.from_string('edx/test123/run') @@ -72,7 +73,7 @@ class IsMasterCourseStaffInstructorTests(TestCase): super(IsMasterCourseStaffInstructorTests, self).setUp() self.permission = IsMasterCourseStaffInstructor() master_course_id = 'edx/test123/run' - self.user = UserFactory.create() + self.user = UserFactory() self.get_request = RequestFactory().get('/?master_course_id={}'.format(master_course_id)) self.get_request.user = self.user self.post_request = RequestFactory().post('/', data={'master_course_id': master_course_id}) @@ -133,36 +134,44 @@ class IsStaffOrOwnerTests(TestCase): def test_staff_user(self): """ Staff users should be permitted. """ - user = UserFactory.create(is_staff=True) + user = UserFactory(is_staff=True) self.assert_user_has_object_permission(user, True) def test_owner(self): """ Owners should be permitted. """ - user = UserFactory.create() + user = UserFactory() self.obj.user = user self.assert_user_has_object_permission(user, True) def test_non_staff_test_non_owner_or_staff_user(self): """ Non-staff and non-owner users should not be permitted. """ - user = UserFactory.create() + user = UserFactory() self.assert_user_has_object_permission(user, False) def test_has_permission_as_staff(self): """ Staff users always have permission. """ - self.request.user = UserFactory.create(is_staff=True) + self.request.user = UserFactory(is_staff=True) self.assertTrue(self.permission.has_permission(self.request, None)) def test_has_permission_as_owner_with_get(self): """ Owners always have permission to make GET actions. """ - user = UserFactory.create() + user = UserFactory() request = RequestFactory().get('/?username={}'.format(user.username)) request.user = user self.assertTrue(self.permission.has_permission(request, None)) + def test_has_permission_with_view_kwargs_as_owner_with_get(self): + """ Owners always have permission to make GET actions. """ + user = UserFactory() + self.request.user = user + view = GenericAPIView() + view.kwargs = {'username': user.username} + self.assertTrue(self.permission.has_permission(self.request, view)) + @ddt.data('patch', 'post', 'put') def test_has_permission_as_owner_with_edit(self, action): """ Owners always have permission to edit. """ - user = UserFactory.create() + user = UserFactory() data = {'username': user.username} request = getattr(RequestFactory(), action)('/', data, format='json') @@ -172,7 +181,15 @@ class IsStaffOrOwnerTests(TestCase): def test_has_permission_as_non_owner(self): """ Non-owners should not have permission. """ - user = UserFactory.create() + user = UserFactory() request = RequestFactory().get('/?username={}'.format(user.username)) - request.user = UserFactory.create() + request.user = UserFactory() self.assertFalse(self.permission.has_permission(request, None)) + + def test_has_permission_with_view_kwargs_as_non_owner(self): + """ Non-owners should not have permission. """ + user = UserFactory() + self.request.user = user + view = GenericAPIView() + view.kwargs = {'username': UserFactory().username} + self.assertFalse(self.permission.has_permission(self.request, view))