Commit 2da42d5e by Clinton Blackburn

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
parent a7e49c21
...@@ -156,11 +156,7 @@ class CourseDetailMixin(object): ...@@ -156,11 +156,7 @@ class CourseDetailMixin(object):
return response return response
def test_not_authenticated(self): def test_not_authenticated(self):
# If debug mode is enabled, the view should always return data. """ The view should return HTTP status 401 if no user is authenticated. """
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)
# HTTP 401 should be returned if the user is not 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) response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), HTTP_AUTHORIZATION=None)
self.assertEqual(response.status_code, 401) self.assertEqual(response.status_code, 401)
...@@ -170,12 +166,6 @@ class CourseDetailMixin(object): ...@@ -170,12 +166,6 @@ class CourseDetailMixin(object):
access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token
auth_header = 'Bearer ' + access_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. # Access should be granted if the proper access token is supplied.
response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}),
HTTP_AUTHORIZATION=auth_header) HTTP_AUTHORIZATION=auth_header)
...@@ -231,11 +221,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): ...@@ -231,11 +221,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase):
self.assertValidResponseCourse(courses[0], self.course) self.assertValidResponseCourse(courses[0], self.course)
def test_not_authenticated(self): 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) response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=None)
self.assertEqual(response.status_code, 401) self.assertEqual(response.status_code, 401)
...@@ -247,11 +232,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): ...@@ -247,11 +232,6 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase):
access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token access_token = AccessTokenFactory.create(user=user, client=self.oauth_client).token
auth_header = 'Bearer ' + access_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. # Data should be returned if the user is authorized.
response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=auth_header) response = self.http_get(reverse(self.view), HTTP_AUTHORIZATION=auth_header)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
......
...@@ -7,6 +7,7 @@ from django.http import Http404 ...@@ -7,6 +7,7 @@ from django.http import Http404
from rest_framework.authentication import OAuth2Authentication, SessionAuthentication from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
from rest_framework.exceptions import PermissionDenied, AuthenticationFailed from rest_framework.exceptions import PermissionDenied, AuthenticationFailed
from rest_framework.generics import RetrieveAPIView, ListAPIView from rest_framework.generics import RetrieveAPIView, ListAPIView
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response from rest_framework.response import Response
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
...@@ -15,7 +16,6 @@ from course_structure_api.v0 import api, serializers ...@@ -15,7 +16,6 @@ from course_structure_api.v0 import api, serializers
from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError
from courseware import courses from courseware import courses
from courseware.access import has_access from courseware.access import has_access
from openedx.core.lib.api.permissions import IsAuthenticatedOrDebug
from openedx.core.lib.api.serializers import PaginationSerializer from openedx.core.lib.api.serializers import PaginationSerializer
from student.roles import CourseInstructorRole, CourseStaffRole from student.roles import CourseInstructorRole, CourseStaffRole
...@@ -29,7 +29,7 @@ class CourseViewMixin(object): ...@@ -29,7 +29,7 @@ class CourseViewMixin(object):
""" """
lookup_field = 'course_id' lookup_field = 'course_id'
authentication_classes = (OAuth2Authentication, SessionAuthentication,) authentication_classes = (OAuth2Authentication, SessionAuthentication,)
permission_classes = (IsAuthenticatedOrDebug,) permission_classes = (IsAuthenticated,)
def get_course_or_404(self): def get_course_or_404(self):
""" """
......
...@@ -35,19 +35,6 @@ class ApiKeyHeaderPermissionIsAuthenticated(ApiKeyHeaderPermission, permissions. ...@@ -35,19 +35,6 @@ class ApiKeyHeaderPermissionIsAuthenticated(ApiKeyHeaderPermission, permissions.
return api_permissions or is_authenticated_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): class IsUserInUrl(permissions.BasePermission):
""" """
Permission that checks to see if the request user matches the user in the URL. Permission that checks to see if the request user matches the user in the URL.
......
...@@ -8,6 +8,7 @@ from django.utils.translation import ugettext as _ ...@@ -8,6 +8,7 @@ from django.utils.translation import ugettext as _
from rest_framework import status, response from rest_framework import status, response
from rest_framework.exceptions import APIException from rest_framework.exceptions import APIException
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response from rest_framework.response import Response
from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin
from rest_framework.generics import GenericAPIView from rest_framework.generics import GenericAPIView
...@@ -20,7 +21,7 @@ from openedx.core.lib.api.authentication import ( ...@@ -20,7 +21,7 @@ from openedx.core.lib.api.authentication import (
SessionAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser,
OAuth2AuthenticationAllowInactiveUser, 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 from util.milestones_helpers import any_unfulfilled_milestones
...@@ -131,7 +132,7 @@ def view_auth_classes(is_user=False): ...@@ -131,7 +132,7 @@ def view_auth_classes(is_user=False):
OAuth2AuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser SessionAuthenticationAllowInactiveUser
) )
func_or_class.permission_classes = (IsAuthenticatedOrDebug,) func_or_class.permission_classes = (IsAuthenticated,)
if is_user: if is_user:
func_or_class.permission_classes += (IsUserInUrl,) func_or_class.permission_classes += (IsUserInUrl,)
return func_or_class return func_or_class
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment