From cbb6e5e6863b98c8b076885760a3027f24900073 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Fri, 22 Nov 2013 15:26:50 -0500 Subject: [PATCH] Changed default user to be anonymous in both has_access and student.views.index Replaced existing test_none_user_index_access_with_startdate_fails test with new test now that the tested function has changed and was causing the original test to fail. --- common/djangoapps/student/views.py | 4 ++-- lms/djangoapps/branding/tests.py | 12 ++++++++---- lms/djangoapps/courseware/access.py | 9 +++++++-- lms/djangoapps/courseware/tests/test_access.py | 4 ++++ 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index a0cf27d786..aaf4ca8274 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -13,7 +13,7 @@ import time from django.conf import settings from django.contrib.auth import logout, authenticate, login -from django.contrib.auth.models import User +from django.contrib.auth.models import User, AnonymousUser from django.contrib.auth.decorators import login_required from django.contrib.auth.views import password_reset_confirm # from django.contrib.sessions.models import Session @@ -91,7 +91,7 @@ def csrf_token(context): # branding/views.py:index(), which is cached for anonymous users. # This means that it should always return the same thing for anon # users. (in particular, no switching based on query params allowed) -def index(request, extra_context={}, user=None): +def index(request, extra_context={}, user=AnonymousUser()): """ Render the edX main page. diff --git a/lms/djangoapps/branding/tests.py b/lms/djangoapps/branding/tests.py index e880ccb62d..3849e54161 100644 --- a/lms/djangoapps/branding/tests.py +++ b/lms/djangoapps/branding/tests.py @@ -4,8 +4,10 @@ Tests for branding page import datetime from pytz import UTC from django.conf import settings +from django.contrib.auth.models import AnonymousUser from django.test.utils import override_settings from django.test.client import RequestFactory + from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.django import editable_modulestore from xmodule.modulestore.tests.factories import CourseFactory @@ -34,11 +36,13 @@ class AnonymousIndexPageTest(ModuleStoreTestCase): @override_settings(FEATURES=FEATURES_WITH_STARTDATE) def test_none_user_index_access_with_startdate_fails(self): """ - This was a "before" test for a bugfix. If someone fixes the bug another way in the future - and this test begins failing (but the other two pass), then feel free to delete this test. + This is a regression test for a bug where the incoming user is + anonymous and start dates are being checked. It replaces a previous + test as it solves the issue in a different way """ - with self.assertRaisesRegexp(AttributeError, "'NoneType' object has no attribute 'is_authenticated'"): - student.views.index(self.factory.get('/'), user=None) # pylint: disable=E1101 + request = self.factory.get('/') + request.user = AnonymousUser() + student.views.index(request) @override_settings(FEATURES=FEATURES_WITH_STARTDATE) def test_anon_user_with_startdate_index(self): diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 9c456afc2f..acb850ff56 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta from functools import partial from django.conf import settings -from django.contrib.auth.models import Group +from django.contrib.auth.models import Group, AnonymousUser from xmodule.course_module import CourseDescriptor from xmodule.error_module import ErrorDescriptor @@ -44,7 +44,8 @@ def has_access(user, obj, action, course_context=None): - DISABLE_START_DATES - different access for instructor, staff, course staff, and students. - user: a Django user object. May be anonymous. + user: a Django user object. May be anonymous. If none is passed, + anonymous is assumed obj: The object to check access for. A module, descriptor, location, or certain special strings (e.g. 'global') @@ -61,6 +62,10 @@ def has_access(user, obj, action, course_context=None): Returns a bool. It is up to the caller to actually deny access in a way that makes sense in context. """ + # Just in case user is passed in as None, make them anonymous + if not user: + user = AnonymousUser() + # delegate the work to type-specific functions. # (start with more specific types, then get more general) if isinstance(obj, CourseDescriptor): diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index a19b6e464d..3b24d5d4c7 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -98,3 +98,7 @@ class AccessTestCase(TestCase): # TODO: # Non-staff cannot enroll outside the open enrollment period if not specifically allowed + + def test__user_passed_as_none(self): + """Ensure has_access handles a user being passed as null""" + access.has_access(None, 'global', 'staff', None)