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
This commit is contained in:
Farhaan Bukhsh
2021-09-10 22:13:26 +05:30
committed by GitHub
parent 42035b669b
commit 48ad7effb1
14 changed files with 459 additions and 51 deletions

View File

@@ -19,6 +19,7 @@ from common.djangoapps.student.roles import (
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
@@ -160,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
@@ -172,7 +194,7 @@ def _check_caller_authority(caller, role):
if GlobalStaff().has_user(caller):
return
if isinstance(role, (GlobalStaff, CourseCreatorRole)): # lint-amnesty, pylint: disable=no-else-raise
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)):

View File

@@ -218,6 +218,12 @@ class RoleBase(AccessRole):
)
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):
"""
@@ -332,6 +338,16 @@ class OrgInstructorRole(OrgRole):
super().__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.

View File

@@ -3,16 +3,28 @@ Tests authz.py
"""
from unittest import mock
import pytest
import pytest
from ccx_keys.locator import CCXLocator
from django.contrib.auth.models import AnonymousUser
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 # lint-amnesty, pylint: disable=line-too-long
from common.djangoapps.student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole
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
@@ -254,3 +266,34 @@ class CourseGroupTest(TestCase):
add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator, self.staff, another_staff)
with pytest.raises(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))

View File

@@ -14,14 +14,12 @@ from common.djangoapps.student.roles import (
CourseRole,
CourseStaffRole,
GlobalStaff,
OrgContentCreatorRole,
OrgInstructorRole,
OrgStaffRole,
RoleCache
)
from common.djangoapps.student.tests.factories import AnonymousUserFactory
from common.djangoapps.student.tests.factories import InstructorFactory
from common.djangoapps.student.tests.factories import StaffFactory
from common.djangoapps.student.tests.factories import UserFactory
from common.djangoapps.student.tests.factories import AnonymousUserFactory, InstructorFactory, StaffFactory, UserFactory
class RolesTestCase(TestCase):
@@ -38,6 +36,7 @@ class RolesTestCase(TestCase):
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):
assert not GlobalStaff().has_user(self.student)
@@ -142,6 +141,18 @@ class RolesTestCase(TestCase):
role.remove_users(self.student)
assert not 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): # lint-amnesty, pylint: disable=missing-class-docstring