New course creator group.

cms/djangoapps/auth/authz.py

@@ -1,5 +1,6 @@
 from django.contrib.auth.models import User, Group
 from django.core.exceptions import PermissionDenied
+from django.conf import settings
 
 from xmodule.modulestore import Location
 
@@ -12,6 +13,9 @@
 INSTRUCTOR_ROLE_NAME = 'instructor'
 STAFF_ROLE_NAME = 'staff'
 
+# This is the group of people who have permission to create new courses on edge or edx.
+COURSE_CREATOR_GROUP_NAME = "course_creator_group"
+
 # we're just making a Django group for each location/role combo
 # to do this we're just creating a Group name which is a formatted string
 # of those two variables
@@ -36,10 +40,10 @@ def get_users_in_course_group_by_role(location, role):
     return group.user_set.all()
 
 
-'''
-Create all permission groups for a new course and subscribe the caller into those roles
-'''
 def create_all_course_groups(creator, location):
+    """
+    Create all permission groups for a new course and subscribe the caller into those roles
+    """
     create_new_course_group(creator, location, INSTRUCTOR_ROLE_NAME)
     create_new_course_group(creator, location, STAFF_ROLE_NAME)
 
@@ -56,10 +60,10 @@ def create_new_course_group(creator, location, role):
     return
 
 def _delete_course_group(location):
-    '''
+    """
     This is to be called only by either a command line code path or through a app which has already
     asserted permissions
-    '''
+    """
     # remove all memberships
     instructors = Group.objects.get(name=get_course_groupname_for_role(location, INSTRUCTOR_ROLE_NAME))
     for user in instructors.user_set.all():
@@ -72,10 +76,10 @@ def _delete_course_group(location):
         user.save()
 
 def _copy_course_group(source, dest):
-    '''
+    """
     This is to be called only by either a command line code path or through an app which has already
     asserted permissions to do this action
-    '''
+    """
     instructors = Group.objects.get(name=get_course_groupname_for_role(source, INSTRUCTOR_ROLE_NAME))
     new_instructors_group = Group.objects.get(name=get_course_groupname_for_role(dest, INSTRUCTOR_ROLE_NAME))
     for user in instructors.user_set.all():
@@ -94,10 +98,34 @@ def add_user_to_course_group(caller, user, location, role):
     if not is_user_in_course_group_role(caller, location, INSTRUCTOR_ROLE_NAME):
         raise PermissionDenied
 
-    if user.is_active and user.is_authenticated:
-        groupname = get_course_groupname_for_role(location, role)
+    group = Group.objects.get(name=get_course_groupname_for_role(location, role))
+    return _add_user_to_group(user, group)
+
+
+def add_user_to_creator_group(caller, user):
+    """
+    Adds the user to the group of course creators.
+
+    The caller must have staff access to perform this operation.
+
+    Note that on the edX site, we currently limit course creators to edX staff, and this
+    method is a no-op in that environment.
+    """
+    if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
+        raise PermissionDenied
 
-        group = Group.objects.get(name=groupname)
+    (group, created) = Group.objects.get_or_create(name=COURSE_CREATOR_GROUP_NAME)
+    if created:
+        group.save()
+    return _add_user_to_group(user, group)
+
+
+def _add_user_to_group(user, group):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    if user.is_active and user.is_authenticated:
         user.groups.add(group)
         user.save()
         return True
@@ -123,11 +151,29 @@ def remove_user_from_course_group(caller, user, location, role):
 
     # see if the user is actually in that role, if not then we don't have to do anything
     if is_user_in_course_group_role(user, location, role):
-        groupname = get_course_groupname_for_role(location, role)
+        _remove_user_from_group(user, get_course_groupname_for_role(location, role))
 
-        group = Group.objects.get(name=groupname)
-        user.groups.remove(group)
-        user.save()
+
+def remove_user_from_creator_group(caller, user):
+    """
+    Removes user from the course creator group.
+
+    The caller must have staff access to perform this operation.
+    """
+    if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
+        raise PermissionDenied
+
+    _remove_user_from_group(user, COURSE_CREATOR_GROUP_NAME)
+
+
+def _remove_user_from_group(user, group_name):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    group = Group.objects.get(name=group_name)
+    user.groups.remove(group)
+    user.save()
 
 
 def is_user_in_course_group_role(user, location, role):
@@ -136,3 +182,26 @@ def is_user_in_course_group_role(user, location, role):
         return user.is_staff or user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0
 
     return False
+
+
+def is_user_in_creator_group(user):
+    """
+    Returns true if the user has permissions to create a course.
+
+    Will always return True if user.is_staff is True.
+
+    Note that on the edX site, we currently limit course creators to edX staff. On
+    other sites, this method checks that the user is in the course creator group.
+    """
+    if user.is_staff:
+        return True
+
+    # On edx, we only allow edX staff to create courses. This may be relaxed in the future.
+    if settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False):
+        return False
+
+    # Feature flag for using the creator group setting. Will be removed once the feature is complete.
+    if settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False):
+        return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0
+
+    return True

cms/djangoapps/auth/tests/test_authz.py

@@ -0,0 +1,176 @@
+"""
+Tests authz.py
+"""
+import mock
+
+from django.test import TestCase
+from django.contrib.auth.models import User
+from django.core.exceptions import PermissionDenied
+
+from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, is_user_in_creator_group,\
+    create_all_course_groups, add_user_to_course_group, STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,\
+    is_user_in_course_group_role, remove_user_from_course_group
+
+
+class CreatorGroupTest(TestCase):
+    """
+    Tests for the course creator group.
+    """
+
+    def setUp(self):
+        """ Test case setup """
+        self.user = User.objects.create_user('testuser', 'test+courses@edx.org', 'foo')
+        self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo')
+        self.admin.is_staff = True
+
+    def test_creator_group_not_enabled(self):
+        """
+        Tests that is_user_in_creator_group always returns True if ENABLE_CREATOR_GROUP
+        and DISABLE_COURSE_CREATION are both not turned on.
+        """
+        self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_creator_group_enabled_but_empty(self):
+        """ Tests creator group feature on, but group empty. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+            # Make user staff. This will cause is_user_in_creator_group to return True.
+            self.user.is_staff = True
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_creator_group_enabled_nonempty(self):
+        """ Tests creator group feature on, user added. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertTrue(add_user_to_creator_group(self.admin, self.user))
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+            # check that a user who has not been added to the group still returns false
+            user_not_added = User.objects.create_user('testuser2', 'test+courses2@edx.org', 'foo2')
+            self.assertFalse(is_user_in_creator_group(user_not_added))
+
+            # remove first user from the group and verify that is_user_in_creator_group now returns false
+            remove_user_from_creator_group(self.admin, self.user)
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+    def test_add_user_not_authenticated(self):
+        """
+        Tests that adding to creator group fails if user is not authenticated
+        """
+        self.user.is_authenticated = False
+        self.assertFalse(add_user_to_creator_group(self.admin, self.user))
+
+    def test_add_user_not_active(self):
+        """
+        Tests that adding to creator group fails if user is not active
+        """
+        self.user.is_active = False
+        self.assertFalse(add_user_to_creator_group(self.admin, self.user))
+
+    def test_course_creation_disabled(self):
+        """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES',
+                             {'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}):
+            # Add user to creator group.
+            self.assertTrue(add_user_to_creator_group(self.admin, self.user))
+
+            # DISABLE_COURSE_CREATION overrides (user is not marked as staff).
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+            # Mark as staff. Now is_user_in_creator_group returns true.
+            self.user.is_staff = True
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+            # Remove user from creator group. is_user_in_creator_group still returns true because is_staff=True
+            remove_user_from_creator_group(self.admin, self.user)
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_add_user_to_group_requires_staff_access(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_staff = False
+            add_user_to_creator_group(self.admin, self.user)
+
+        with self.assertRaises(PermissionDenied):
+            add_user_to_creator_group(self.user, self.user)
+
+    def test_add_user_to_group_requires_active(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_active = False
+            add_user_to_creator_group(self.admin, self.user)
+
+    def test_add_user_to_group_requires_authenticated(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_authenticated = False
+            add_user_to_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_staff_access(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_staff = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_active(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_active = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_authenticated(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_authenticated = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+
+class CourseGroupTest(TestCase):
+    """
+    Tests for instructor and staff groups for a particular course.
+    """
+
+    def setUp(self):
+        """ Test case setup """
+        self.creator = User.objects.create_user('testcreator', 'testcreator+courses@edx.org', 'foo')
+        self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo')
+        self.location = 'i4x', 'mitX', '101', 'course', 'test'
+
+    def test_add_user_to_course_group(self):
+        """
+        Tests adding user to course group (happy path).
+        """
+        # Create groups for a new course (and assign instructor role to the creator).
+        self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+        create_all_course_groups(self.creator, self.location)
+        self.assertTrue(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+
+        # Add another user to the staff role.
+        self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+    def test_add_user_to_course_group_permission_denied(self):
+        """
+        Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role.
+        """
+        create_all_course_groups(self.creator, self.location)
+        with self.assertRaises(PermissionDenied):
+            add_user_to_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME)
+
+    def test_remove_user_from_course_group(self):
+        """
+        Tests removing user from course group (happy path).
+        """
+        create_all_course_groups(self.creator, self.location)
+
+        self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+        remove_user_from_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)
+        self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+        remove_user_from_course_group(self.creator, self.creator, self.location, INSTRUCTOR_ROLE_NAME)
+        self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+
+    def test_remove_user_from_course_group_permission_denied(self):
+        """
+        Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role.
+        """
+        create_all_course_groups(self.creator, self.location)
+        with self.assertRaises(PermissionDenied):
+            remove_user_from_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME)