Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update Permissions to use latest edx-drf-extensions and rest_condition

Browse Source
This commit is contained in:
Nimisha Asthagiri
2018-06-28 18:13:23 -04:00
parent 522349c502
commit ce4cb53672
7 changed files with 61 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lms/djangoapps/mobile_api/testutils.py
View File

@@ -116,7 +116,7 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin):
"""
def test_invalid_user(self):
self.login_and_enroll()
self.api_response(expected_response_code=404, username='no_user')
self.api_response(expected_response_code=403, username='no_user')
def test_other_user(self):
# login and enroll as the test user
@@ -131,7 +131,7 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin):
# now login and call the API as the test user
self.login()
self.api_response(expected_response_code=404, username=other.username)
self.api_response(expected_response_code=403, username=other.username)
@ddt.ddt

24
openedx/core/djangoapps/bookmarks/tests/test_views.py
View File

@@ -17,7 +17,6 @@ from .test_api import BookmarkApiEventTestMixin
from .test_models import BookmarksTestsBase
# pylint: disable=no-member
class BookmarksViewsTestsBase(BookmarksTestsBase, BookmarkApiEventTestMixin):
"""
Base class for bookmarks views tests.
@@ -397,15 +396,28 @@ class BookmarksDetailViewTests(BookmarksViewsTestsBase):
def test_get_bookmark_that_belongs_to_other_user(self):
"""
Test that requesting bookmark that belongs to other user returns 404 status code.
Test that requesting bookmark that belongs to other user returns 403 status code.
"""
self.send_get(
client=self.client,
url=reverse(
'bookmarks_detail',
kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)}
kwargs={'username': self.other_user.username, 'usage_id': unicode(self.vertical_1.location)}
),
expected_status=404
expected_status=403
)
def test_get_bookmark_that_belongs_to_nonexistent_user(self):
"""
Test that requesting bookmark that belongs to a non-existent user also returns 403 status code.
"""
self.send_get(
client=self.client,
url=reverse(
'bookmarks_detail',
kwargs={'username': 'non-existent', 'usage_id': unicode(self.vertical_1.location)}
),
expected_status=403
)
def test_get_bookmark_that_does_not_exist(self):
@@ -482,7 +494,7 @@ class BookmarksDetailViewTests(BookmarksViewsTestsBase):
def test_delete_bookmark_that_belongs_to_other_user(self):
"""
Test that delete bookmark that belongs to other user returns 404.
Test that delete bookmark that belongs to other user returns 403.
"""
self.send_delete(
client=self.client,
@@ -490,7 +502,7 @@ class BookmarksDetailViewTests(BookmarksViewsTestsBase):
'bookmarks_detail',
kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)}
),
expected_status=404
expected_status=403
)
def test_delete_bookmark_that_does_not_exist(self):

16
openedx/core/djangoapps/profile_images/tests/test_views.py
View File

@@ -243,6 +243,18 @@ class ProfileImageViewPostTestCase(ProfileImageEndpointMixin, APITestCase):
self.assertFalse(mock_log.info.called)
self.assert_no_events_were_emitted()
def test_upload_nonexistent_user(self, mock_log):
"""
Test that an authenticated user who POSTs to a non-existent user's upload
endpoint gets an indistinguishable 403.
"""
nonexistent_user_url = reverse(self._view_name, kwargs={'username': 'nonexistent'})
with make_image_file() as image_file:
response = self.client.post(nonexistent_user_url, {'file': image_file}, format='multipart')
self.check_response(response, 403)
self.assertFalse(mock_log.info.called)
def test_upload_other(self, mock_log):
"""
Test that an authenticated user cannot POST to another user's upload
@@ -255,7 +267,7 @@ class ProfileImageViewPostTestCase(ProfileImageEndpointMixin, APITestCase):
different_client.login(username=different_user.username, password=TEST_PASSWORD)
with make_image_file() as image_file:
response = different_client.post(self.url, {'file': image_file}, format='multipart')
self.check_response(response, 404)
self.check_response(response, 403)
self.check_images(False)
self.check_has_profile_image(False)
self.assertFalse(mock_log.info.called)
@@ -407,7 +419,7 @@ class ProfileImageViewDeleteTestCase(ProfileImageEndpointMixin, APITestCase):
different_client = APIClient()
different_client.login(username=different_user.username, password=TEST_PASSWORD)
response = different_client.delete(self.url)
self.check_response(response, 404)
self.check_response(response, 403)
self.check_images(True) # thumbnails should remain intact.
self.check_has_profile_image(True)
self.assertFalse(mock_log.info.called)

3
openedx/core/djangoapps/user_api/preferences/api.py
View File

@@ -315,12 +315,13 @@ def _get_authorized_user(requesting_user, username=None, allow_staff=False):
# Otherwise, treat this as a request against a separate user
username = requesting_user.username
_check_authorized(requesting_user, username, allow_staff)
try:
existing_user = User.objects.get(username=username)
except ObjectDoesNotExist:
raise UserNotFound()
_check_authorized(requesting_user, username, allow_staff)
return existing_user

10
openedx/core/djangoapps/user_api/preferences/tests/test_api.py
View File

@@ -77,7 +77,7 @@ class TestPreferenceAPI(CacheIsolationTestCase):
"""
Verifies that get_user_preference returns appropriate errors.
"""
with self.assertRaises(UserNotFound):
with self.assertRaises(UserNotAuthorized):
get_user_preference(self.user, self.test_preference_key, username="no_such_user")
with self.assertRaises(UserNotFound):
@@ -100,7 +100,7 @@ class TestPreferenceAPI(CacheIsolationTestCase):
"""
Verifies that get_user_preferences returns appropriate errors.
"""
with self.assertRaises(UserNotFound):
with self.assertRaises(UserNotAuthorized):
get_user_preferences(self.user, username="no_such_user")
with self.assertRaises(UserNotFound):
@@ -125,7 +125,7 @@ class TestPreferenceAPI(CacheIsolationTestCase):
"""
Verifies that set_user_preference returns appropriate errors.
"""
with self.assertRaises(UserNotFound):
with self.assertRaises(UserNotAuthorized):
set_user_preference(self.user, self.test_preference_key, "new_value", username="no_such_user")
with self.assertRaises(UserNotFound):
@@ -227,7 +227,7 @@ class TestPreferenceAPI(CacheIsolationTestCase):
update_data = {
self.test_preference_key: "new_value"
}
with self.assertRaises(UserNotFound):
with self.assertRaises(UserNotAuthorized):
update_user_preferences(self.user, update_data, user="no_such_user")
with self.assertRaises(UserNotFound):
@@ -303,7 +303,7 @@ class TestPreferenceAPI(CacheIsolationTestCase):
"""
Verifies that delete_user_preference returns appropriate errors.
"""
with self.assertRaises(UserNotFound):
with self.assertRaises(UserNotAuthorized):
delete_user_preference(self.user, self.test_preference_key, username="no_such_user")
with self.assertRaises(UserNotFound):

33
openedx/core/djangoapps/user_api/preferences/tests/test_views.py
View File

@@ -52,7 +52,7 @@ class TestPreferencesAPI(UserAPITestCase):
Test that a client (logged in) cannot get the preferences information for a different client.
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.send_get(self.different_client, expected_status=404)
self.send_get(self.different_client, expected_status=403)
@ddt.data(
("client", "user"),
@@ -61,11 +61,11 @@ class TestPreferencesAPI(UserAPITestCase):
@ddt.unpack
def test_get_unknown_user(self, api_client, username):
"""
Test that requesting a user who does not exist returns a 404.
Test that requesting a user who does not exist returns a 404 for staff users, but 403 for others.
"""
client = self.login_client(api_client, username)
response = client.get(reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist"}))
self.assertEqual(404, response.status_code)
self.assertEqual(404 if username == "staff_user" else 403, response.status_code)
def test_get_preferences_default(self):
"""
@@ -83,8 +83,7 @@ class TestPreferencesAPI(UserAPITestCase):
@ddt.unpack
def test_get_preferences(self, api_client, user):
"""
Test that a client (logged in) can get her own preferences information. Also verifies that a "is_staff"
user can get the preferences information for other users.
Test that a client (logged in) can get her own preferences information.
"""
# Create some test preferences values.
set_user_preference(self.user, "dict_pref", {"int_key": 10})
@@ -104,14 +103,14 @@ class TestPreferencesAPI(UserAPITestCase):
@ddt.unpack
def test_patch_unknown_user(self, api_client, user):
"""
Test that trying to update preferences for a user who does not exist returns a 404.
Test that trying to update preferences for a user who does not exist returns a 403.
"""
client = self.login_client(api_client, user)
response = client.patch(
reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist"}),
data=json.dumps({"string_pref": "value"}), content_type="application/merge-patch+json"
)
self.assertEqual(404, response.status_code)
self.assertEqual(403, response.status_code)
def test_patch_bad_content_type(self):
"""
@@ -168,7 +167,7 @@ class TestPreferencesAPI(UserAPITestCase):
"dict_pref": {"int_key": 10},
"string_pref": "value",
},
expected_status=403 if user == "staff_user" else 404,
expected_status=403,
)
def test_update_preferences(self):
@@ -311,7 +310,7 @@ class TestPreferencesAPI(UserAPITestCase):
"new_pref": "new_value",
"extra_pref": None,
},
expected_status=403 if user == "staff_user" else 404
expected_status=403
)
@@ -405,9 +404,9 @@ class TestPreferencesDetailAPI(UserAPITestCase):
Test that a client (logged in) cannot manipulate a preference for a different client.
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.send_get(self.different_client, expected_status=404)
self.send_put(self.different_client, "new_value", expected_status=404)
self.send_delete(self.different_client, expected_status=404)
self.send_get(self.different_client, expected_status=403)
self.send_put(self.different_client, "new_value", expected_status=403)
self.send_delete(self.different_client, expected_status=403)
@ddt.data(
("client", "user"),
@@ -416,13 +415,13 @@ class TestPreferencesDetailAPI(UserAPITestCase):
@ddt.unpack
def test_get_unknown_user(self, api_client, username):
"""
Test that requesting a user who does not exist returns a 404.
Test that requesting a user who does not exist returns a 404 for staff users, but 403 for others.
"""
client = self.login_client(api_client, username)
response = client.get(
reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist", 'preference_key': self.test_pref_key})
)
self.assertEqual(404, response.status_code)
self.assertEqual(404 if username == "staff_user" else 403, response.status_code)
def test_get_preference_does_not_exist(self):
"""
@@ -532,7 +531,7 @@ class TestPreferencesDetailAPI(UserAPITestCase):
self._set_url("new_key")
client = self.login_client(api_client, user)
new_value = "new value"
self.send_put(client, new_value, expected_status=403 if user == "staff_user" else 404)
self.send_put(client, new_value, expected_status=403)
@ddt.data(
(u"new value",),
@@ -560,7 +559,7 @@ class TestPreferencesDetailAPI(UserAPITestCase):
"""
client = self.login_client(api_client, user)
new_value = "new value"
self.send_put(client, new_value, expected_status=403 if user == "staff_user" else 404)
self.send_put(client, new_value, expected_status=403)
@ddt.data(
(None,),
@@ -607,4 +606,4 @@ class TestPreferencesDetailAPI(UserAPITestCase):
Test that a client (logged in) cannot delete a preference for another user.
"""
client = self.login_client(api_client, user)
self.send_delete(client, expected_status=403 if user == "staff_user" else 404)
self.send_delete(client, expected_status=403)

48
openedx/core/lib/api/permissions.py
View File

@@ -6,8 +6,10 @@ from django.conf import settings
from django.http import Http404
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from rest_condition import C
from rest_framework import permissions
from edx_rest_framework_extensions.permissions import IsStaff, IsUserInUrl
from openedx.core.lib.log_utils import audit_log
from student.roles import CourseInstructorRole, CourseStaffRole
@@ -20,7 +22,6 @@ class ApiKeyHeaderPermission(permissions.BasePermission):
def has_permission(self, request, view):
"""
Check for permissions by matching the configured API key and header
Allow the request if and only if settings.EDX_API_KEY is set and
the X-Edx-Api-Key HTTP header is present in the request and
matches the setting.
@@ -39,7 +40,6 @@ class ApiKeyHeaderPermission(permissions.BasePermission):
class ApiKeyHeaderPermissionIsAuthenticated(ApiKeyHeaderPermission, permissions.IsAuthenticated):
"""
Allow someone to access the view if they have the API key OR they are authenticated.
See ApiKeyHeaderPermission for more information how the API key portion is implemented.
"""
@@ -50,29 +50,6 @@ class ApiKeyHeaderPermissionIsAuthenticated(ApiKeyHeaderPermission, permissions.
return api_permissions or is_authenticated_permissions
class IsUserInUrl(permissions.BasePermission):
"""
Permission that checks to see if the request user matches the user in the URL.
"""
def has_permission(self, request, view):
"""
Returns true if the current request is by the user themselves.
Note: a 404 is returned for non-staff instead of a 403. This is to prevent
users from being able to detect the existence of accounts.
"""
url_username = (
request.parser_context.get('kwargs', {}).get('username') or
request.GET.get('username', '')
)
if request.user.username.lower() != url_username.lower():
if request.user.is_staff:
return False # staff gets 403
raise Http404()
return True
class IsCourseStaffInstructor(permissions.BasePermission):
"""
Permission to check that user is a course instructor or staff of
@@ -118,26 +95,9 @@ class IsMasterCourseStaffInstructor(permissions.BasePermission):
return False
class IsStaff(permissions.BasePermission):
"""
Permission that checks to see if the request user has is_staff access.
"""
class IsUserInUrlOrStaff(permissions.BasePermission):
def has_permission(self, request, view):
if request.user.is_staff:
return True
class IsUserInUrlOrStaff(IsUserInUrl):
"""
Permission that checks to see if the request user matches the user in the URL or has is_staff access.
"""
def has_permission(self, request, view):
if request.user.is_staff:
return True
return super(IsUserInUrlOrStaff, self).has_permission(request, view)
return C(IsStaff) | IsUserInUrl
class IsStaffOrReadOnly(permissions.BasePermission):