From bc72c28a031dcbfbb40404506e55197c3c35e383 Mon Sep 17 00:00:00 2001 From: bmedx Date: Wed, 20 Dec 2017 16:36:26 -0500 Subject: [PATCH 1/2] Remove unnecessary patches to User::is_authenticated --- cms/djangoapps/contentstore/tests/utils.py | 1 - lms/djangoapps/courseware/tests/test_tabs.py | 17 ++++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 87975ce395..dfc7c3b233 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -98,7 +98,6 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase): client = AjaxEnabledTestClient() if authenticate: client.login(username=nonstaff.username, password=password) - nonstaff.is_authenticated = lambda: authenticate return client, nonstaff def reload_course(self): diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index f7d8a8fd08..298c0ace49 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -57,13 +57,12 @@ class TabTestCase(SharedModuleStoreTestCase): super(TabTestCase, self).setUp() self.reverse = lambda name, args: "name/{0}/args/{1}".format(name, ",".join(str(a) for a in args)) - def create_mock_user(self, is_authenticated=True, is_staff=True, is_enrolled=True): + def create_mock_user(self, is_staff=True, is_enrolled=True): """ Creates a mock user with the specified properties. """ user = UserFactory(is_staff=is_staff) user.is_enrolled = is_enrolled - user.is_authenticated = lambda: is_authenticated return user def is_tab_enabled(self, tab, course, user): @@ -155,16 +154,16 @@ class TabTestCase(SharedModuleStoreTestCase): ): """Checks can display results for various users""" if for_staff_only: - user = self.create_mock_user(is_authenticated=True, is_staff=True, is_enrolled=True) + user = self.create_mock_user(is_staff=True, is_enrolled=True) self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) if for_authenticated_users_only: - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=False) + user = self.create_mock_user(is_staff=False, is_enrolled=False) self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: - user = self.create_mock_user(is_authenticated=False, is_staff=False, is_enrolled=False) + user = self.create_mock_user(is_staff=False, is_enrolled=False) self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) if for_enrolled_users_only: - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) + user = self.create_mock_user(is_staff=False, is_enrolled=True) self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) def check_get_and_set_methods(self, tab): @@ -214,7 +213,7 @@ class TextbooksTestCase(TabTestCase): type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} num_textbooks_found = 0 - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) + user = self.create_mock_user(is_staff=False, is_enrolled=True) for tab in xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user): # verify all textbook type tabs if tab.type == 'single_textbook': @@ -704,7 +703,7 @@ class CourseTabListTestCase(TabListTestCase): course_staff_only=True)) self.course.save() - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) + user = self.create_mock_user(is_staff=False, is_enrolled=True) request = get_mock_request(user) course_tab_list = get_course_tab_list(request, self.course) name_list = [x.name for x in course_tab_list] @@ -837,7 +836,7 @@ class DiscussionLinkTestCase(TabTestCase): self.course.tabs = tab_list self.course.discussion_link = discussion_link_in_course discussion_tab = xmodule_tabs.CourseTabList.get_discussion(self.course) - user = self.create_mock_user(is_authenticated=True, is_staff=is_staff, is_enrolled=is_enrolled) + user = self.create_mock_user(is_staff=is_staff, is_enrolled=is_enrolled) with patch('student.models.CourseEnrollment.is_enrolled') as check_is_enrolled: check_is_enrolled.return_value = is_enrolled self.assertEquals( From b5d6fca44ec05433db2101a26c1102944749977d Mon Sep 17 00:00:00 2001 From: bmedx Date: Thu, 21 Dec 2017 13:21:06 -0500 Subject: [PATCH 2/2] Update is_authenticated patches to work with Django 1.10+ --- common/djangoapps/student/tests/test_authz.py | 10 ++++++---- lms/djangoapps/courseware/tests/test_tabs.py | 4 +++- lms/djangoapps/lti_provider/tests/test_users.py | 13 +++++++------ lms/djangoapps/lti_provider/tests/test_views.py | 3 +-- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index 2395d58e07..048ec34c5f 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -113,8 +113,9 @@ class CreatorGroupTest(TestCase): def test_add_user_to_group_requires_authenticated(self): with self.assertRaises(PermissionDenied): - self.admin.is_authenticated = mock.Mock(return_value=False) - add_users(self.admin, CourseCreatorRole(), self.user) + with mock.patch('django.contrib.auth.models.User.is_authenticated') as mock_is_auth: + mock_is_auth.return_value = False + add_users(self.admin, CourseCreatorRole(), self.user) def test_remove_user_from_group_requires_staff_access(self): with self.assertRaises(PermissionDenied): @@ -128,8 +129,9 @@ class CreatorGroupTest(TestCase): def test_remove_user_from_group_requires_authenticated(self): with self.assertRaises(PermissionDenied): - self.admin.is_authenticated = mock.Mock(return_value=False) - remove_users(self.admin, CourseCreatorRole(), self.user) + with mock.patch('django.contrib.auth.models.User.is_authenticated') as mock_is_auth: + mock_is_auth.return_value = False + remove_users(self.admin, CourseCreatorRole(), self.user) class CCXCourseGroupTest(TestCase): diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 298c0ace49..00825797b1 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -3,6 +3,7 @@ Test cases for tabs. """ import pytest +from django.contrib.auth.models import AnonymousUser from django.core.urlresolvers import reverse from django.http import Http404 from milestones.tests.utils import MilestonesTestCaseMixin @@ -159,8 +160,9 @@ class TabTestCase(SharedModuleStoreTestCase): if for_authenticated_users_only: user = self.create_mock_user(is_staff=False, is_enrolled=False) self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) + assert False if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: - user = self.create_mock_user(is_staff=False, is_enrolled=False) + user = AnonymousUser() self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) if for_enrolled_users_only: user = self.create_mock_user(is_staff=False, is_enrolled=True) diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py index ca8f5364b4..0865efa99e 100644 --- a/lms/djangoapps/lti_provider/tests/test_users.py +++ b/lms/djangoapps/lti_provider/tests/test_users.py @@ -116,7 +116,7 @@ class AuthenticateLtiUserTest(TestCase): def test_authentication_with_authenticated_user(self, create_user, switch_user): lti_user = self.create_lti_user_model() self.request.user = lti_user.edx_user - self.request.user.is_authenticated = MagicMock(return_value=True) + assert self.request.user.is_authenticated() users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer) self.assertFalse(create_user.called) self.assertFalse(switch_user.called) @@ -124,15 +124,16 @@ class AuthenticateLtiUserTest(TestCase): def test_authentication_with_unauthenticated_user(self, create_user, switch_user): lti_user = self.create_lti_user_model() self.request.user = lti_user.edx_user - self.request.user.is_authenticated = MagicMock(return_value=False) - users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer) - self.assertFalse(create_user.called) - switch_user.assert_called_with(self.request, lti_user, self.lti_consumer) + with patch('django.contrib.auth.models.User.is_authenticated') as mock_is_auth: + mock_is_auth.return_value = False + users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer) + self.assertFalse(create_user.called) + switch_user.assert_called_with(self.request, lti_user, self.lti_consumer) def test_authentication_with_wrong_user(self, create_user, switch_user): lti_user = self.create_lti_user_model() self.request.user = self.old_user - self.request.user.is_authenticated = MagicMock(return_value=True) + assert self.request.user.is_authenticated() users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer) self.assertFalse(create_user.called) switch_user.assert_called_with(self.request, lti_user, self.lti_consumer) diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index a8a1ee7902..482dc866f6 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -45,13 +45,12 @@ COURSE_PARAMS = { ALL_PARAMS = dict(LTI_DEFAULT_PARAMS.items() + COURSE_PARAMS.items()) -def build_launch_request(authenticated=True): +def build_launch_request(): """ Helper method to create a new request object for the LTI launch. """ request = RequestFactory().post('/') request.user = UserFactory.create() - request.user.is_authenticated = MagicMock(return_value=authenticated) request.session = {} request.POST.update(LTI_DEFAULT_PARAMS) return request