From 36ea6df506ba2ed03dfc168fa85953cd71141400 Mon Sep 17 00:00:00 2001 From: Farhaan Bukhsh Date: Wed, 27 Oct 2021 17:56:14 +0530 Subject: [PATCH] feat: grant course/library creation rights by organization (#26616) Current State (before this commit): Studio, as of today doesn't have a way to restrict a user to create a course in a particular organization. What Studio provides right now is a CourseCreator permission which gives an Admin the power to grant a user the permission to create a course. For example: If the Admin has given a user Spiderman the permission to create courses, Spiderman can now create courses in any organization i.e Marvel as well as DC. There is no way to restrict Spiderman from creating courses under DC. Purpose of this commit: The changes done here gives Admin the ability to restrict a user on an Organization level from creating courses via the Course Creators section of the Studio Django administration panel. For example: Now, the Admin can give the user Spiderman the privilege of creating courses only under Marvel organization. The moment Spiderman tries to create a course under some other organization(i.e DC), Studio will show an error message. This change is available to all Studio instances that enable the FEATURES['ENABLE_CREATOR_GROUP'] flag. Regardless of the flag, it will not affect any instances that choose not to use it. BB-3622 Signed-off-by: Farhaan Bukhsh --- .../tests/test_course_create_rerun.py | 163 ++++++++++++------ .../contentstore/tests/test_libraries.py | 72 -------- cms/djangoapps/contentstore/views/course.py | 77 +++++---- cms/djangoapps/contentstore/views/helpers.py | 20 ++- cms/djangoapps/contentstore/views/library.py | 50 ++++-- .../contentstore/views/tests/test_library.py | 14 +- cms/djangoapps/course_creators/admin.py | 77 ++++++++- ...002_add_org_support_for_course_creators.py | 24 +++ cms/djangoapps/course_creators/models.py | 21 ++- .../course_creators/tests/test_views.py | 12 +- cms/djangoapps/course_creators/views.py | 12 +- cms/envs/common.py | 15 -- common/djangoapps/student/auth.py | 33 +++- common/djangoapps/student/roles.py | 16 ++ common/djangoapps/student/tests/test_authz.py | 50 +++++- common/djangoapps/student/tests/test_roles.py | 16 +- 16 files changed, 447 insertions(+), 225 deletions(-) create mode 100644 cms/djangoapps/course_creators/migrations/0002_add_org_support_for_course_creators.py diff --git a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py index bd83edf4137d..e1a0f51c2582 100644 --- a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py +++ b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py @@ -4,25 +4,37 @@ import datetime +from unittest import mock import ddt import six +from django.contrib.admin.sites import AdminSite +from django.http import HttpRequest +from django.test import override_settings from django.test.client import RequestFactory from django.urls import reverse from mock import patch from opaque_keys.edx.keys import CourseKey - -from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, parse_json -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole -from common.djangoapps.student.models import CourseAccessRole -from common.djangoapps.student.tests.factories import UserFactory -from common.djangoapps.util.organizations_helpers import add_organization, get_course_organizations +from organizations.api import add_organization, get_course_organizations, get_organization_by_short_name +from organizations.models import Organization from xmodule.course_module import CourseFields from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from cms.djangoapps.course_creators.admin import CourseCreatorAdmin +from cms.djangoapps.course_creators.models import CourseCreator +from common.djangoapps.student.auth import update_org_role +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, OrgContentCreatorRole +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory + + +def mock_render_to_string(template_name, context): + """Return a string that encodes template_name and context""" + return str((template_name, context)) + @ddt.ddt class TestCourseListing(ModuleStoreTestCase): @@ -35,12 +47,10 @@ def setUp(self): """ super(TestCourseListing, self).setUp() # create and log in a staff user. - self.admin_user = UserFactory(is_staff=True) - self.admin_client = AjaxEnabledTestClient() - self.admin_client.login(username=self.admin_user.username, password='test') # create and log in a non-staff user self.user = UserFactory() self.factory = RequestFactory() + self.global_admin = AdminFactory() self.client = AjaxEnabledTestClient() self.client.login(username=self.user.username, password='test') self.course_create_rerun_url = reverse('course_handler') @@ -60,6 +70,12 @@ def setUp(self): ) self.source_course_key = source_course.id + self.course_creator_entry = CourseCreator(user=self.user) + self.course_creator_entry.save() + self.request = HttpRequest() + self.request.user = self.global_admin + self.creator_admin = CourseCreatorAdmin(self.course_creator_entry, AdminSite()) + for role in [CourseInstructorRole, CourseStaffRole]: role(self.source_course_key).add_users(self.user) @@ -68,7 +84,6 @@ def tearDown(self): Reverse the setup """ self.client.logout() - self.admin_client.logout() ModuleStoreTestCase.tearDown(self) @patch.dict('django.conf.settings.FEATURES', {'ORGANIZATIONS_APP': True}) @@ -182,11 +197,11 @@ def test_course_creation_with_org_in_system(self, store): self.assertEqual(len(course_orgs), 1) self.assertEqual(course_orgs[0]['short_name'], 'orgX') - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) + @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_creation_when_user_not_in_org(self, store): """ - Tests course creation with restriction and user not registered in CourseAccessRole. + Tests course creation when user doesn't have the required role. """ with modulestore().default_store(store): response = self.client.ajax_post(self.course_create_rerun_url, { @@ -195,67 +210,117 @@ def test_course_creation_when_user_not_in_org(self, store): 'display_name': 'Course with web certs enabled', 'run': '2021_T1' }) - self.assertEqual(response.status_code, 400) - data = parse_json(response) - self.assertEqual( - data["error"], - 'User does not have the permission to create courses in this organization' - ) + self.assertEqual(response.status_code, 403) - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) + @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_creation_when_user_in_org(self, store): + def test_course_creation_when_user_in_org_with_creator_role(self, store): """ - Tests course creation with restriction and user registered as staff. + Tests course creation with user having the organization content creation role. """ - staff_role = 'staff' - CourseAccessRole.objects.create( - org='TestorgX', role=staff_role, user=self.user - ) + add_organization({ + 'name': 'Test Organization', + 'short_name': self.source_course_key.org, + 'description': 'Testing Organization Description', + }) + update_org_role(self.global_admin, OrgContentCreatorRole, self.user, [self.source_course_key.org]) with modulestore().default_store(store): response = self.client.ajax_post(self.course_create_rerun_url, { - 'org': 'TestorgX', + 'org': self.source_course_key.org, 'number': 'CS101', 'display_name': 'Course with web certs enabled', 'run': '2021_T1' }) self.assertEqual(response.status_code, 200) - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) + @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_creation_when_user_in_org_with_non_access_role(self, store): + @mock.patch( + 'cms.djangoapps.course_creators.admin.render_to_string', + mock.Mock(side_effect=mock_render_to_string, autospec=True) + ) + def test_course_creation_with_all_org_checked(self, store): """ - Tests course creation with restriction and user registered as role who doesn't have the access. + Tests course creation with user having permission to create course for all organization. """ - staff_role = 'finance_admin' - CourseAccessRole.objects.create( - org='Stark', role=staff_role, user=self.user - ) + add_organization({ + 'name': 'Test Organization', + 'short_name': self.source_course_key.org, + 'description': 'Testing Organization Description', + }) + self.course_creator_entry.all_organizations = True + self.course_creator_entry.state = CourseCreator.GRANTED + self.creator_admin.save_model(self.request, self.course_creator_entry, None, True) with modulestore().default_store(store): response = self.client.ajax_post(self.course_create_rerun_url, { - 'org': 'Stark', - 'number': 'AV101', - 'display_name': 'Build Iron Man Suit', + 'org': self.source_course_key.org, + 'number': 'CS101', + 'display_name': 'Course with web certs enabled', 'run': '2021_T1' }) - self.assertEqual(response.status_code, 400) - data = parse_json(response) - self.assertEqual( - data["error"], - 'User does not have the permission to create courses in this organization' - ) + self.assertEqual(response.status_code, 200) - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) + @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_creation_when_user_is_global_staff(self, store): + @mock.patch( + 'cms.djangoapps.course_creators.admin.render_to_string', + mock.Mock(side_effect=mock_render_to_string, autospec=True) + ) + def test_course_creation_with_permission_for_specific_organization(self, store): """ - Tests course creation with restriction and user is global staff. + Tests course creation with user having permission to create course for specific organization. """ + add_organization({ + 'name': 'Test Organization', + 'short_name': self.source_course_key.org, + 'description': 'Testing Organization Description', + }) + self.course_creator_entry.all_organizations = False + self.course_creator_entry.state = CourseCreator.GRANTED + self.creator_admin.save_model(self.request, self.course_creator_entry, None, True) + dc_org_object = Organization.objects.get(name='Test Organization') + self.course_creator_entry.organizations.add(dc_org_object) with modulestore().default_store(store): - response = self.admin_client.ajax_post(self.course_create_rerun_url, { - 'org': 'Oscorp', - 'number': 'SP101', - 'display_name': 'Making better web', + response = self.client.ajax_post(self.course_create_rerun_url, { + 'org': self.source_course_key.org, + 'number': 'CS101', + 'display_name': 'Course with web certs enabled', 'run': '2021_T1' }) self.assertEqual(response.status_code, 200) + + @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + @mock.patch( + 'cms.djangoapps.course_creators.admin.render_to_string', + mock.Mock(side_effect=mock_render_to_string, autospec=True) + ) + def test_course_creation_without_permission_for_specific_organization(self, store): + """ + Tests course creation with user not having permission to create course for specific organization. + """ + add_organization({ + 'name': 'Test Organization', + 'short_name': self.source_course_key.org, + 'description': 'Testing Organization Description', + }) + add_organization({ + 'name': 'DC', + 'short_name': 'DC', + 'description': 'DC Comics', + }) + self.course_creator_entry.all_organizations = False + self.course_creator_entry.state = CourseCreator.GRANTED + self.creator_admin.save_model(self.request, self.course_creator_entry, None, True) + # User has been given the permission to create course under `DC` organization. + # When the user tries to create course under `Test Organization` it throws a 403. + dc_org_object = Organization.objects.get(name='DC') + self.course_creator_entry.organizations.add(dc_org_object) + with modulestore().default_store(store): + response = self.client.ajax_post(self.course_create_rerun_url, { + 'org': self.source_course_key.org, + 'number': 'CS101', + 'display_name': 'Course with web certs enabled', + 'run': '2021_T1' + }) + self.assertEqual(response.status_code, 403) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 52ed6fa2a368..f38776491659 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -26,7 +26,6 @@ OrgLibraryUserRole, OrgStaffRole ) -from common.djangoapps.student.models import CourseAccessRole from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from xmodule.modulestore import ModuleStoreEnum @@ -830,77 +829,6 @@ def _get_settings_html(): self.assertNotIn('admin_lib_2', non_staff_settings_html) - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) - def test_library_creation_when_user_is_global_staff(self): - """ - Tests course creation with restriction and user is global staff. - """ - self._login_as_staff_user() - response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'Oscorp', - 'library': 'CentralLibrary', - 'display_name': 'Making better web', - }) - self.assertEqual(response.status_code, 200) - - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) - def test_library_creation_with_normaL_user_with_no_role(self): - """ - Tests course creation with restriction and user is not a global staff. - """ - self._login_as_non_staff_user() - response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'Stark', - 'library': 'AvengerLibrary', - 'display_name': 'Alien Science', - }) - self.assertEqual(response.status_code, 400) - data = parse_json(response) - self.assertEqual( - data["ErrMsg"], - "User does not have the permission to create library in this organization" - ) - - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) - def test_library_creation_with_normaL_user_with_non_access_role(self): - """ - Tests course creation with restriction and user doesn't have access role for org. - """ - staff_role = "finance_admin" - self._login_as_non_staff_user() - CourseAccessRole.objects.create( - org='Stark', role=staff_role, user=self.non_staff_user - ) - response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'Stark', - 'library': 'AvengerLibrary', - 'display_name': 'Alien Science', - }) - self.assertEqual(response.status_code, 400) - data = parse_json(response) - self.assertEqual( - data["ErrMsg"], - "User does not have the permission to create library in this organization" - ) - - @patch.dict('django.conf.settings.FEATURES', {'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': True}) - def test_library_creation_with_normaL_user_with_role(self): - """ - Tests course creation with restriction and user has role access. - """ - staff_role = "instructor" - self._login_as_non_staff_user() - CourseAccessRole.objects.create( - org='Stark', role=staff_role, user=self.non_staff_user - ) - response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'Stark', - 'library': 'AvengerLibrary', - 'display_name': 'Alien Science', - }) - self.assertEqual(response.status_code, 200) - - @ddt.ddt @override_settings(SEARCH_ENGINE=None) class TestOverrides(LibraryTestCase): diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 4183ccc6425a..0dfb4903d0a4 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -51,10 +51,12 @@ from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML from openedx.features.course_experience.waffle import waffle as course_experience_waffle -from common.djangoapps.student import auth from common.djangoapps.student.auth import has_course_author_access, has_studio_read_access, has_studio_write_access from common.djangoapps.student.roles import ( - CourseCreatorRole, CourseInstructorRole, CourseStaffRole, GlobalStaff, UserBasedRole + CourseInstructorRole, + CourseStaffRole, + GlobalStaff, + UserBasedRole ) from common.djangoapps.util.course import get_link_for_about_page from common.djangoapps.util.date_utils import get_default_time_display @@ -102,12 +104,13 @@ reverse_usage_url ) from .component import ADVANCED_COMPONENT_TYPES +from .helpers import is_content_creator from .entrance_exam import create_entrance_exam, delete_entrance_exam, update_entrance_exam from .item import create_xblock_info from .library import ( LIBRARIES_ENABLED, LIBRARY_AUTHORING_MICROFRONTEND_URL, - get_library_creator_status, + user_can_create_library, should_redirect_to_library_authoring_mfe ) @@ -558,22 +561,23 @@ def format_library_for_view(library): active_courses, archived_courses = _process_courses_list(courses_iter, in_process_course_actions, split_archived) in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] - return render_to_response(u'index.html', { - u'courses': active_courses, - u'archived_courses': archived_courses, - u'in_process_course_actions': in_process_course_actions, - u'libraries_enabled': LIBRARIES_ENABLED, - u'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), - u'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, - u'libraries': [format_library_for_view(lib) for lib in libraries], - u'show_new_library_button': get_library_creator_status(user) and not should_redirect_to_library_authoring_mfe(), - u'user': user, - u'request_course_creator_url': reverse('request_course_creator'), - u'course_creator_status': _get_course_creator_status(user), - u'rerun_creator_status': GlobalStaff().has_user(user), - u'allow_unicode_course_id': settings.FEATURES.get(u'ALLOW_UNICODE_COURSE_ID', False), - u'allow_course_reruns': settings.FEATURES.get(u'ALLOW_COURSE_RERUNS', True), - u'optimization_enabled': optimization_enabled + return render_to_response('index.html', { + 'courses': active_courses, + 'archived_courses': archived_courses, + 'in_process_course_actions': in_process_course_actions, + 'libraries_enabled': LIBRARIES_ENABLED, + 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), + 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, + 'libraries': [format_library_for_view(lib) for lib in libraries], + 'show_new_library_button': user_can_create_library(user) and not should_redirect_to_library_authoring_mfe(), + 'user': user, + 'request_course_creator_url': reverse('request_course_creator'), + 'course_creator_status': _get_course_creator_status(user), + 'rerun_creator_status': GlobalStaff().has_user(user), + 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), + 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), + 'optimization_enabled': optimization_enabled, + 'active_tab': 'courses' }) @@ -812,9 +816,6 @@ def _create_or_rerun_course(request): Returns the destination course_key and overriding fields for the new course. Raises DuplicateCourseError and InvalidKeyError """ - if not auth.user_has_role(request.user, CourseCreatorRole()): - raise PermissionDenied() - try: org = request.json.get('org') course = request.json.get('number', request.json.get('course')) @@ -822,6 +823,10 @@ def _create_or_rerun_course(request): # force the start date for reruns and allow us to override start via the client start = request.json.get('start', CourseFields.start.default) run = request.json.get('run') + has_course_creator_role = is_content_creator(request.user, org) + + if not has_course_creator_role: + raise PermissionDenied() # allow/disable unicode characters in course_id according to settings if not settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID'): @@ -831,20 +836,6 @@ def _create_or_rerun_course(request): status=400 ) - # Allow user to create the course only if they belong to the organisation - # This flag doesn't apply to Global Staff and Superusers - if settings.FEATURES.get('RESTRICT_COURSE_CREATION_TO_ORG_ROLES', False): - has_org_permission = has_studio_write_access(request.user, None, org) - if not has_org_permission: - log.exception( - "User does not have the permission to create course in this organization." - "User: {} Org: {} Course: {}".format(request.user.id, org, course) - ) - return JsonResponse( - {'error': _('User does not have the permission to create courses in this organization')}, - status=400 - ) - fields = {'start': start} if display_name is not None: fields['display_name'] = display_name @@ -892,6 +883,20 @@ def _create_or_rerun_course(request): return JsonResponse({ "ErrMsg": _(u"Unable to create course '{name}'.\n\n{err}").format(name=display_name, err=text_type(error))} ) + except PermissionDenied as error: + log.info( + "User does not have the permission to create course in this organization" + "or course creation is disabled." + "User: '%s' Org: '%s' Course #: '%s'.", + request.user.id, + org, + course, + ) + return JsonResponse({ + 'error': _('User does not have the permission to create courses in this organization ' + 'or course creation is disabled')}, + status=403 + ) def create_new_course(user, org, number, run, fields): diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index bf563b412bbc..0949f8d191c2 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -10,12 +10,14 @@ from django.utils.translation import ugettext as _ from opaque_keys.edx.keys import UsageKey from xblock.core import XBlock +from xmodule.modulestore.django import modulestore +from xmodule.tabs import StaticTab from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from openedx.core.toggles import ENTRANCE_EXAMS from common.djangoapps.edxmako.shortcuts import render_to_string -from xmodule.modulestore.django import modulestore -from xmodule.tabs import StaticTab +from common.djangoapps.student import auth +from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole +from openedx.core.toggles import ENTRANCE_EXAMS from ..utils import reverse_course_url, reverse_library_url, reverse_usage_url @@ -290,3 +292,15 @@ def is_item_in_course_tree(item): ancestor = ancestor.get_parent() return ancestor is not None + + +def is_content_creator(user, org): + """ + Check if the user has the role to create content. + + This function checks if the User has role to create content + or if the org is supplied, it checks for Org level course content + creator. + """ + return (auth.user_has_role(user, CourseCreatorRole()) or + auth.user_has_role(user, OrgContentCreatorRole(org=org))) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8cb496d4e1a5..167e4d773ca3 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -19,6 +19,10 @@ from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator from six import text_type +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import DuplicateCourseError + from cms.djangoapps.course_creators.views import get_course_creator_status from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.student.auth import ( @@ -28,15 +32,17 @@ has_studio_read_access, has_studio_write_access ) -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + LibraryUserRole +) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import DuplicateCourseError from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url from .component import CONTAINER_TEMPLATES, get_component_templates +from .helpers import is_content_creator from .item import create_xblock_info from .user import user_with_role @@ -62,7 +68,7 @@ def should_redirect_to_library_authoring_mfe(): ) -def get_library_creator_status(user): +def user_can_create_library(user, org=None): """ Helper method for returning the library creation status for a particular user, taking into account the value LIBRARIES_ENABLED. @@ -73,7 +79,10 @@ def get_library_creator_status(user): elif user.is_staff: return True elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - return get_course_creator_status(user) == 'granted' + has_course_creator_role = True + if org: + has_course_creator_role = is_content_creator(user, org) + return get_course_creator_status(user) == 'granted' and has_course_creator_role else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) @@ -96,7 +105,7 @@ def library_handler(request, library_key_string=None): raise Http404 # Should never happen because we test the feature in urls.py also if request.method == 'POST': - if not get_library_creator_status(request.user): + if not user_can_create_library(request.user): return HttpResponseForbidden() if library_key_string is not None: @@ -186,18 +195,8 @@ def _create_library(request): library = request.json.get('number', None) if library is None: library = request.json['library'] - # Allow user to create libraries only if they belong to the organization - # This flag doesn't apply to Global Staff and Superusers - if settings.FEATURES.get('RESTRICT_COURSE_CREATION_TO_ORG_ROLES', False): - has_org_permission = has_studio_write_access(request.user, None, org) - if not has_org_permission: - log.exception( - "User does not have the permission to create library in this organization." - "User: {} Org: {} Library: {}".format(request.user.id, org, library) - ) - return JsonResponseBadRequest({ - "ErrMsg": _(u"User does not have the permission to create library in this organization") - }) + if not user_can_create_library(request.user, org): + raise PermissionDenied() store = modulestore() with store.default_store(ModuleStoreEnum.Type.split): new_lib = store.create_library( @@ -208,6 +207,19 @@ def _create_library(request): ) # Give the user admin ("Instructor") role for this library: add_instructor(new_lib.location.library_key, request.user, request.user) + except PermissionDenied as error: + log.info( + "User does not have the permission to create LIBRARY in this organization." + "User: '%s' Org: '%s' LIBRARY #: '%s'.", + request.user.id, + org, + library, + ) + return JsonResponse({ + 'error': _('User does not have the permission to create library in this organization ' + 'or course creation is disabled')}, + status=403 + ) except KeyError as error: log.exception("Unable to create library - missing required JSON key.") return JsonResponseBadRequest({ diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 96d3f68012fa..17f7467ef667 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -21,7 +21,7 @@ from xmodule.modulestore.tests.factories import LibraryFactory from ..component import get_component_templates -from ..library import get_library_creator_status +from ..library import user_can_create_library LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries @@ -52,23 +52,23 @@ def setUp(self): @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", False) def test_library_creator_status_libraries_not_enabled(self): _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(get_library_creator_status(nostaff_user), False) + self.assertEqual(user_can_create_library(nostaff_user), False) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_is_staff_user(self): - self.assertEqual(get_library_creator_status(self.user), True) + self.assertEqual(user_can_create_library(self.user), True) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_creator_role(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): grant_course_creator_status(self.user, nostaff_user) - self.assertEqual(get_library_creator_status(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user), True) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_no_course_creator_role(self): _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(get_library_creator_status(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user), True) @ddt.data( (False, False, True), @@ -92,7 +92,7 @@ def test_library_creator_status_settings(self, disable_course, disable_library, "DISABLE_LIBRARY_CREATION": disable_library } ): - self.assertEqual(get_library_creator_status(nostaff_user), expected_status) + self.assertEqual(user_can_create_library(nostaff_user), expected_status) @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True}) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) @@ -101,7 +101,7 @@ def test_library_creator_status_with_no_course_creator_role_and_disabled_nonstaf Ensure that `DISABLE_COURSE_CREATION` feature works with libraries as well. """ nostaff_client, nostaff_user = self.create_non_staff_authed_user_client() - self.assertFalse(get_library_creator_status(nostaff_user)) + self.assertFalse(user_can_create_library(nostaff_user)) # To be explicit, this user can GET, but not POST get_response = nostaff_client.get_json(LIBRARY_REST_URL) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 5b607eb60351..9f8e2ae494ef 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -6,9 +6,12 @@ import logging from smtplib import SMTPException +from django import forms from django.conf import settings from django.contrib import admin +from django.core.exceptions import ValidationError from django.core.mail import send_mail +from django.db.models.signals import m2m_changed from django.dispatch import receiver from cms.djangoapps.course_creators.models import ( @@ -17,7 +20,7 @@ send_user_notification, update_creator_state ) -from cms.djangoapps.course_creators.views import update_course_creator_group +from cms.djangoapps.course_creators.views import update_course_creator_group, update_org_content_creator_role from common.djangoapps.edxmako.shortcuts import render_to_string log = logging.getLogger("studio.coursecreatoradmin") @@ -30,18 +33,50 @@ def get_email(obj): get_email.short_description = 'email' +class CourseCreatorForm(forms.ModelForm): + """ + Admin form for course creator + """ + class Meta: + model = CourseCreator + fields = '__all__' + + def clean(self): + """ + Validate the 'state', 'organizations' and 'all_orgs' field before saving. + """ + all_orgs = self.cleaned_data.get("all_organizations") + orgs = self.cleaned_data.get("organizations").exists() + state = self.cleaned_data.get("state") + is_all_org_selected_with_orgs = (orgs and all_orgs) + is_orgs_added_with_all_orgs_selected = (not orgs and not all_orgs) + is_state_granted = state == CourseCreator.GRANTED + if is_state_granted: + if is_all_org_selected_with_orgs: + raise ValidationError( + "The role can be granted either to ALL organizations or to " + "specific organizations but not both." + ) + if is_orgs_added_with_all_orgs_selected: + raise ValidationError( + "Specific organizations needs to be selected to grant this role," + "if it is not granted to all organiztions" + ) + + class CourseCreatorAdmin(admin.ModelAdmin): """ Admin for the course creator table. """ # Fields to display on the overview page. - list_display = ['username', get_email, 'state', 'state_changed', 'note'] + list_display = ['username', get_email, 'state', 'state_changed', 'note', 'all_organizations'] + filter_horizontal = ('organizations',) readonly_fields = ['username', 'state_changed'] # Controls the order on the edit form (without this, read-only fields appear at the end). fieldsets = ( (None, { - 'fields': ['username', 'state', 'state_changed', 'note'] + 'fields': ['username', 'state', 'state_changed', 'note', 'all_organizations', 'organizations'] }), ) # Fields that filtering support @@ -50,6 +85,7 @@ class CourseCreatorAdmin(admin.ModelAdmin): search_fields = ['user__username', 'user__email', 'state', 'note'] # Turn off the action bar (we have no bulk actions) actions = None + form = CourseCreatorForm def username(self, inst): """ @@ -75,22 +111,31 @@ def save_model(self, request, obj, form, change): obj.admin = request.user obj.save() + # This functions is overriden to update the m2m query + def save_related(self, request, form, formsets, change): + super().save_related(request, form, formsets, change) + state = form.instance.state + if state != CourseCreator.GRANTED: + form.instance.organizations.clear() + admin.site.register(CourseCreator, CourseCreatorAdmin) @receiver(update_creator_state, sender=CourseCreator) -def update_creator_group_callback(sender, **kwargs): +def update_creator_group_callback(sender, **kwargs): # pylint: disable=unused-argument """ Callback for when the model's creator status has changed. """ user = kwargs['user'] updated_state = kwargs['state'] - update_course_creator_group(kwargs['caller'], user, updated_state == CourseCreator.GRANTED) + all_orgs = kwargs['all_organizations'] + create_role = all_orgs and (updated_state == CourseCreator.GRANTED) + update_course_creator_group(kwargs['caller'], user, create_role) @receiver(send_user_notification, sender=CourseCreator) -def send_user_notification_callback(sender, **kwargs): +def send_user_notification_callback(sender, **kwargs): # pylint: disable=unused-argument """ Callback for notifying user about course creator status change. """ @@ -118,7 +163,7 @@ def send_user_notification_callback(sender, **kwargs): @receiver(send_admin_notification, sender=CourseCreator) -def send_admin_notification_callback(sender, **kwargs): +def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unused-argument """ Callback for notifying admin of a user in the 'pending' state. """ @@ -141,3 +186,21 @@ def send_admin_notification_callback(sender, **kwargs): ) except SMTPException: log.warning(u"Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email) + + +@receiver(m2m_changed, sender=CourseCreator.organizations.through) +def course_creator_organizations_changed_callback(sender, **kwargs): # pylint: disable=unused-argument + """ + Callback for addition and removal of orgs field. + """ + instance = kwargs["instance"] + action = kwargs["action"] + orgs = list(instance.organizations.all().values_list('short_name', flat=True)) + updated_state = instance.state + is_granted = updated_state == CourseCreator.GRANTED + should_update_role = ( + (action in ["post_add", "post_remove"] and is_granted) or + (action == "post_clear" and not is_granted) + ) + if should_update_role: + update_org_content_creator_role(instance.admin, instance.user, orgs) diff --git a/cms/djangoapps/course_creators/migrations/0002_add_org_support_for_course_creators.py b/cms/djangoapps/course_creators/migrations/0002_add_org_support_for_course_creators.py new file mode 100644 index 000000000000..479afbb69022 --- /dev/null +++ b/cms/djangoapps/course_creators/migrations/0002_add_org_support_for_course_creators.py @@ -0,0 +1,24 @@ +# Generated by Django 2.2.20 on 2021-10-04 06:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organizations', '0001_squashed_0007_historicalorganization'), + ('course_creators', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='coursecreator', + name='all_organizations', + field=models.BooleanField(default=True, help_text='Grant the user the permission to create courses in ALL organizations'), + ), + migrations.AddField( + model_name='coursecreator', + name='organizations', + field=models.ManyToManyField(blank=True, help_text='Organizations under which the user is allowed to create courses.', to='organizations.Organization'), + ), + ] diff --git a/cms/djangoapps/course_creators/models.py b/cms/djangoapps/course_creators/models.py index bfb09a0c3053..d9e133fda32f 100644 --- a/cms/djangoapps/course_creators/models.py +++ b/cms/djangoapps/course_creators/models.py @@ -2,17 +2,17 @@ Table for storing information about whether or not Studio users have course creation privileges. """ - -from django.contrib.auth.models import User +from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.db import models from django.db.models.signals import post_init, post_save from django.dispatch import Signal, receiver from django.utils import timezone from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ +from organizations.models import Organization # A signal that will be sent when users should be added or removed from the creator group -update_creator_state = Signal(providing_args=["caller", "user", "state"]) +update_creator_state = Signal(providing_args=["caller", "user", "state", "organizations"]) # A signal that will be sent when admin should be notified of a pending user request send_admin_notification = Signal(providing_args=["user"]) @@ -48,6 +48,12 @@ class CourseCreator(models.Model): help_text=_("Current course creator state")) note = models.CharField(max_length=512, blank=True, help_text=_("Optional notes about this user (for example, " "why course creation access was denied)")) + organizations = models.ManyToManyField(Organization, blank=True, + help_text=_("Organizations under which the user is allowed " + "to create courses.")) + all_organizations = models.BooleanField(default=True, + help_text=_("Grant the user the permission to create courses " + "in ALL organizations")) def __str__(self): return u"{0} | {1} [{2}]".format(self.user, self.state, self.state_changed) @@ -60,6 +66,7 @@ def post_init_callback(sender, **kwargs): """ instance = kwargs['instance'] instance.orig_state = instance.state + instance.orig_all_organizations = instance.all_organizations @receiver(post_save, sender=CourseCreator) @@ -70,7 +77,9 @@ def post_save_callback(sender, **kwargs): instance = kwargs['instance'] # We only wish to modify the state_changed time if the state has been modified. We don't wish to # modify it for changes to the notes field. - if instance.state != instance.orig_state: + # We need to keep track of all_organization switch. If this switch is changed we are going to remove the + # Course Creator group. + if instance.state != instance.orig_state or instance.all_organizations != instance.orig_all_organizations: granted_state_change = instance.state == CourseCreator.GRANTED or instance.orig_state == CourseCreator.GRANTED # If either old or new state is 'granted', we must manipulate the course creator # group maintained by authz. That requires staff permissions (stored admin). @@ -80,7 +89,8 @@ def post_save_callback(sender, **kwargs): sender=sender, caller=instance.admin, user=instance.user, - state=instance.state + state=instance.state, + all_organizations=instance.all_organizations ) # If user has been denied access, granted access, or previously granted access has been @@ -101,4 +111,5 @@ def post_save_callback(sender, **kwargs): instance.state_changed = timezone.now() instance.orig_state = instance.state + instance.orig_all_organizations = instance.all_organizations instance.save() diff --git a/cms/djangoapps/course_creators/tests/test_views.py b/cms/djangoapps/course_creators/tests/test_views.py index 78493ce85715..c53a58c48747 100644 --- a/cms/djangoapps/course_creators/tests/test_views.py +++ b/cms/djangoapps/course_creators/tests/test_views.py @@ -14,10 +14,11 @@ add_user_with_status_unrequested, get_course_creator_status, update_course_creator_group, + update_org_content_creator_role, user_requested_access ) from common.djangoapps.student import auth -from common.djangoapps.student.roles import CourseCreatorRole +from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole class CourseCreatorView(TestCase): @@ -31,6 +32,7 @@ def setUp(self): self.user = User.objects.create_user('test_user', 'test_user+courses@edx.org', 'foo') self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo') self.admin.is_staff = True + self.org = "Edx" def test_staff_permission_required(self): """ @@ -75,6 +77,14 @@ def test_update_creator_group(self): update_course_creator_group(self.admin, self.user, False) self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) + def test_update_org_content_creator_role(self): + with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) + update_org_content_creator_role(self.admin, self.user, [self.org]) + self.assertTrue(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) + update_org_content_creator_role(self.admin, self.user, []) + self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) + def test_user_requested_access(self): add_user_with_status_unrequested(self.user) self.assertEqual('unrequested', get_course_creator_status(self.user)) diff --git a/cms/djangoapps/course_creators/views.py b/cms/djangoapps/course_creators/views.py index 80f081706722..199bae1c41de 100644 --- a/cms/djangoapps/course_creators/views.py +++ b/cms/djangoapps/course_creators/views.py @@ -5,7 +5,7 @@ from cms.djangoapps.course_creators.models import CourseCreator from common.djangoapps.student import auth -from common.djangoapps.student.roles import CourseCreatorRole +from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole def add_user_with_status_unrequested(user): @@ -50,6 +50,16 @@ def update_course_creator_group(caller, user, add): auth.remove_users(caller, CourseCreatorRole(), user) +def update_org_content_creator_role(caller, user, orgs): + """ + Method for updating users for OrgContentCreatorRole, this method + takes care of creating and deleting the role as required. + + Caller must have staff permissions. + """ + auth.update_org_role(caller, OrgContentCreatorRole, user, orgs) + + def get_course_creator_status(user): """ Returns the status for a particular user, or None if user is not in the table. diff --git a/cms/envs/common.py b/cms/envs/common.py index bb5e89247708..a148108cffa0 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -410,21 +410,6 @@ # .. toggle_warnings: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and see # REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND for rollout. 'ENABLE_LIBRARY_AUTHORING_MICROFRONTEND': False, - - # .. toggle_name: RESTRICT_COURSE_CREATION_TO_ORG_ROLES - # .. toggle_implementation: DjangoSetting - # .. toggle_default: False - # .. toggle_description: Restricts users from creating courses/libraries in organisations - # which they don't belong to. This flag doesn't apply to Global Staff and Superusers. - # To enable, set to True. - # To disable, set to False. - # .. toggle_category: n/a - # .. toggle_use_cases: open_edx - # .. toggle_creation_date: 2021-06-23 - # .. toggle_expiration_date: None - # .. toggle_status: supported - 'RESTRICT_COURSE_CREATION_TO_ORG_ROLES': False - } ENABLE_JASMINE = False diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 0d29b4d867a6..5ef4d0459e74 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -19,6 +19,7 @@ CourseStaffRole, GlobalStaff, LibraryUserRole, + OrgContentCreatorRole, OrgInstructorRole, OrgLibraryUserRole, OrgStaffRole @@ -49,7 +50,7 @@ def user_has_role(user, role): """ if not user.is_active: return False - # do cheapest check first even tho it's not the direct one + # Do cheapest check first even though it's not the direct one if GlobalStaff().has_user(user): return True # CourseCreator is odd b/c it can be disabled via config @@ -63,10 +64,11 @@ def user_has_role(user, role): if role.has_user(user): return True - # if not, then check inferred permissions + # If not, then check inferred permissions if (isinstance(role, (CourseStaffRole, CourseBetaTesterRole)) and CourseInstructorRole(role.course_key).has_user(user)): return True + return False @@ -100,7 +102,7 @@ def get_user_permissions(user, course_key, org=None): return STUDIO_NO_PERMISSIONS -def has_studio_write_access(user, course_key, org=None): +def has_studio_write_access(user, course_key): """ Return True if user has studio write access to the given course. Note that the CMS permissions model is with respect to courses. @@ -112,9 +114,8 @@ def has_studio_write_access(user, course_key, org=None): :param user: :param course_key: a CourseKey - :param org: name of organisation """ - return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key, org)) + return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key)) def has_course_author_access(user, course_key): @@ -161,6 +162,26 @@ def remove_users(caller, role, *users): role.remove_users(*users) +def update_org_role(caller, role, user, orgs): + """ + The caller requests updating the Org role for the user. Checks that the caller has + sufficient authority. + + :param caller: an user + :param role: an AccessRole class + :param user: an user for which org roles are updated + :param orgs: List of organization names to update the org role + """ + _check_caller_authority(caller, role()) + existing_org_roles = set(role().get_orgs_for_user(user)) + orgs_roles_to_create = list(set(orgs) - existing_org_roles) + org_roles_to_delete = list(existing_org_roles - set(orgs)) + for org in orgs_roles_to_create: + role(org=org).add_users(user) + for org in org_roles_to_delete: + role(org=org).remove_users(user) + + def _check_caller_authority(caller, role): """ Internal function to check whether the caller has authority to manipulate this role @@ -173,7 +194,7 @@ def _check_caller_authority(caller, role): if GlobalStaff().has_user(caller): return - if isinstance(role, (GlobalStaff, CourseCreatorRole)): + if isinstance(role, (GlobalStaff, CourseCreatorRole, OrgContentCreatorRole)): # lint-amnesty, pylint: disable=no-else-raise raise PermissionDenied elif isinstance(role, CourseRole): # instructors can change the roles w/in their course if not user_has_role(caller, CourseInstructorRole(role.course_key)): diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 37596f416262..a17c9a08f481 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -219,6 +219,12 @@ def users_with_role(self): ) return entries + def get_orgs_for_user(self, user): + """ + Returns a list of org short names for the user with given role. + """ + return CourseAccessRole.objects.filter(user=user, role=self._role_name).values_list('org', flat=True) + class CourseRole(RoleBase): """ @@ -333,6 +339,16 @@ def __init__(self, *args, **kwargs): super(OrgInstructorRole, self).__init__('instructor', *args, **kwargs) +@register_access_role +class OrgContentCreatorRole(OrgRole): + """An organization content creator""" + + ROLE = "org_course_creator_group" + + def __init__(self, *args, **kwargs): + super().__init__(self.ROLE, *args, **kwargs) + + class OrgLibraryUserRole(OrgRole): """ A user who can view any libraries in an org and import content from them, but not edit them. diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index 3704a96c63b7..29fdb6649cb9 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -4,15 +4,28 @@ import mock +import pytest from ccx_keys.locator import CCXLocator from django.contrib.auth.models import AnonymousUser, User from django.core.exceptions import PermissionDenied from django.test import TestCase from opaque_keys.edx.locator import CourseLocator -from common.djangoapps.student.auth import add_users, has_studio_read_access, has_studio_write_access, remove_users, user_has_role -from common.djangoapps.student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole -from common.djangoapps.student.tests.factories import AdminFactory +from common.djangoapps.student.auth import ( + add_users, + has_studio_read_access, + has_studio_write_access, + remove_users, + update_org_role, + user_has_role +) +from common.djangoapps.student.roles import ( + CourseCreatorRole, + CourseInstructorRole, + CourseStaffRole, + OrgContentCreatorRole +) +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory class CreatorGroupTest(TestCase): @@ -243,3 +256,34 @@ def test_remove_user_from_course_group_permission_denied(self): add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator, self.staff, another_staff) with self.assertRaises(PermissionDenied): remove_users(self.staff, CourseStaffRole(self.course_key), another_staff) + + +class CourseOrgGroupTest(TestCase): + """ + Tests for Org Content Creator groups for a particular course. + """ + + def setUp(self): + """ Test case setup """ + super().setUp() + self.global_admin = AdminFactory() + self.user = UserFactory.create( + username='test', email='test+courses@edx.org', password='foo' + ) + self.org = 'mitx' + self.course_key = CourseLocator(self.org, '101', 'test') + + def test_update_org_role_permission_denied(self): + """ + Verifies PermissionDenied if caller of update_org_role is not instructor role. + """ + with pytest.raises(PermissionDenied): + update_org_role(self.user, OrgContentCreatorRole, self.user, [self.org]) + + def test_update_org_role_permission(self): + """ + Verifies if caller of update_org_role is GlobalAdmin. + """ + assert not user_has_role(self.user, OrgContentCreatorRole(self.org)) + update_org_role(self.global_admin, OrgContentCreatorRole, self.user, [self.org]) + assert user_has_role(self.user, OrgContentCreatorRole(self.org)) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 54e0652ea088..2bc3dde3492f 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -15,11 +15,12 @@ CourseRole, CourseStaffRole, GlobalStaff, + OrgContentCreatorRole, OrgInstructorRole, OrgStaffRole, RoleCache ) -from common.djangoapps.student.tests.factories import AnonymousUserFactory +from common.djangoapps.student.tests.factories import AnonymousUserFactory, InstructorFactory, StaffFactory, UserFactory class RolesTestCase(TestCase): @@ -36,6 +37,7 @@ def setUp(self): self.global_staff = UserFactory(is_staff=True) self.course_staff = StaffFactory(course_key=self.course_key) self.course_instructor = InstructorFactory(course_key=self.course_key) + self.orgs = ["Marvel", "DC"] def test_global_staff(self): self.assertFalse(GlobalStaff().has_user(self.student)) @@ -164,6 +166,18 @@ def test_add_users_doesnt_add_duplicate_entry(self): role.remove_users(self.student) self.assertFalse(role.has_user(self.student)) + def test_get_orgs_for_user(self): + """ + Test get_orgs_for_user + """ + role = OrgContentCreatorRole(org=self.orgs[0]) + assert len(role.get_orgs_for_user(self.student)) == 0 + role.add_users(self.student) + assert len(role.get_orgs_for_user(self.student)) == 1 + role_second_org = OrgContentCreatorRole(org=self.orgs[1]) + role_second_org.add_users(self.student) + assert len(role.get_orgs_for_user(self.student)) == 2 + @ddt.ddt class RoleCacheTestCase(TestCase):