From 42f71156debfcb0735541ace593f8eb5c919d249 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 26 Jun 2013 14:42:17 -0400 Subject: [PATCH 1/7] Script for making all instructors content creators. --- cms/djangoapps/auth/authz.py | 7 ++++ cms/djangoapps/auth/tests/test_authz.py | 32 ++++++++++++++++++- .../management/commands/populate_creators.py | 14 ++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 cms/djangoapps/contentstore/management/commands/populate_creators.py diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index a544906875..169332e820 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -205,3 +205,10 @@ def is_user_in_creator_group(user): return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0 return True + + +def _grant_instructors_creator_access(caller): + for group in Group.objects.all(): + if group.name.startswith(INSTRUCTOR_ROLE_NAME + "_"): + for user in group.user_set.all(): + add_user_to_creator_group(caller, user) diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py index 173155df4c..511ac8ead8 100644 --- a/cms/djangoapps/auth/tests/test_authz.py +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -9,7 +9,7 @@ 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 + is_user_in_course_group_role, remove_user_from_course_group, _grant_instructors_creator_access class CreatorGroupTest(TestCase): @@ -174,3 +174,33 @@ class CourseGroupTest(TestCase): 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) + + +class GrantInstructorsCreatorAccessTest(TestCase): + + def create_course(self, index): + creator = User.objects.create_user('testcreator'+str(index), 'testcreator+courses@edx.org', 'foo') + staff = User.objects.create_user('teststaff'+str(index), 'teststaff+courses@edx.org', 'foo') + location = 'i4x', 'mitX', str(index), 'course', 'test' + create_all_course_groups(creator, location) + add_user_to_course_group(creator, staff, location, STAFF_ROLE_NAME) + return [creator, staff] + + def test_grant_creator_access(self): + [creator1, staff1] = self.create_course(1) + [creator2, staff2] = self.create_course(2) + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + self.assertFalse(is_user_in_creator_group(creator1)) + self.assertFalse(is_user_in_creator_group(creator2)) + self.assertFalse(is_user_in_creator_group(staff1)) + self.assertFalse(is_user_in_creator_group(staff2)) + + admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo') + admin.is_staff = True + _grant_instructors_creator_access(admin) + _grant_instructors_creator_access(admin) + + self.assertTrue(is_user_in_creator_group(creator1)) + self.assertTrue(is_user_in_creator_group(creator2)) + self.assertFalse(is_user_in_creator_group(staff1)) + self.assertFalse(is_user_in_creator_group(staff2)) diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py new file mode 100644 index 0000000000..e9453025a0 --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -0,0 +1,14 @@ +from auth.authz import _grant_instructors_creator_access +from django.core.management.base import BaseCommand + +from django.contrib.auth.models import User + + +class Command(BaseCommand): + help = 'Grants all users with INSTRUCTOR role permission to create courses' + + def handle(self, *args, **options): + admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo') + admin.is_staff = True + _grant_instructors_creator_access(admin) + admin.delete() From 3babf5392584350a2d34a0076b6bdaa4bb3bf521 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 09:44:09 -0400 Subject: [PATCH 2/7] Handle the case of script being terminated before the user was deleted. --- .../management/commands/populate_creators.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py index e9453025a0..28f360bacf 100644 --- a/cms/djangoapps/contentstore/management/commands/populate_creators.py +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -2,13 +2,23 @@ from auth.authz import _grant_instructors_creator_access from django.core.management.base import BaseCommand from django.contrib.auth.models import User +from django.db.utils import IntegrityError class Command(BaseCommand): help = 'Grants all users with INSTRUCTOR role permission to create courses' def handle(self, *args, **options): - admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo') - admin.is_staff = True + username = 'populate_creators_command' + email = 'grant+creator+access@edx.org' + try: + admin = User.objects.create_user(username, email, 'foo') + admin.is_staff = True + admin.save() + except IntegrityError: + # If the script did not complete the last time it was run, + # the admin user will already exist. + admin = User.objects.get(username=username, email=email) + _grant_instructors_creator_access(admin) admin.delete() From ce100bad8819ddcfb8649bfffd2e8c902ee31da4 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 09:48:47 -0400 Subject: [PATCH 3/7] Add doc strings. --- cms/djangoapps/auth/authz.py | 4 ++++ cms/djangoapps/auth/tests/test_authz.py | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 169332e820..bc11828b2a 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -208,6 +208,10 @@ def is_user_in_creator_group(user): def _grant_instructors_creator_access(caller): + """ + 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 + """ for group in Group.objects.all(): if group.name.startswith(INSTRUCTOR_ROLE_NAME + "_"): for user in group.user_set.all(): diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py index 511ac8ead8..a615a78bbc 100644 --- a/cms/djangoapps/auth/tests/test_authz.py +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -177,8 +177,13 @@ class CourseGroupTest(TestCase): class GrantInstructorsCreatorAccessTest(TestCase): - + """ + Tests granting existing instructors course creator rights. + """ def create_course(self, index): + """ + Creates a course with one instructor and one staff member. + """ creator = User.objects.create_user('testcreator'+str(index), 'testcreator+courses@edx.org', 'foo') staff = User.objects.create_user('teststaff'+str(index), 'teststaff+courses@edx.org', 'foo') location = 'i4x', 'mitX', str(index), 'course', 'test' @@ -187,9 +192,13 @@ class GrantInstructorsCreatorAccessTest(TestCase): return [creator, staff] def test_grant_creator_access(self): + """ + Test for _grant_instructors_creator_access. + """ [creator1, staff1] = self.create_course(1) [creator2, staff2] = self.create_course(2) with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + # Initially no creators. self.assertFalse(is_user_in_creator_group(creator1)) self.assertFalse(is_user_in_creator_group(creator2)) self.assertFalse(is_user_in_creator_group(staff1)) @@ -198,9 +207,10 @@ class GrantInstructorsCreatorAccessTest(TestCase): admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo') admin.is_staff = True _grant_instructors_creator_access(admin) - _grant_instructors_creator_access(admin) + # Now instructors only are creators. self.assertTrue(is_user_in_creator_group(creator1)) self.assertTrue(is_user_in_creator_group(creator2)) self.assertFalse(is_user_in_creator_group(staff1)) self.assertFalse(is_user_in_creator_group(staff2)) + From 0e0f22999d9c12236b5cee755a562f5cd3a2f81c Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 09:52:18 -0400 Subject: [PATCH 4/7] Add doc strings. --- .../management/commands/populate_creators.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py index 28f360bacf..41c5d8194a 100644 --- a/cms/djangoapps/contentstore/management/commands/populate_creators.py +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -1,3 +1,6 @@ +""" +Script for granting existing course instructors course creator privileges. +""" from auth.authz import _grant_instructors_creator_access from django.core.management.base import BaseCommand @@ -6,9 +9,15 @@ from django.db.utils import IntegrityError class Command(BaseCommand): + """ + Script for granting existing course instructors course creator privileges. + """ help = 'Grants all users with INSTRUCTOR role permission to create courses' def handle(self, *args, **options): + """ + The logic of the command. + """ username = 'populate_creators_command' email = 'grant+creator+access@edx.org' try: From 27e720cf3b8d8802524381e61b9f94ea64b81d79 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 11:00:11 -0400 Subject: [PATCH 5/7] Make it clear that this should only be run once. --- cms/djangoapps/auth/authz.py | 5 ++++- .../contentstore/management/commands/populate_creators.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index bc11828b2a..438c9d0697 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -210,7 +210,10 @@ def is_user_in_creator_group(user): def _grant_instructors_creator_access(caller): """ 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 + asserted permissions to do this action. + + Gives all users with instructor role course creator rights. + This is only intended to be run once on a given environment. """ for group in Group.objects.all(): if group.name.startswith(INSTRUCTOR_ROLE_NAME + "_"): diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py index 41c5d8194a..f627df88f5 100644 --- a/cms/djangoapps/contentstore/management/commands/populate_creators.py +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -1,5 +1,7 @@ """ Script for granting existing course instructors course creator privileges. + +This script is only intended to be run once on a given environment. """ from auth.authz import _grant_instructors_creator_access from django.core.management.base import BaseCommand From f095c5fec6507ba38552b0e02530e8ce981ffcbe Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 11:00:43 -0400 Subject: [PATCH 6/7] Add ENABLE_CREATOR_GROUP (set to False). --- cms/envs/common.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 7f4c106e6d..87c130a4b5 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -54,7 +54,11 @@ MITX_FEATURES = { 'ENABLE_SERVICE_STATUS': False, # Don't autoplay videos for course authors - 'AUTOPLAY_VIDEOS': False + 'AUTOPLAY_VIDEOS': False, + + # If set to True, new Studio users won't be able to author courses unless + # edX has explicitly added them to the course creator group. + 'ENABLE_CREATOR_GROUP': False } ENABLE_JASMINE = False From 32e6d4819f5a4d1f879b3b33e3b1f81940ea2736 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 27 Jun 2013 16:18:16 -0400 Subject: [PATCH 7/7] pep8/pylint cleanup. --- cms/djangoapps/auth/authz.py | 4 +++- cms/djangoapps/auth/tests/test_authz.py | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 438c9d0697..23dde88e94 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -36,7 +36,7 @@ def get_course_groupname_for_role(location, role): def get_users_in_course_group_by_role(location, role): groupname = get_course_groupname_for_role(location, role) - (group, created) = Group.objects.get_or_create(name=groupname) + (group, _created) = Group.objects.get_or_create(name=groupname) return group.user_set.all() @@ -59,6 +59,7 @@ 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 @@ -75,6 +76,7 @@ def _delete_course_group(location): user.groups.remove(staff) 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 diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py index a615a78bbc..658c176498 100644 --- a/cms/djangoapps/auth/tests/test_authz.py +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -184,8 +184,8 @@ class GrantInstructorsCreatorAccessTest(TestCase): """ Creates a course with one instructor and one staff member. """ - creator = User.objects.create_user('testcreator'+str(index), 'testcreator+courses@edx.org', 'foo') - staff = User.objects.create_user('teststaff'+str(index), 'teststaff+courses@edx.org', 'foo') + creator = User.objects.create_user('testcreator' + str(index), 'testcreator+courses@edx.org', 'foo') + staff = User.objects.create_user('teststaff' + str(index), 'teststaff+courses@edx.org', 'foo') location = 'i4x', 'mitX', str(index), 'course', 'test' create_all_course_groups(creator, location) add_user_to_course_group(creator, staff, location, STAFF_ROLE_NAME) @@ -213,4 +213,3 @@ class GrantInstructorsCreatorAccessTest(TestCase): self.assertTrue(is_user_in_creator_group(creator2)) self.assertFalse(is_user_in_creator_group(staff1)) self.assertFalse(is_user_in_creator_group(staff2)) -