From 2da42d5efa0a3e785490babe24dfcf0bfb5ab8cd Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Wed, 10 Jun 2015 18:55:27 -0400 Subject: [PATCH] Removed IsAuthenticatedOrDebug IsAuthenticatedOrDebug hides potential issues with API client code that is run in local environments and later deployed to production where authentication fails. XCOM-193 --- .../course_structure_api/v0/tests.py | 22 +------------------ .../course_structure_api/v0/views.py | 4 ++-- openedx/core/lib/api/permissions.py | 13 ----------- openedx/core/lib/api/view_utils.py | 5 +++-- 4 files changed, 6 insertions(+), 38 deletions(-) diff --git a/lms/djangoapps/course_structure_api/v0/tests.py b/lms/djangoapps/course_structure_api/v0/tests.py index 9426126217..3545479a37 100644 --- a/lms/djangoapps/course_structure_api/v0/tests.py +++ b/lms/djangoapps/course_structure_api/v0/tests.py @@ -156,11 +156,7 @@ class CourseDetailMixin(object): return response def test_not_authenticated(self): - # If debug mode is enabled, the view should always return data. - with override_settings(DEBUG=True): - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), HTTP_AUTHORIZATION=None) - self.assertEqual(response.status_code, 200) - + """ The view should return HTTP status 401 if no user is authenticated. """ # HTTP 401 should be returned if the user is not authenticated. response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), HTTP_AUTHORIZATION=None) self.assertEqual(response.status_code, 401) @@ -170,12 +166,6 @@ class CourseDetailMixin(object): access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token auth_header = 'Bearer ' + access_token - # If debug mode is enabled, the view should always return data. - with override_settings(DEBUG=True): - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), - HTTP_AUTHORIZATION=auth_header) - self.assertEqual(response.status_code, 200) - # Access should be granted if the proper access token is supplied. response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), HTTP_AUTHORIZATION=auth_header) @@ -231,11 +221,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): self.assertValidResponseCourse(courses[0], self.course) def test_not_authenticated(self): - # If debug mode is enabled, the view should always return data. - with override_settings(DEBUG=True): - response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=None) - self.assertEqual(response.status_code, 200) - response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=None) self.assertEqual(response.status_code, 401) @@ -247,11 +232,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token auth_header = 'Bearer ' + access_token - # If debug mode is enabled, the view should always return data. - with override_settings(DEBUG=True): - response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=auth_header) - self.assertEqual(response.status_code, 200) - # Data should be returned if the user is authorized. response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=auth_header) self.assertEqual(response.status_code, 200) diff --git a/lms/djangoapps/course_structure_api/v0/views.py b/lms/djangoapps/course_structure_api/v0/views.py index e182315aff..d9468b2c1c 100644 --- a/lms/djangoapps/course_structure_api/v0/views.py +++ b/lms/djangoapps/course_structure_api/v0/views.py @@ -7,6 +7,7 @@ from django.http import Http404 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication from rest_framework.exceptions import PermissionDenied, AuthenticationFailed from rest_framework.generics import RetrieveAPIView, ListAPIView +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from xmodule.modulestore.django import modulestore from opaque_keys.edx.keys import CourseKey @@ -15,7 +16,6 @@ from course_structure_api.v0 import api, serializers from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError from courseware import courses from courseware.access import has_access -from openedx.core.lib.api.permissions import IsAuthenticatedOrDebug from openedx.core.lib.api.serializers import PaginationSerializer from student.roles import CourseInstructorRole, CourseStaffRole @@ -29,7 +29,7 @@ class CourseViewMixin(object): """ lookup_field = 'course_id' authentication_classes = (OAuth2Authentication, SessionAuthentication,) - permission_classes = (IsAuthenticatedOrDebug,) + permission_classes = (IsAuthenticated,) def get_course_or_404(self): """ diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index a48c577988..5b2701787c 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -35,19 +35,6 @@ class ApiKeyHeaderPermissionIsAuthenticated(ApiKeyHeaderPermission, permissions. return api_permissions or is_authenticated_permissions -class IsAuthenticatedOrDebug(permissions.BasePermission): - """ - Allows access only to authenticated users, or anyone if debug mode is enabled. - """ - - def has_permission(self, request, view): - if settings.DEBUG: - return True - - user = getattr(request, 'user', None) - return user and user.is_authenticated() - - class IsUserInUrl(permissions.BasePermission): """ Permission that checks to see if the request user matches the user in the URL. diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py index ca1c85740f..517c9c9508 100644 --- a/openedx/core/lib/api/view_utils.py +++ b/openedx/core/lib/api/view_utils.py @@ -8,6 +8,7 @@ from django.utils.translation import ugettext as _ from rest_framework import status, response from rest_framework.exceptions import APIException +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin from rest_framework.generics import GenericAPIView @@ -20,7 +21,7 @@ from openedx.core.lib.api.authentication import ( SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser, ) -from openedx.core.lib.api.permissions import IsUserInUrl, IsAuthenticatedOrDebug +from openedx.core.lib.api.permissions import IsUserInUrl from util.milestones_helpers import any_unfulfilled_milestones @@ -131,7 +132,7 @@ def view_auth_classes(is_user=False): OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser ) - func_or_class.permission_classes = (IsAuthenticatedOrDebug,) + func_or_class.permission_classes = (IsAuthenticated,) if is_user: func_or_class.permission_classes += (IsUserInUrl,) return func_or_class