From e7450874557b48c5479dad6bc9f008c6c50100ce Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Tue, 29 Jan 2013 14:30:15 -0500 Subject: [PATCH] Fix permissions bug and add test cases for django comment client permissions. --- .../django_comment_client/permissions.py | 7 +- lms/djangoapps/django_comment_client/tests.py | 111 --------------- .../django_comment_client/tests/__init__.py | 0 .../tests/test_permissions.py | 128 ++++++++++++++++++ 4 files changed, 133 insertions(+), 113 deletions(-) delete mode 100644 lms/djangoapps/django_comment_client/tests.py create mode 100644 lms/djangoapps/django_comment_client/tests/__init__.py create mode 100644 lms/djangoapps/django_comment_client/tests/test_permissions.py diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index b95a890dda..b583c3fe74 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -29,6 +29,7 @@ def has_permission(user, permission, course_id=None): CONDITIONS = ['is_open', 'is_author'] +# data may be a json file def check_condition(user, condition, course_id, data): def check_open(user, condition, course_id, data): try: @@ -61,8 +62,10 @@ def check_conditions_permissions(user, permissions, course_id, **kwargs): def test(user, per, operator="or"): if isinstance(per, basestring): if per in CONDITIONS: - return check_condition(user, per, course_id, kwargs) + return check_condition(user, per, course_id, kwargs['data']) return cached_has_permission(user, per, course_id=course_id) + # TODO: refactor this to be more clear. + # e.g. the "and operator in" bit on the next line is not needed? elif isinstance(per, list) and operator in ["and", "or"]: results = [test(user, x, operator="and") for x in per] if operator == "or": @@ -102,4 +105,4 @@ def check_permissions_by_view(user, course_id, content, name): p = VIEW_PERMISSIONS[name] except KeyError: logging.warning("Permission for view named %s does not exist in permissions.py" % name) - return check_conditions_permissions(user, p, course_id, content=content) + return check_conditions_permissions(user, p, course_id, data=content) diff --git a/lms/djangoapps/django_comment_client/tests.py b/lms/djangoapps/django_comment_client/tests.py deleted file mode 100644 index ac059a1e3f..0000000000 --- a/lms/djangoapps/django_comment_client/tests.py +++ /dev/null @@ -1,111 +0,0 @@ -from django.contrib.auth.models import User, Group -from django.core.urlresolvers import reverse -from django.test import TestCase -from django.test.client import RequestFactory -from django.conf import settings - -from mock import Mock - -from override_settings import override_settings - -import xmodule.modulestore.django - -from student.models import CourseEnrollment - -from django.db.models.signals import m2m_changed, pre_delete, pre_save, post_delete, post_save -from django.dispatch.dispatcher import _make_id -import string -import random -from .permissions import has_permission -from .models import Role, Permission - -from xmodule.modulestore.django import modulestore -from xmodule.modulestore import Location -from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.modulestore.xml import XMLModuleStore - -import comment_client - -from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE - -#@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) -#class TestCohorting(PageLoader): -# """Check that cohorting works properly""" -# -# def setUp(self): -# xmodule.modulestore.django._MODULESTORES = {} -# -# # Assume courses are there -# self.toy = modulestore().get_course("edX/toy/2012_Fall") -# -# # Create two accounts -# self.student = 'view@test.com' -# self.student2 = 'view2@test.com' -# self.password = 'foo' -# self.create_account('u1', self.student, self.password) -# self.create_account('u2', self.student2, self.password) -# self.activate_user(self.student) -# self.activate_user(self.student2) -# -# def test_create_thread(self): -# my_save = Mock() -# comment_client.perform_request = my_save -# -# resp = self.client.post( -# reverse('django_comment_client.base.views.create_thread', -# kwargs={'course_id': 'edX/toy/2012_Fall', -# 'commentable_id': 'General'}), -# {'some': "some", -# 'data': 'data'}) -# self.assertTrue(my_save.called) -# -# #self.assertEqual(resp.status_code, 200) -# #self.assertEqual(my_save.something, "expected", "complaint if not true") -# -# self.toy.metadata["cohort_config"] = {"cohorted": True} -# -# # call the view again ... -# -# # assert that different things happened - - - -class PermissionsTestCase(TestCase): - def random_str(self, length=15, chars=string.ascii_uppercase + string.digits): - return ''.join(random.choice(chars) for x in range(length)) - - def setUp(self): - - self.course_id = "edX/toy/2012_Fall" - - self.moderator_role = Role.objects.get_or_create(name="Moderator", course_id=self.course_id)[0] - self.student_role = Role.objects.get_or_create(name="Student", course_id=self.course_id)[0] - - self.student = User.objects.create(username=self.random_str(), - password="123456", email="john@yahoo.com") - self.moderator = User.objects.create(username=self.random_str(), - password="123456", email="staff@edx.org") - self.moderator.is_staff = True - self.moderator.save() - self.student_enrollment = CourseEnrollment.objects.create(user=self.student, course_id=self.course_id) - self.moderator_enrollment = CourseEnrollment.objects.create(user=self.moderator, course_id=self.course_id) - - def tearDown(self): - self.student_enrollment.delete() - self.moderator_enrollment.delete() - -# Do we need to have this? We shouldn't be deleting students, ever -# self.student.delete() -# self.moderator.delete() - - def testDefaultRoles(self): - self.assertTrue(self.student_role in self.student.roles.all()) - self.assertTrue(self.moderator_role in self.moderator.roles.all()) - - def testPermission(self): - name = self.random_str() - self.moderator_role.add_permission(name) - self.assertTrue(has_permission(self.moderator, name, self.course_id)) - - self.student_role.add_permission(name) - self.assertTrue(has_permission(self.student, name, self.course_id)) diff --git a/lms/djangoapps/django_comment_client/tests/__init__.py b/lms/djangoapps/django_comment_client/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/django_comment_client/tests/test_permissions.py b/lms/djangoapps/django_comment_client/tests/test_permissions.py new file mode 100644 index 0000000000..44e1a3a128 --- /dev/null +++ b/lms/djangoapps/django_comment_client/tests/test_permissions.py @@ -0,0 +1,128 @@ +import string +import random +import collections + +import factory +from django.test import TestCase + +from django.contrib.auth.models import User +from student.models import UserProfile, CourseEnrollment +from django_comment_client.models import Role, Permission +import django_comment_client.permissions as p + +class UserFactory(factory.Factory): + FACTORY_FOR = User + username = 'robot' + password = '123456' + email = 'robot@edx.org' + is_active = True + is_staff = False + +class CourseEnrollmentFactory(factory.Factory): + FACTORY_FOR = CourseEnrollment + user = factory.SubFactory(UserFactory) + course_id = 'edX/toy/2012_Fall' + +class RoleFactory(factory.Factory): + FACTORY_FOR = Role + name = 'Student' + course_id = 'edX/toy/2012_Fall' + +class PermissionFactory(factory.Factory): + FACTORY_FOR = Permission + name = 'create_comment' + + +class PermissionsTestCase(TestCase): + def setUp(self): + self.course_id = "edX/toy/2012_Fall" + + self.student_role = RoleFactory(name='Student') + self.moderator_role = RoleFactory(name='Moderator') + self.student = UserFactory(username='student', email='student@edx.org') + self.moderator = UserFactory(username='moderator', email='staff@edx.org', is_staff=True) + self.update_thread_permission = PermissionFactory(name='update_thread') + self.update_thread_permission.roles.add(self.student_role) + self.update_thread_permission.roles.add(self.moderator_role) + self.manage_moderator_permission = PermissionFactory(name='manage_moderator') + self.manage_moderator_permission.roles.add(self.moderator_role) + self.student_enrollment = CourseEnrollmentFactory(user=self.student) + self.moderator_enrollment = CourseEnrollmentFactory(user=self.moderator) + + self.student_open_thread = {'content': { + 'closed': False, + 'user_id': str(self.student.id)} + } + self.student_closed_thread = {'content': { + 'closed': True, + 'user_id': str(self.student.id)} + } + + def test_user_has_permission(self): + s_ut = p.has_permission(self.student, 'update_thread', self.course_id) + m_ut = p.has_permission(self.moderator, 'update_thread', self.course_id) + s_mm = p.has_permission(self.student, 'manage_moderator', self.course_id) + m_mm = p.has_permission(self.moderator, 'manage_moderator', self.course_id) + self.assertTrue(s_ut) + self.assertTrue(m_ut) + self.assertFalse(s_mm) + self.assertTrue(m_mm) + + def test_check_conditions(self): + # Checks whether the discussion thread is open, or whether the author is user + s_o = p.check_condition(self.student, 'is_open', self.course_id, self.student_open_thread) + s_a = p.check_condition(self.student, 'is_author', self.course_id, self.student_open_thread) + m_c = p.check_condition(self.moderator, 'is_open', self.course_id, self.student_closed_thread) + m_a = p.check_condition(self.moderator,'is_author', self.course_id, self.student_open_thread) + self.assertTrue(s_o) + self.assertTrue(s_a) + self.assertFalse(m_c) + self.assertFalse(m_a) + + def test_check_conditions_and_permissions(self): + # Check conditions + ret = p.check_conditions_permissions(self.student, + 'is_open', + self.course_id, + data=self.student_open_thread) + self.assertTrue(ret) + + # Check permissions + ret = p.check_conditions_permissions(self.student, + 'update_thread', + self.course_id, + data=self.student_open_thread) + self.assertTrue(ret) + + # Check that a list of permissions/conditions will be OR'd + ret = p.check_conditions_permissions(self.moderator, + ['is_open','manage_moderator'], + self.course_id, + data=self.student_open_thread) + self.assertTrue(ret) + + # Check that a list of permissions will be OR'd + ret = p.check_conditions_permissions(self.student, + ['update_thread','manage_moderator'], + self.course_id, + data=self.student_open_thread) + self.assertTrue(ret) + + # Check that a list of list of permissions will be AND'd + ret = p.check_conditions_permissions(self.student, + [['update_thread','manage_moderator']], + self.course_id, + data=self.student_open_thread) + self.assertFalse(ret) + + def test_check_permissions_by_view(self): + ret = p.check_permissions_by_view(self.student, self.course_id, + self.student_open_thread, 'openclose_thread') + self.assertFalse(ret) + + # Check a view permission that includes both a condition and a permission + self.vote_permission = PermissionFactory(name='vote') + self.vote_permission.roles.add(self.student_role) + ret = p.check_permissions_by_view(self.student, self.course_id, + self.student_open_thread, 'vote_for_comment') + self.assertTrue(ret) \ No newline at end of file