Skip to content

E
edx-platform
Commit 7e029515 by Don Mitchell
Options

Merge pull request #2064 from edx/dhm/auth_consolidate 

Replace authz with roles.py and reconcile their behaviors
parents 9db8b497 ad3140a6
Showing with 485 additions and 672 deletions
CHANGELOG.rst
...@@ -28,6 +28,8 @@ Studio: Newly-created courses default to being published on Jan 1, 2030 ...@@ -28,6 +28,8 @@ Studio: Newly-created courses default to being published on Jan 1, 2030
Studio: Added pagination to the Files & Uploads page. Studio: Added pagination to the Files & Uploads page.
Common: Centralized authorization mechanisms and removed the app-specific ones.
Blades: Video player improvements: Blades: Video player improvements:
- Disable edX controls on iPhone/iPod (native controls are used). - Disable edX controls on iPhone/iPod (native controls are used).
- Disable unsupported controls (volume, playback rate) on iPad/Android. - Disable unsupported controls (volume, playback rate) on iPad/Android.
......
cms/djangoapps/auth/authz.py deleted 100644 → 0
"""
Studio authorization functions primarily for course creators, instructors, and staff
"""
#=======================================================================================================================
#
# This code is somewhat duplicative of access.py in the LMS. We will unify the code as a separate story
# but this implementation should be data compatible with the LMS implementation
#
#=======================================================================================================================
from django.contrib.auth.models import User, Group
from django.core.exceptions import PermissionDenied
from django.conf import settings
from xmodule.modulestore import Location
from xmodule.modulestore.locator import CourseLocator, Locator
from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
import itertools
# define a couple of simple roles, we just need ADMIN and EDITOR now for our purposes
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
def get_all_course_role_groupnames(location, role, use_filter=True):
'''
Get all of the possible groupnames for this role location pair. If use_filter==True,
only return the ones defined in the groups collection.
'''
location = Locator.to_locator_or_location(location)
groupnames = []
if isinstance(location, Location):
try:
groupnames.append('{0}_{1}'.format(role, location.course_id))
except InvalidLocationError: # will occur on old locations where location is not of category course
pass
try:
locator = loc_mapper().translate_location(location.course_id, location, False, False)
groupnames.append('{0}_{1}'.format(role, locator.package_id))
except (InvalidLocationError, ItemNotFoundError):
pass
# least preferred role_course format for legacy reasons
groupnames.append('{0}_{1}'.format(role, location.course))
elif isinstance(location, CourseLocator):
groupnames.append('{0}_{1}'.format(role, location.package_id))
old_location = loc_mapper().translate_locator_to_location(location, get_course=True)
if old_location:
# the slashified version of the course_id (myu/mycourse/myrun)
groupnames.append('{0}_{1}'.format(role, old_location.course_id))
# add the least desirable but sometimes occurring format.
groupnames.append('{0}_{1}'.format(role, old_location.course))
# filter to the ones which exist
default = groupnames[0]
if use_filter:
groupnames = [group.name for group in Group.objects.filter(name__in=groupnames)]
return groupnames, default
def get_course_groupname_for_role(location, role):
'''
Get the preferred used groupname for this role, location combo.
Preference order:
* role_course_id (e.g., staff_myu.mycourse.myrun)
* role_old_course_id (e.g., staff_myu/mycourse/myrun)
* role_old_course (e.g., staff_mycourse)
'''
groupnames, default = get_all_course_role_groupnames(location, role)
return groupnames[0] if groupnames else default
def get_course_role_users(course_locator, role):
'''
Get all of the users with the given role in the given course.
'''
groupnames, _ = get_all_course_role_groupnames(course_locator, role)
groups = [Group.objects.get(name=groupname) for groupname in groupnames]
return list(itertools.chain.from_iterable(group.user_set.all() for group in groups))
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)
def create_new_course_group(creator, location, role):
'''
Create the new course group always using the preferred name even if another form already exists.
'''
groupnames, __ = get_all_course_role_groupnames(location, role, use_filter=False)
group, __ = Group.objects.get_or_create(name=groupnames[0])
creator.groups.add(group)
creator.save()
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
for role in [INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME]:
groupnames, _ = get_all_course_role_groupnames(location, role)
for groupname in groupnames:
group = Group.objects.get(name=groupname)
for user in group.user_set.all():
user.groups.remove(group)
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
"""
for role in [INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME]:
groupnames, _ = get_all_course_role_groupnames(source, role)
for groupname in groupnames:
group = Group.objects.get(name=groupname)
new_group, _ = Group.objects.get_or_create(name=get_course_groupname_for_role(dest, INSTRUCTOR_ROLE_NAME))
for user in group.user_set.all():
user.groups.add(new_group)
user.save()
def add_user_to_course_group(caller, user, location, role):
"""
If caller is authorized, add the given user to the given course's role
"""
# only admins can add/remove other users
if not is_user_in_course_group_role(caller, location, INSTRUCTOR_ROLE_NAME):
raise PermissionDenied
group, _ = Group.objects.get_or_create(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_or_create(name=COURSE_CREATOR_GROUP_NAME)
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
return False
def get_user_by_email(email):
"""
Get the user whose email is the arg. Return None if no such user exists.
"""
user = None
# try to look up user, return None if not found
try:
user = User.objects.get(email=email)
except:
pass
return user
def remove_user_from_course_group(caller, user, location, role):
"""
If caller is authorized, remove the given course x role authorization for user
"""
# only admins can add/remove other users
if not is_user_in_course_group_role(caller, location, INSTRUCTOR_ROLE_NAME):
raise PermissionDenied
# see if the user is actually in that role, if not then we don't have to do anything
groupnames, _ = get_all_course_role_groupnames(location, role)
user.groups.remove(*user.groups.filter(name__in=groupnames))
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, check_staff=True):
"""
Check whether the given user has the given role in this course. If check_staff
then give permission if the user is staff without doing a course-role query.
"""
if user.is_active and user.is_authenticated:
# all "is_staff" flagged accounts belong to all groups
if check_staff and user.is_staff:
return True
groupnames, _ = get_all_course_role_groupnames(location, role)
return user.groups.filter(name__in=groupnames).exists()
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.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.FEATURES.get('ENABLE_CREATOR_GROUP', False):
return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).exists()
return True
def get_users_with_instructor_role():
"""
Returns all users with the role 'instructor'
"""
return _get_users_with_role(INSTRUCTOR_ROLE_NAME)
def get_users_with_staff_role():
"""
Returns all users with the role 'staff'
"""
return _get_users_with_role(STAFF_ROLE_NAME)
def _get_users_with_role(role):
"""
Returns all users with the specified role.
"""
users = set()
for group in Group.objects.all():
if group.name.startswith(role + "_"):
for user in group.user_set.all():
users.add(user)
return users
cms/djangoapps/contentstore/features/common.py
# pylint: disable=C0111 # pylint: disable=C0111
# pylint: disable=W0621 # pylint: disable=W0621
import time
import os
from lettuce import world, step from lettuce import world, step
from nose.tools import assert_true, assert_in, assert_false # pylint: disable=E0611 from nose.tools import assert_true, assert_in # pylint: disable=no-name-in-module
from auth.authz import get_user_by_email, get_course_groupname_for_role
from django.conf import settings from django.conf import settings
from student.roles import CourseRole, CourseStaffRole, CourseInstructorRole
from student.models import get_user
from selenium.webdriver.common.keys import Keys from selenium.webdriver.common.keys import Keys
import time
import os
from django.contrib.auth.models import Group
from logging import getLogger from logging import getLogger
from student.tests.factories import AdminFactory
from student import auth
logger = getLogger(__name__) logger = getLogger(__name__)
from terrain.browser import reset_data from terrain.browser import reset_data
...@@ -158,11 +160,9 @@ def add_course_author(user, course): ...@@ -158,11 +160,9 @@ def add_course_author(user, course):
Add the user to the instructor group of the course Add the user to the instructor group of the course
so they will have the permissions to see it in studio so they will have the permissions to see it in studio
""" """
for role in ("staff", "instructor"): global_admin = AdminFactory()
groupname = get_course_groupname_for_role(course.location, role) for role in (CourseStaffRole, CourseInstructorRole):
group, __ = Group.objects.get_or_create(name=groupname) auth.add_users(global_admin, role(course.location), user)
user.groups.add(group)
user.save()
def create_a_course(): def create_a_course():
...@@ -171,7 +171,7 @@ def create_a_course(): ...@@ -171,7 +171,7 @@ def create_a_course():
user = world.scenario_dict.get("USER") user = world.scenario_dict.get("USER")
if not user: if not user:
user = get_user_by_email('robot+studio@edx.org') user = get_user('robot+studio@edx.org')
add_course_author(user, course) add_course_author(user, course)
...@@ -358,7 +358,7 @@ def other_user_login(step, name): ...@@ -358,7 +358,7 @@ def other_user_login(step, name):
login_form.find_by_name('submit').click() login_form.find_by_name('submit').click()
world.retry_on_exception(fill_login_form) world.retry_on_exception(fill_login_form)
assert_true(world.is_css_present('.new-course-button')) assert_true(world.is_css_present('.new-course-button'))
world.scenario_dict['USER'] = get_user_by_email(name + '@edx.org') world.scenario_dict['USER'] = get_user(name + '@edx.org')
@step(u'the user "([^"]*)" exists( as a course (admin|staff member|is_staff))?$') @step(u'the user "([^"]*)" exists( as a course (admin|staff member|is_staff))?$')
...@@ -368,18 +368,17 @@ def create_other_user(_step, name, has_extra_perms, role_name): ...@@ -368,18 +368,17 @@ def create_other_user(_step, name, has_extra_perms, role_name):
if has_extra_perms: if has_extra_perms:
if role_name == "is_staff": if role_name == "is_staff":
user.is_staff = True user.is_staff = True
user.save()
else: else:
if role_name == "admin": if role_name == "admin":
# admins get staff privileges, as well # admins get staff privileges, as well
roles = ("staff", "instructor") roles = (CourseStaffRole, CourseInstructorRole)
else: else:
roles = ("staff",) roles = (CourseStaffRole,)
location = world.scenario_dict["COURSE"].location location = world.scenario_dict["COURSE"].location
global_admin = AdminFactory()
for role in roles: for role in roles:
groupname = get_course_groupname_for_role(location, role) auth.add_users(global_admin, role(location), user)
group, __ = Group.objects.get_or_create(name=groupname)
user.groups.add(group)
user.save()
@step('I log out') @step('I log out')
...@@ -393,7 +392,7 @@ def i_edit_a_draft(_step): ...@@ -393,7 +392,7 @@ def i_edit_a_draft(_step):
@step(u'I click on "replace with draft"$') @step(u'I click on "replace with draft"$')
def i_edit_a_draft(_step): def i_replace_w_draft(_step):
world.css_click("a.publish-draft") world.css_click("a.publish-draft")
......
cms/djangoapps/contentstore/management/commands/clone_course.py
...@@ -6,12 +6,11 @@ from xmodule.modulestore.store_utilities import clone_course ...@@ -6,12 +6,11 @@ from xmodule.modulestore.store_utilities import clone_course
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore from xmodule.contentstore.django import contentstore
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
from student.roles import CourseInstructorRole, CourseStaffRole
from auth.authz import _copy_course_group
# #
# To run from command line: rake cms:clone SOURCE_LOC=edX/111/Foo1 DEST_LOC=edX/135/Foo3 # To run from command line: ./manage.py cms clone_course --settings=dev master/300/cough edx/111/foo
# #
class Command(BaseCommand): class Command(BaseCommand):
"""Clone a MongoDB-backed course to another location""" """Clone a MongoDB-backed course to another location"""
...@@ -20,7 +19,7 @@ class Command(BaseCommand): ...@@ -20,7 +19,7 @@ class Command(BaseCommand):
def handle(self, *args, **options): def handle(self, *args, **options):
"Execute the command" "Execute the command"
if len(args) != 2: if len(args) != 2:
raise CommandError("clone requires two arguments: <source-course_id> <dest-course_id>") raise CommandError("clone requires 2 arguments: <source-course_id> <dest-course_id>")
source_course_id = args[0] source_course_id = args[0]
dest_course_id = args[1] dest_course_id = args[1]
...@@ -28,7 +27,7 @@ class Command(BaseCommand): ...@@ -28,7 +27,7 @@ class Command(BaseCommand):
mstore = modulestore('direct') mstore = modulestore('direct')
cstore = contentstore() cstore = contentstore()
org, course_num, run = dest_course_id.split("/") org, course_num, _ = dest_course_id.split("/")
mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))
print("Cloning course {0} to {1}".format(source_course_id, dest_course_id)) print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))
...@@ -41,4 +40,10 @@ class Command(BaseCommand): ...@@ -41,4 +40,10 @@ class Command(BaseCommand):
mstore.refresh_cached_metadata_inheritance_tree(dest_location) mstore.refresh_cached_metadata_inheritance_tree(dest_location)
print("copying User permissions...") print("copying User permissions...")
_copy_course_group(source_location, dest_location) # purposely avoids auth.add_user b/c it doesn't have a caller to authorize
CourseInstructorRole(dest_location).add_users(
*CourseInstructorRole(source_location).users_with_role()
)
CourseStaffRole(dest_location).add_users(
*CourseStaffRole(source_location).users_with_role()
)
cms/djangoapps/contentstore/management/commands/populate_creators.py
...@@ -3,14 +3,14 @@ Script for granting existing course instructors course creator privileges. ...@@ -3,14 +3,14 @@ Script for granting existing course instructors course creator privileges.
This script is only intended to be run once on a given environment. This script is only intended to be run once on a given environment.
""" """
from auth.authz import get_users_with_instructor_role, get_users_with_staff_role
from course_creators.views import add_user_with_status_granted, add_user_with_status_unrequested from course_creators.views import add_user_with_status_granted, add_user_with_status_unrequested
from django.core.management.base import BaseCommand from django.core.management.base import BaseCommand
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.db.utils import IntegrityError from django.db.utils import IntegrityError
from student.roles import CourseInstructorRole, CourseStaffRole
#------------ to run: ./manage.py cms populate_creators --settings=dev
class Command(BaseCommand): class Command(BaseCommand):
""" """
Script for granting existing course instructors course creator privileges. Script for granting existing course instructors course creator privileges.
...@@ -32,13 +32,13 @@ class Command(BaseCommand): ...@@ -32,13 +32,13 @@ class Command(BaseCommand):
# the admin user will already exist. # the admin user will already exist.
admin = User.objects.get(username=username, email=email) admin = User.objects.get(username=username, email=email)
for user in get_users_with_instructor_role(): for user in get_users_with_role(CourseInstructorRole.ROLE):
add_user_with_status_granted(admin, user) add_user_with_status_granted(admin, user)
# Some users will be both staff and instructors. Those folks have been # Some users will be both staff and instructors. Those folks have been
# added with status granted above, and add_user_with_status_unrequested # added with status granted above, and add_user_with_status_unrequested
# will not try to add them again if they already exist in the course creator database. # will not try to add them again if they already exist in the course creator database.
for user in get_users_with_staff_role(): for user in get_users_with_role(CourseStaffRole.ROLE):
add_user_with_status_unrequested(user) add_user_with_status_unrequested(user)
# There could be users who are not in either staff or instructor (they've # There could be users who are not in either staff or instructor (they've
...@@ -46,3 +46,12 @@ class Command(BaseCommand): ...@@ -46,3 +46,12 @@ class Command(BaseCommand):
# when they first go to their dashboard. # when they first go to their dashboard.
admin.delete() admin.delete()
#=============================================================================================================
# Because these are expensive and far-reaching, I moved them here
def get_users_with_role(role_prefix):
"""
An expensive operation which finds all users in the db with the given role prefix
"""
return User.objects.filter(groups__name__startswith=role_prefix)
cms/djangoapps/contentstore/tests/test_contentstore.py
...@@ -20,8 +20,6 @@ from django.dispatch import Signal ...@@ -20,8 +20,6 @@ from django.dispatch import Signal
from contentstore.utils import get_modulestore from contentstore.utils import get_modulestore
from contentstore.tests.utils import parse_json, AjaxEnabledTestClient from contentstore.tests.utils import parse_json, AjaxEnabledTestClient
from auth.authz import add_user_to_creator_group
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from contentstore.tests.modulestore_config import TEST_MODULESTORE from contentstore.tests.modulestore_config import TEST_MODULESTORE
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
...@@ -57,6 +55,8 @@ import re ...@@ -57,6 +55,8 @@ import re
from contentstore.utils import delete_course_and_groups from contentstore.utils import delete_course_and_groups
from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.django import loc_mapper
from student.roles import CourseCreatorRole
from student import auth
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
...@@ -1530,7 +1530,7 @@ class ContentStoreTest(ModuleStoreTestCase): ...@@ -1530,7 +1530,7 @@ class ContentStoreTest(ModuleStoreTestCase):
def test_create_course_with_course_creator(self): def test_create_course_with_course_creator(self):
"""Test new course creation -- use course creator group""" """Test new course creation -- use course creator group"""
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
add_user_to_creator_group(self.user, self.user) auth.add_users(self.user, CourseCreatorRole(), self.user)
self.assert_created_course() self.assert_created_course()
def assert_course_permission_denied(self): def assert_course_permission_denied(self):
......
cms/djangoapps/contentstore/tests/test_permissions.py
""" """
Test CRUD for authorization. Test CRUD for authorization.
""" """
import copy
from django.test.utils import override_settings from django.test.utils import override_settings
from django.contrib.auth.models import User, Group from django.contrib.auth.models import User, Group
...@@ -9,10 +11,9 @@ from contentstore.tests.modulestore_config import TEST_MODULESTORE ...@@ -9,10 +11,9 @@ from contentstore.tests.modulestore_config import TEST_MODULESTORE
from contentstore.tests.utils import AjaxEnabledTestClient from contentstore.tests.utils import AjaxEnabledTestClient
from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore import Location from xmodule.modulestore import Location
from auth.authz import INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME from student.roles import CourseInstructorRole, CourseStaffRole
from auth import authz from contentstore.views.access import has_course_access
import copy from student import auth
from contentstore.views.access import has_access
@override_settings(MODULESTORE=TEST_MODULESTORE) @override_settings(MODULESTORE=TEST_MODULESTORE)
...@@ -87,30 +88,35 @@ class TestCourseAccess(ModuleStoreTestCase): ...@@ -87,30 +88,35 @@ class TestCourseAccess(ModuleStoreTestCase):
Test getting all authors for a course where their permissions run the gamut of allowed group Test getting all authors for a course where their permissions run the gamut of allowed group
types. types.
""" """
# first check the groupname for the course creator. # first check the course creator.has explicit access (don't use has_access as is_staff
# will trump the actual test)
self.assertTrue( self.assertTrue(
self.user.groups.filter( CourseInstructorRole(self.course_locator).has_user(self.user),
name="{}_{}".format(INSTRUCTOR_ROLE_NAME, self.course_locator.package_id)
).exists(),
"Didn't add creator as instructor." "Didn't add creator as instructor."
) )
users = copy.copy(self.users) users = copy.copy(self.users)
# doesn't use role.users_with_role b/c it's verifying the roles.py behavior
user_by_role = {} user_by_role = {}
# add the misc users to the course in different groups # add the misc users to the course in different groups
for role in [INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME]: for role in [CourseInstructorRole, CourseStaffRole]:
user_by_role[role] = [] user_by_role[role] = []
groupnames, _ = authz.get_all_course_role_groupnames(self.course_locator, role) # pylint: disable=protected-access
groupnames = role(self.course_locator)._group_names
self.assertGreater(len(groupnames), 1, "Only 0 or 1 groupname for {}".format(role.ROLE))
# NOTE: this loop breaks the roles.py abstraction by purposely assigning
# users to one of each possible groupname in order to test that has_course_access
# and remove_user work
for groupname in groupnames: for groupname in groupnames:
group, _ = Group.objects.get_or_create(name=groupname) group, _ = Group.objects.get_or_create(name=groupname)
user = users.pop() user = users.pop()
user_by_role[role].append(user) user_by_role[role].append(user)
user.groups.add(group) user.groups.add(group)
user.save() user.save()
self.assertTrue(has_access(user, self.course_locator), "{} does not have access".format(user)) self.assertTrue(has_course_access(user, self.course_locator), "{} does not have access".format(user))
self.assertTrue(has_access(user, self.course_location), "{} does not have access".format(user)) self.assertTrue(has_course_access(user, self.course_location), "{} does not have access".format(user))
response = self.client.get_html(self.course_locator.url_reverse('course_team')) response = self.client.get_html(self.course_locator.url_reverse('course_team'))
for role in [INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME]: for role in [CourseInstructorRole, CourseStaffRole]:
for user in user_by_role[role]: for user in user_by_role[role]:
self.assertContains(response, user.email) self.assertContains(response, user.email)
...@@ -119,9 +125,23 @@ class TestCourseAccess(ModuleStoreTestCase): ...@@ -119,9 +125,23 @@ class TestCourseAccess(ModuleStoreTestCase):
copy_course_locator = loc_mapper().translate_location( copy_course_locator = loc_mapper().translate_location(
copy_course_location.course_id, copy_course_location, False, True copy_course_location.course_id, copy_course_location, False, True
) )
# pylint: disable=protected-access for role in [CourseInstructorRole, CourseStaffRole]:
authz._copy_course_group(self.course_locator, copy_course_locator) auth.add_users(
for role in [INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME]: self.user,
role(copy_course_locator),
*role(self.course_locator).users_with_role()
)
# verify access in copy course and verify that removal from source course w/ the various
# groupnames works
for role in [CourseInstructorRole, CourseStaffRole]:
for user in user_by_role[role]: for user in user_by_role[role]:
self.assertTrue(has_access(user, copy_course_locator), "{} no copy access".format(user)) # forcefully decache the groups: premise is that any real request will not have
self.assertTrue(has_access(user, copy_course_location), "{} no copy access".format(user)) # multiple objects repr the same user but this test somehow uses different instance
# in above add_users call
if hasattr(user, '_groups'):
del user._groups
self.assertTrue(has_course_access(user, copy_course_locator), "{} no copy access".format(user))
self.assertTrue(has_course_access(user, copy_course_location), "{} no copy access".format(user))
auth.remove_users(self.user, role(self.course_locator), user)
self.assertFalse(has_course_access(user, self.course_locator), "{} remove didn't work".format(user))
cms/djangoapps/contentstore/tests/test_users.py
...@@ -4,9 +4,10 @@ Tests for contentstore/views/user.py. ...@@ -4,9 +4,10 @@ Tests for contentstore/views/user.py.
import json import json
from .utils import CourseTestCase from .utils import CourseTestCase
from django.contrib.auth.models import User, Group from django.contrib.auth.models import User, Group
from auth.authz import get_course_groupname_for_role
from student.models import CourseEnrollment from student.models import CourseEnrollment
from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.django import loc_mapper
from student.roles import CourseStaffRole, CourseInstructorRole
from student import auth
class UsersTestCase(CourseTestCase): class UsersTestCase(CourseTestCase):
...@@ -29,8 +30,6 @@ class UsersTestCase(CourseTestCase): ...@@ -29,8 +30,6 @@ class UsersTestCase(CourseTestCase):
self.detail_url = self.location.url_reverse('course_team', self.ext_user.email) self.detail_url = self.location.url_reverse('course_team', self.ext_user.email)
self.inactive_detail_url = self.location.url_reverse('course_team', self.inactive_user.email) self.inactive_detail_url = self.location.url_reverse('course_team', self.inactive_user.email)
self.invalid_detail_url = self.location.url_reverse('course_team', "nonexistent@user.com") self.invalid_detail_url = self.location.url_reverse('course_team', "nonexistent@user.com")
self.staff_groupname = get_course_groupname_for_role(self.course_locator, "staff")
self.inst_groupname = get_course_groupname_for_role(self.course_locator, "instructor")
def test_index(self): def test_index(self):
resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html') resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html')
...@@ -39,9 +38,7 @@ class UsersTestCase(CourseTestCase): ...@@ -39,9 +38,7 @@ class UsersTestCase(CourseTestCase):
self.assertNotContains(resp, self.ext_user.email) self.assertNotContains(resp, self.ext_user.email)
def test_index_member(self): def test_index_member(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.ext_user)
self.ext_user.groups.add(group)
self.ext_user.save()
resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html') resp = self.client.get(self.index_url, HTTP_ACCEPT='text/html')
self.assertContains(resp, self.ext_user.email) self.assertContains(resp, self.ext_user.email)
...@@ -73,10 +70,9 @@ class UsersTestCase(CourseTestCase): ...@@ -73,10 +70,9 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()]
# no content: should not be in any roles # no content: should not be in any roles
self.assertNotIn(self.staff_groupname, groups) self.assertFalse(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertNotIn(self.inst_groupname, groups) self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assert_not_enrolled() self.assert_not_enrolled()
def test_detail_post_staff(self): def test_detail_post_staff(self):
...@@ -89,15 +85,12 @@ class UsersTestCase(CourseTestCase): ...@@ -89,15 +85,12 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertIn(self.staff_groupname, groups) self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.inst_groupname, groups)
self.assert_enrolled() self.assert_enrolled()
def test_detail_post_staff_other_inst(self): def test_detail_post_staff_other_inst(self):
inst_group, _ = Group.objects.get_or_create(name=self.inst_groupname) auth.add_users(self.user, CourseInstructorRole(self.course_locator), self.user)
self.user.groups.add(inst_group)
self.user.save()
resp = self.client.post( resp = self.client.post(
self.detail_url, self.detail_url,
...@@ -108,15 +101,13 @@ class UsersTestCase(CourseTestCase): ...@@ -108,15 +101,13 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertIn(self.staff_groupname, groups) self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.inst_groupname, groups)
self.assert_enrolled() self.assert_enrolled()
# check that other user is unchanged # check that other user is unchanged
user = User.objects.get(email=self.user.email) user = User.objects.get(email=self.user.email)
groups = [g.name for g in user.groups.all()] self.assertTrue(auth.has_access(user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.staff_groupname, groups) self.assertFalse(CourseStaffRole(self.course_locator).has_user(user))
self.assertIn(self.inst_groupname, groups)
def test_detail_post_instructor(self): def test_detail_post_instructor(self):
resp = self.client.post( resp = self.client.post(
...@@ -128,9 +119,8 @@ class UsersTestCase(CourseTestCase): ...@@ -128,9 +119,8 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.staff_groupname, groups) self.assertFalse(CourseStaffRole(self.course_locator).has_user(ext_user))
self.assertIn(self.inst_groupname, groups)
self.assert_enrolled() self.assert_enrolled()
def test_detail_post_missing_role(self): def test_detail_post_missing_role(self):
...@@ -154,15 +144,12 @@ class UsersTestCase(CourseTestCase): ...@@ -154,15 +144,12 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertIn(self.staff_groupname, groups) self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.inst_groupname, groups)
self.assert_enrolled() self.assert_enrolled()
def test_detail_delete_staff(self): def test_detail_delete_staff(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.ext_user)
self.ext_user.groups.add(group)
self.ext_user.save()
resp = self.client.delete( resp = self.client.delete(
self.detail_url, self.detail_url,
...@@ -171,15 +158,10 @@ class UsersTestCase(CourseTestCase): ...@@ -171,15 +158,10 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertFalse(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertNotIn(self.staff_groupname, groups)
def test_detail_delete_instructor(self): def test_detail_delete_instructor(self):
group, _ = Group.objects.get_or_create(name=self.inst_groupname) auth.add_users(self.user, CourseInstructorRole(self.course_locator), self.ext_user, self.user)
self.user.groups.add(group)
self.ext_user.groups.add(group)
self.user.save()
self.ext_user.save()
resp = self.client.delete( resp = self.client.delete(
self.detail_url, self.detail_url,
...@@ -188,13 +170,10 @@ class UsersTestCase(CourseTestCase): ...@@ -188,13 +170,10 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertNotIn(self.inst_groupname, groups)
def test_delete_last_instructor(self): def test_delete_last_instructor(self):
group, _ = Group.objects.get_or_create(name=self.inst_groupname) auth.add_users(self.user, CourseInstructorRole(self.course_locator), self.ext_user)
self.ext_user.groups.add(group)
self.ext_user.save()
resp = self.client.delete( resp = self.client.delete(
self.detail_url, self.detail_url,
...@@ -205,13 +184,10 @@ class UsersTestCase(CourseTestCase): ...@@ -205,13 +184,10 @@ class UsersTestCase(CourseTestCase):
self.assertIn("error", result) self.assertIn("error", result)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertIn(self.inst_groupname, groups)
def test_post_last_instructor(self): def test_post_last_instructor(self):
group, _ = Group.objects.get_or_create(name=self.inst_groupname) auth.add_users(self.user, CourseInstructorRole(self.course_locator), self.ext_user)
self.ext_user.groups.add(group)
self.ext_user.save()
resp = self.client.post( resp = self.client.post(
self.detail_url, self.detail_url,
...@@ -223,12 +199,10 @@ class UsersTestCase(CourseTestCase): ...@@ -223,12 +199,10 @@ class UsersTestCase(CourseTestCase):
self.assertIn("error", result) self.assertIn("error", result)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course_locator)))
self.assertIn(self.inst_groupname, groups)
def test_permission_denied_self(self): def test_permission_denied_self(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.user)
self.user.groups.add(group)
self.user.is_staff = False self.user.is_staff = False
self.user.save() self.user.save()
...@@ -244,8 +218,7 @@ class UsersTestCase(CourseTestCase): ...@@ -244,8 +218,7 @@ class UsersTestCase(CourseTestCase):
self.assertIn("error", result) self.assertIn("error", result)
def test_permission_denied_other(self): def test_permission_denied_other(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.user)
self.user.groups.add(group)
self.user.is_staff = False self.user.is_staff = False
self.user.save() self.user.save()
...@@ -259,8 +232,7 @@ class UsersTestCase(CourseTestCase): ...@@ -259,8 +232,7 @@ class UsersTestCase(CourseTestCase):
self.assertIn("error", result) self.assertIn("error", result)
def test_staff_can_delete_self(self): def test_staff_can_delete_self(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.user)
self.user.groups.add(group)
self.user.is_staff = False self.user.is_staff = False
self.user.save() self.user.save()
...@@ -270,16 +242,12 @@ class UsersTestCase(CourseTestCase): ...@@ -270,16 +242,12 @@ class UsersTestCase(CourseTestCase):
self.assertEqual(resp.status_code, 204) self.assertEqual(resp.status_code, 204)
# reload user from DB # reload user from DB
user = User.objects.get(email=self.user.email) user = User.objects.get(email=self.user.email)
groups = [g.name for g in user.groups.all()] self.assertFalse(auth.has_access(user, CourseStaffRole(self.course_locator)))
self.assertNotIn(self.staff_groupname, groups)
def test_staff_cannot_delete_other(self): def test_staff_cannot_delete_other(self):
group, _ = Group.objects.get_or_create(name=self.staff_groupname) auth.add_users(self.user, CourseStaffRole(self.course_locator), self.user, self.ext_user)
self.user.groups.add(group)
self.user.is_staff = False self.user.is_staff = False
self.user.save() self.user.save()
self.ext_user.groups.add(group)
self.ext_user.save()
resp = self.client.delete(self.detail_url) resp = self.client.delete(self.detail_url)
self.assertEqual(resp.status_code, 400) self.assertEqual(resp.status_code, 400)
...@@ -287,8 +255,7 @@ class UsersTestCase(CourseTestCase): ...@@ -287,8 +255,7 @@ class UsersTestCase(CourseTestCase):
self.assertIn("error", result) self.assertIn("error", result)
# reload user from DB # reload user from DB
ext_user = User.objects.get(email=self.ext_user.email) ext_user = User.objects.get(email=self.ext_user.email)
groups = [g.name for g in ext_user.groups.all()] self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course_locator)))
self.assertIn(self.staff_groupname, groups)
def test_user_not_initially_enrolled(self): def test_user_not_initially_enrolled(self):
# Verify that ext_user is not enrolled in the new course before being added as a staff member. # Verify that ext_user is not enrolled in the new course before being added as a staff member.
......
cms/djangoapps/contentstore/utils.py
...@@ -13,10 +13,10 @@ from xmodule.modulestore import Location ...@@ -13,10 +13,10 @@ from xmodule.modulestore import Location
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from django_comment_common.utils import unseed_permissions_roles from django_comment_common.utils import unseed_permissions_roles
from auth.authz import _delete_course_group
from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.store_utilities import delete_course
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
from student.roles import CourseInstructorRole, CourseStaffRole
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -35,7 +35,7 @@ def delete_course_and_groups(course_id, commit=False): ...@@ -35,7 +35,7 @@ def delete_course_and_groups(course_id, commit=False):
module_store = modulestore('direct') module_store = modulestore('direct')
content_store = contentstore() content_store = contentstore()
org, course_num, run = course_id.split("/") org, course_num, _ = course_id.split("/")
module_store.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) module_store.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))
loc = CourseDescriptor.id_to_location(course_id) loc = CourseDescriptor.id_to_location(course_id)
...@@ -47,7 +47,10 @@ def delete_course_and_groups(course_id, commit=False): ...@@ -47,7 +47,10 @@ def delete_course_and_groups(course_id, commit=False):
# in the django layer, we need to remove all the user permissions groups associated with this course # in the django layer, we need to remove all the user permissions groups associated with this course
if commit: if commit:
try: try:
_delete_course_group(loc) staff_role = CourseStaffRole(loc)
staff_role.remove_users(*staff_role.users_with_role())
instructor_role = CourseInstructorRole(loc)
instructor_role.remove_users(*instructor_role.users_with_role())
except Exception as err: except Exception as err:
log.error("Error in deleting course groups for {0}: {1}".format(loc, err)) log.error("Error in deleting course groups for {0}: {1}".format(loc, err))
......
cms/djangoapps/contentstore/views/access.py
from auth.authz import STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME
from auth.authz import is_user_in_course_group_role
from ..utils import get_course_location_for_item from ..utils import get_course_location_for_item
from xmodule.modulestore.locator import CourseLocator from xmodule.modulestore.locator import CourseLocator
from student.roles import CourseStaffRole, GlobalStaff
from student import auth
def has_access(user, location, role=STAFF_ROLE_NAME): def has_course_access(user, location, role=CourseStaffRole):
""" """
Return True if user allowed to access this piece of data Return True if user allowed to access this piece of data
Note that the CMS permissions model is with respect to courses Note that the CMS permissions model is with respect to courses
...@@ -14,14 +14,9 @@ def has_access(user, location, role=STAFF_ROLE_NAME): ...@@ -14,14 +14,9 @@ def has_access(user, location, role=STAFF_ROLE_NAME):
will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our
queries here as INSTRUCTOR has all the rights that STAFF do queries here as INSTRUCTOR has all the rights that STAFF do
""" """
if GlobalStaff().has_user(user):
return True
if not isinstance(location, CourseLocator): if not isinstance(location, CourseLocator):
# this can be expensive if location is not category=='course'
location = get_course_location_for_item(location) location = get_course_location_for_item(location)
_has_access = is_user_in_course_group_role(user, location, role) return auth.has_access(user, role(location))
# if we're not in STAFF, perhaps we're in INSTRUCTOR groups
if not _has_access and role == STAFF_ROLE_NAME:
_has_access = is_user_in_course_group_role(
user,
location,
INSTRUCTOR_ROLE_NAME
)
return _has_access
cms/djangoapps/contentstore/views/assets.py
...@@ -21,13 +21,13 @@ from xmodule.modulestore import InvalidLocationError ...@@ -21,13 +21,13 @@ from xmodule.modulestore import InvalidLocationError
from xmodule.exceptions import NotFoundError from xmodule.exceptions import NotFoundError
from django.core.exceptions import PermissionDenied from django.core.exceptions import PermissionDenied
from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.django import loc_mapper
from .access import has_access
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from util.json_request import JsonResponse from util.json_request import JsonResponse
from django.http import HttpResponseNotFound from django.http import HttpResponseNotFound
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from pymongo import ASCENDING, DESCENDING from pymongo import ASCENDING, DESCENDING
from .access import has_course_access
__all__ = ['assets_handler'] __all__ = ['assets_handler']
...@@ -56,7 +56,7 @@ def assets_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -56,7 +56,7 @@ def assets_handler(request, tag=None, package_id=None, branch=None, version_guid
json: delete an asset json: delete an asset
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
response_format = request.REQUEST.get('format', 'html') response_format = request.REQUEST.get('format', 'html')
......
cms/djangoapps/contentstore/views/checklist.py
...@@ -15,7 +15,7 @@ from xmodule.modulestore.inheritance import own_metadata ...@@ -15,7 +15,7 @@ from xmodule.modulestore.inheritance import own_metadata
from ..utils import get_modulestore from ..utils import get_modulestore
from .access import has_access from .access import has_course_access
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
...@@ -37,7 +37,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_ ...@@ -37,7 +37,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
json: updates the checked state for items within a particular checklist. checklist_index is required. json: updates the checked state for items within a particular checklist. checklist_index is required.
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(location) old_location = loc_mapper().translate_locator_to_location(location)
......
cms/djangoapps/contentstore/views/component.py
...@@ -29,7 +29,7 @@ from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitSt ...@@ -29,7 +29,7 @@ from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitSt
from models.settings.course_grading import CourseGradingModel from models.settings.course_grading import CourseGradingModel
from .access import has_access from .access import has_course_access
__all__ = ['OPEN_ENDED_COMPONENT_TYPES', __all__ = ['OPEN_ENDED_COMPONENT_TYPES',
'ADVANCED_COMPONENT_POLICY_KEY', 'ADVANCED_COMPONENT_POLICY_KEY',
...@@ -311,7 +311,7 @@ def _get_item_in_course(request, locator): ...@@ -311,7 +311,7 @@ def _get_item_in_course(request, locator):
Verifies that the caller has permission to access this item. Verifies that the caller has permission to access this item.
""" """
if not has_access(request.user, locator): if not has_course_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
......
cms/djangoapps/contentstore/views/course.py
...@@ -35,10 +35,9 @@ from models.settings.course_details import CourseDetails, CourseSettingsEncoder ...@@ -35,10 +35,9 @@ from models.settings.course_details import CourseDetails, CourseSettingsEncoder
from models.settings.course_grading import CourseGradingModel from models.settings.course_grading import CourseGradingModel
from models.settings.course_metadata import CourseMetadata from models.settings.course_metadata import CourseMetadata
from auth.authz import create_all_course_groups, is_user_in_creator_group
from util.json_request import expect_json from util.json_request import expect_json
from .access import has_access from .access import has_course_access
from .tabs import initialize_course_tabs from .tabs import initialize_course_tabs
from .component import ( from .component import (
OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES, OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES,
...@@ -52,6 +51,8 @@ from xmodule.html_module import AboutDescriptor ...@@ -52,6 +51,8 @@ from xmodule.html_module import AboutDescriptor
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from course_creators.views import get_course_creator_status, add_user_with_status_unrequested from course_creators.views import get_course_creator_status, add_user_with_status_unrequested
from contentstore import utils from contentstore import utils
from student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole
from student import auth
__all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler',
'settings_handler', 'settings_handler',
...@@ -66,7 +67,7 @@ def _get_locator_and_course(package_id, branch, version_guid, block_id, user, de ...@@ -66,7 +67,7 @@ def _get_locator_and_course(package_id, branch, version_guid, block_id, user, de
for the view functions in this file. for the view functions in this file.
""" """
locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block_id) locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block_id)
if not has_access(user, locator): if not has_course_access(user, locator):
raise PermissionDenied() raise PermissionDenied()
course_location = loc_mapper().translate_locator_to_location(locator) course_location = loc_mapper().translate_locator_to_location(locator)
course_module = modulestore().get_item(course_location, depth=depth) course_module = modulestore().get_item(course_location, depth=depth)
...@@ -102,7 +103,7 @@ def course_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -102,7 +103,7 @@ def course_handler(request, tag=None, package_id=None, branch=None, version_guid
raise NotImplementedError('coming soon') raise NotImplementedError('coming soon')
elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access
return create_new_course(request) return create_new_course(request)
elif not has_access( elif not has_course_access(
request.user, request.user,
BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
): ):
...@@ -136,7 +137,7 @@ def course_listing(request): ...@@ -136,7 +137,7 @@ def course_listing(request):
""" """
Get courses to which this user has access Get courses to which this user has access
""" """
return (has_access(request.user, course.location) return (has_course_access(request.user, course.location)
# pylint: disable=fixme # pylint: disable=fixme
# TODO remove this condition when templates purged from db # TODO remove this condition when templates purged from db
and course.location.course != 'templates' and course.location.course != 'templates'
...@@ -207,7 +208,7 @@ def create_new_course(request): ...@@ -207,7 +208,7 @@ def create_new_course(request):
Returns the URL for the course overview page. Returns the URL for the course overview page.
""" """
if not is_user_in_creator_group(request.user): if not auth.has_access(request.user, CourseCreatorRole()):
raise PermissionDenied() raise PermissionDenied()
org = request.json.get('org') org = request.json.get('org')
...@@ -297,7 +298,10 @@ def create_new_course(request): ...@@ -297,7 +298,10 @@ def create_new_course(request):
initialize_course_tabs(new_course) initialize_course_tabs(new_course)
new_location = loc_mapper().translate_location(new_course.location.course_id, new_course.location, False, True) new_location = loc_mapper().translate_location(new_course.location.course_id, new_course.location, False, True)
create_all_course_groups(request.user, new_location) # can't use auth.add_users here b/c it requires request.user to already have Instructor perms in this course
# however, we can assume that b/c this user had authority to create the course, the user can add themselves
CourseInstructorRole(new_location).add_users(request.user)
auth.add_users(request.user, CourseStaffRole(new_location), request.user)
# seed the forums # seed the forums
seed_permissions_roles(new_course.location.course_id) seed_permissions_roles(new_course.location.course_id)
...@@ -370,7 +374,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None, ...@@ -370,7 +374,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None,
provided_id = None provided_id = None
# check that logged in user has permissions to this item (GET shouldn't require this level?) # check that logged in user has permissions to this item (GET shouldn't require this level?)
if not has_access(request.user, updates_location): if not has_course_access(request.user, updates_location):
raise PermissionDenied() raise PermissionDenied()
if request.method == 'GET': if request.method == 'GET':
......
cms/djangoapps/contentstore/views/import_export.py
...@@ -22,7 +22,6 @@ from django.views.decorators.http import require_http_methods, require_GET ...@@ -22,7 +22,6 @@ from django.views.decorators.http import require_http_methods, require_GET
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from edxmako.shortcuts import render_to_response from edxmako.shortcuts import render_to_response
from auth.authz import create_all_course_groups
from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.contentstore.django import contentstore from xmodule.contentstore.django import contentstore
...@@ -31,10 +30,12 @@ from xmodule.modulestore.django import modulestore, loc_mapper ...@@ -31,10 +30,12 @@ from xmodule.modulestore.django import modulestore, loc_mapper
from xmodule.exceptions import SerializationError from xmodule.exceptions import SerializationError
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from .access import has_access from .access import has_course_access
from util.json_request import JsonResponse from util.json_request import JsonResponse
from extract_tar import safetar_extractall from extract_tar import safetar_extractall
from student.roles import CourseInstructorRole, CourseStaffRole
from student import auth
__all__ = ['import_handler', 'import_status_handler', 'export_handler'] __all__ = ['import_handler', 'import_status_handler', 'export_handler']
...@@ -61,7 +62,7 @@ def import_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -61,7 +62,7 @@ def import_handler(request, tag=None, package_id=None, branch=None, version_guid
json: import a course via the .tar.gz file specified in request.FILES json: import a course via the .tar.gz file specified in request.FILES
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(location) old_location = loc_mapper().translate_locator_to_location(location)
...@@ -225,13 +226,15 @@ def import_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -225,13 +226,15 @@ def import_handler(request, tag=None, package_id=None, branch=None, version_guid
draft_store=modulestore() draft_store=modulestore()
) )
logging.debug('new course at {0}'.format(course_items[0].location)) new_location = course_items[0].location
logging.debug('new course at {0}'.format(new_location))
session_status[key] = 3 session_status[key] = 3
request.session.modified = True request.session.modified = True
create_all_course_groups(request.user, course_items[0].location) auth.add_users(request.user, CourseInstructorRole(new_location), request.user)
logging.debug('created all course groups at {0}'.format(course_items[0].location)) auth.add_users(request.user, CourseStaffRole(new_location), request.user)
logging.debug('created all course groups at {0}'.format(new_location))
# Send errors to client with stage at which error occured. # Send errors to client with stage at which error occured.
except Exception as exception: # pylint: disable=W0703 except Exception as exception: # pylint: disable=W0703
...@@ -272,7 +275,7 @@ def import_status_handler(request, tag=None, package_id=None, branch=None, versi ...@@ -272,7 +275,7 @@ def import_status_handler(request, tag=None, package_id=None, branch=None, versi
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
try: try:
...@@ -303,7 +306,7 @@ def export_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -303,7 +306,7 @@ def export_handler(request, tag=None, package_id=None, branch=None, version_guid
which describes the error. which describes the error.
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(location) old_location = loc_mapper().translate_locator_to_location(location)
......
cms/djangoapps/contentstore/views/item.py
...@@ -28,7 +28,7 @@ from ..transcripts_utils import manage_video_subtitles_save ...@@ -28,7 +28,7 @@ from ..transcripts_utils import manage_video_subtitles_save
from ..utils import get_modulestore from ..utils import get_modulestore
from .access import has_access from .access import has_course_access
from .helpers import _xmodule_recurse from .helpers import _xmodule_recurse
from preview import handler_prefix, get_preview_html from preview import handler_prefix, get_preview_html
from edxmako.shortcuts import render_to_response, render_to_string from edxmako.shortcuts import render_to_response, render_to_string
...@@ -81,7 +81,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -81,7 +81,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
""" """
if package_id is not None: if package_id is not None:
locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, locator): if not has_course_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
...@@ -250,7 +250,7 @@ def _create_item(request): ...@@ -250,7 +250,7 @@ def _create_item(request):
display_name = request.json.get('display_name') display_name = request.json.get('display_name')
if not has_access(request.user, parent_location): if not has_course_access(request.user, parent_location):
raise PermissionDenied() raise PermissionDenied()
parent = get_modulestore(category).get_item(parent_location) parent = get_modulestore(category).get_item(parent_location)
...@@ -335,7 +335,7 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid ...@@ -335,7 +335,7 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid
# DHM: when split becomes back-end, move or conditionalize this conversion # DHM: when split becomes back-end, move or conditionalize this conversion
old_location = loc_mapper().translate_locator_to_location(location) old_location = loc_mapper().translate_locator_to_location(location)
if request.method == 'GET': if request.method == 'GET':
if has_access(request.user, old_location): if has_course_access(request.user, old_location):
return JsonResponse(modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft')) return JsonResponse(modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft'))
else: else:
raise PermissionDenied() raise PermissionDenied()
......
cms/djangoapps/contentstore/views/tabs.py
""" """
Views related to course tabs Views related to course tabs
""" """
from access import has_access from access import has_course_access
from util.json_request import expect_json, JsonResponse from util.json_request import expect_json, JsonResponse
from django.http import HttpResponseNotFound from django.http import HttpResponseNotFound
...@@ -63,7 +63,7 @@ def tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N ...@@ -63,7 +63,7 @@ def tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N
Instead use the general xblock URL (see item.xblock_handler). Instead use the general xblock URL (see item.xblock_handler).
""" """
locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, locator): if not has_course_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
......
cms/djangoapps/contentstore/views/transcripts_ajax.py
...@@ -37,7 +37,7 @@ from ..transcripts_utils import ( ...@@ -37,7 +37,7 @@ from ..transcripts_utils import (
TranscriptsRequestValidationException TranscriptsRequestValidationException
) )
from .access import has_access from .access import has_course_access
__all__ = [ __all__ = [
'upload_transcripts', 'upload_transcripts',
...@@ -546,11 +546,11 @@ def _get_item(request, data): ...@@ -546,11 +546,11 @@ def _get_item(request, data):
locator = BlockUsageLocator(data.get('locator')) locator = BlockUsageLocator(data.get('locator'))
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
# This is placed before has_access() to validate the location, # This is placed before has_course_access() to validate the location,
# because has_access() raises InvalidLocationError if location is invalid. # because has_course_access() raises InvalidLocationError if location is invalid.
item = modulestore().get_item(old_location) item = modulestore().get_item(old_location)
if not has_access(request.user, locator): if not has_course_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
return item return item
cms/djangoapps/contentstore/views/user.py
import json
from django.core.exceptions import PermissionDenied from django.core.exceptions import PermissionDenied
from django.contrib.auth.models import User, Group from django.contrib.auth.models import User
from django.contrib.auth.decorators import login_required from django.contrib.auth.decorators import login_required
from django.views.decorators.http import require_http_methods from django.views.decorators.http import require_http_methods
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
...@@ -10,17 +9,15 @@ from edxmako.shortcuts import render_to_response ...@@ -10,17 +9,15 @@ from edxmako.shortcuts import render_to_response
from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.django import modulestore, loc_mapper
from util.json_request import JsonResponse, expect_json from util.json_request import JsonResponse, expect_json
from auth.authz import ( from student.roles import CourseRole, CourseInstructorRole, CourseStaffRole, GlobalStaff
STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME, get_course_groupname_for_role,
get_course_role_users
)
from course_creators.views import user_requested_access from course_creators.views import user_requested_access
from .access import has_access from .access import has_course_access
from student.models import CourseEnrollment from student.models import CourseEnrollment
from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore.locator import BlockUsageLocator
from django.http import HttpResponseNotFound from django.http import HttpResponseNotFound
from student import auth
__all__ = ['request_course_creator', 'course_team_handler'] __all__ = ['request_course_creator', 'course_team_handler']
...@@ -53,7 +50,7 @@ def course_team_handler(request, tag=None, package_id=None, branch=None, version ...@@ -53,7 +50,7 @@ def course_team_handler(request, tag=None, package_id=None, branch=None, version
json: remove a particular course team member from the course team (email is required). json: remove a particular course team member from the course team (email is required).
""" """
location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) location = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
if not has_access(request.user, location): if not has_course_access(request.user, location):
raise PermissionDenied() raise PermissionDenied()
if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
...@@ -71,19 +68,19 @@ def _manage_users(request, locator): ...@@ -71,19 +68,19 @@ def _manage_users(request, locator):
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
# check that logged in user has permissions to this item # check that logged in user has permissions to this item
if not has_access(request.user, locator): if not has_course_access(request.user, locator):
raise PermissionDenied() raise PermissionDenied()
course_module = modulestore().get_item(old_location) course_module = modulestore().get_item(old_location)
instructors = get_course_role_users(locator, INSTRUCTOR_ROLE_NAME) instructors = CourseInstructorRole(locator).users_with_role()
# the page only lists staff and assumes they're a superset of instructors. Do a union to ensure. # the page only lists staff and assumes they're a superset of instructors. Do a union to ensure.
staff = set(get_course_role_users(locator, STAFF_ROLE_NAME)).union(instructors) staff = set(CourseStaffRole(locator).users_with_role()).union(instructors)
return render_to_response('manage_users.html', { return render_to_response('manage_users.html', {
'context_course': course_module, 'context_course': course_module,
'staff': staff, 'staff': staff,
'instructors': instructors, 'instructors': instructors,
'allow_actions': has_access(request.user, locator, role=INSTRUCTOR_ROLE_NAME), 'allow_actions': has_course_access(request.user, locator, role=CourseInstructorRole),
}) })
...@@ -93,10 +90,10 @@ def _course_team_user(request, locator, email): ...@@ -93,10 +90,10 @@ def _course_team_user(request, locator, email):
Handle the add, remove, promote, demote requests ensuring the requester has authority Handle the add, remove, promote, demote requests ensuring the requester has authority
""" """
# check that logged in user has permissions to this item # check that logged in user has permissions to this item
if has_access(request.user, locator, role=INSTRUCTOR_ROLE_NAME): if has_course_access(request.user, locator, role=CourseInstructorRole):
# instructors have full permissions # instructors have full permissions
pass pass
elif has_access(request.user, locator, role=STAFF_ROLE_NAME) and email == request.user.email: elif has_course_access(request.user, locator, role=CourseStaffRole) and email == request.user.email:
# staff can only affect themselves # staff can only affect themselves
pass pass
else: else:
...@@ -107,15 +104,13 @@ def _course_team_user(request, locator, email): ...@@ -107,15 +104,13 @@ def _course_team_user(request, locator, email):
try: try:
user = User.objects.get(email=email) user = User.objects.get(email=email)
except: except Exception:
msg = { msg = {
"error": _("Could not find user by email address '{email}'.").format(email=email), "error": _("Could not find user by email address '{email}'.").format(email=email),
} }
return JsonResponse(msg, 404) return JsonResponse(msg, 404)
# role hierarchy: "instructor" has more permissions than "staff" (in a course) # role hierarchy: globalstaff > "instructor" > "staff" (in a course)
roles = ["instructor", "staff"]
if request.method == "GET": if request.method == "GET":
# just return info about the user # just return info about the user
msg = { msg = {
...@@ -123,12 +118,10 @@ def _course_team_user(request, locator, email): ...@@ -123,12 +118,10 @@ def _course_team_user(request, locator, email):
"active": user.is_active, "active": user.is_active,
"role": None, "role": None,
} }
# what's the highest role that this user has? # what's the highest role that this user has? (How should this report global staff?)
groupnames = set(g.name for g in user.groups.all()) for role in [CourseInstructorRole(locator), CourseStaffRole(locator)]:
for role in roles: if role.has_user(user):
role_groupname = get_course_groupname_for_role(locator, role) msg["role"] = role.ROLE
if role_groupname in groupnames:
msg["role"] = role
break break
return JsonResponse(msg) return JsonResponse(msg)
...@@ -139,29 +132,13 @@ def _course_team_user(request, locator, email): ...@@ -139,29 +132,13 @@ def _course_team_user(request, locator, email):
} }
return JsonResponse(msg, 400) return JsonResponse(msg, 400)
# make sure that the role groups exist
groups = {}
for role in roles:
groupname = get_course_groupname_for_role(locator, role)
group, __ = Group.objects.get_or_create(name=groupname)
groups[role] = group
if request.method == "DELETE": if request.method == "DELETE":
# remove all roles in this course from this user: but fail if the user try:
# is the last instructor in the course team try_remove_instructor(request, locator, user)
instructors = set(groups["instructor"].user_set.all()) except CannotOrphanCourse as oops:
staff = set(groups["staff"].user_set.all()) return JsonResponse(oops.msg, 400)
if user in instructors and len(instructors) == 1:
msg = {
"error": _("You may not remove the last instructor from a course")
}
return JsonResponse(msg, 400)
if user in instructors: auth.remove_users(request.user, CourseStaffRole(locator), user)
user.groups.remove(groups["instructor"])
if user in staff:
user.groups.remove(groups["staff"])
user.save()
return JsonResponse() return JsonResponse()
# all other operations require the requesting user to specify a role # all other operations require the requesting user to specify a role
...@@ -171,29 +148,45 @@ def _course_team_user(request, locator, email): ...@@ -171,29 +148,45 @@ def _course_team_user(request, locator, email):
old_location = loc_mapper().translate_locator_to_location(locator) old_location = loc_mapper().translate_locator_to_location(locator)
if role == "instructor": if role == "instructor":
if not has_access(request.user, locator, role=INSTRUCTOR_ROLE_NAME): if not has_course_access(request.user, locator, role=CourseInstructorRole):
msg = { msg = {
"error": _("Only instructors may create other instructors") "error": _("Only instructors may create other instructors")
} }
return JsonResponse(msg, 400) return JsonResponse(msg, 400)
user.groups.add(groups["instructor"]) auth.add_users(request.user, CourseInstructorRole(locator), user)
user.save()
# auto-enroll the course creator in the course so that "View Live" will work. # auto-enroll the course creator in the course so that "View Live" will work.
CourseEnrollment.enroll(user, old_location.course_id) CourseEnrollment.enroll(user, old_location.course_id)
elif role == "staff": elif role == "staff":
# if we're trying to downgrade a user from "instructor" to "staff", # add to staff regardless (can't do after removing from instructors as will no longer
# make sure we have at least one other instructor in the course team. # be allowed)
instructors = set(groups["instructor"].user_set.all()) auth.add_users(request.user, CourseStaffRole(locator), user)
if user in instructors: try:
if len(instructors) == 1: try_remove_instructor(request, locator, user)
msg = { except CannotOrphanCourse as oops:
"error": _("You may not remove the last instructor from a course") return JsonResponse(oops.msg, 400)
}
return JsonResponse(msg, 400)
user.groups.remove(groups["instructor"])
user.groups.add(groups["staff"])
user.save()
# auto-enroll the course creator in the course so that "View Live" will work. # auto-enroll the course creator in the course so that "View Live" will work.
CourseEnrollment.enroll(user, old_location.course_id) CourseEnrollment.enroll(user, old_location.course_id)
return JsonResponse() return JsonResponse()
class CannotOrphanCourse(Exception):
"""
Exception raised if an attempt is made to remove all responsible instructors from course.
"""
def __init__(self, msg):
self.msg = msg
Exception.__init__(self)
def try_remove_instructor(request, locator, user):
# remove all roles in this course from this user: but fail if the user
# is the last instructor in the course team
instructors = CourseInstructorRole(locator)
if instructors.has_user(user):
if instructors.users_with_role().count() == 1:
msg = {"error":_("You may not remove the last instructor from a course")}
raise CannotOrphanCourse(msg)
else:
auth.remove_users(request.user, instructors, user)
cms/djangoapps/course_creators/tests/test_admin.py
...@@ -10,8 +10,9 @@ import mock ...@@ -10,8 +10,9 @@ import mock
from course_creators.admin import CourseCreatorAdmin from course_creators.admin import CourseCreatorAdmin
from course_creators.models import CourseCreator from course_creators.models import CourseCreator
from auth.authz import is_user_in_creator_group
from django.core import mail from django.core import mail
from student.roles import CourseCreatorRole
from student import auth
def mock_render_to_string(template_name, context): def mock_render_to_string(template_name, context):
...@@ -54,7 +55,7 @@ class CourseCreatorAdminTest(TestCase): ...@@ -54,7 +55,7 @@ class CourseCreatorAdminTest(TestCase):
def change_state_and_verify_email(state, is_creator): def change_state_and_verify_email(state, is_creator):
""" Changes user state, verifies creator status, and verifies e-mail is sent based on transition """ """ Changes user state, verifies creator status, and verifies e-mail is sent based on transition """
self._change_state(state) self._change_state(state)
self.assertEqual(is_creator, is_user_in_creator_group(self.user)) self.assertEqual(is_creator, auth.has_access(self.user, CourseCreatorRole()))
context = {'studio_request_email': self.studio_request_email} context = {'studio_request_email': self.studio_request_email}
if state == CourseCreator.GRANTED: if state == CourseCreator.GRANTED:
...@@ -72,7 +73,7 @@ class CourseCreatorAdminTest(TestCase): ...@@ -72,7 +73,7 @@ class CourseCreatorAdminTest(TestCase):
with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch):
# User is initially unrequested. # User is initially unrequested.
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
change_state_and_verify_email(CourseCreator.GRANTED, True) change_state_and_verify_email(CourseCreator.GRANTED, True)
......
cms/djangoapps/course_creators/tests/test_views.py
...@@ -8,9 +8,9 @@ from django.core.exceptions import PermissionDenied ...@@ -8,9 +8,9 @@ from django.core.exceptions import PermissionDenied
from course_creators.views import add_user_with_status_unrequested, add_user_with_status_granted from course_creators.views import add_user_with_status_unrequested, add_user_with_status_granted
from course_creators.views import get_course_creator_status, update_course_creator_group, user_requested_access from course_creators.views import get_course_creator_status, update_course_creator_group, user_requested_access
from course_creators.models import CourseCreator
from auth.authz import is_user_in_creator_group
import mock import mock
from student.roles import CourseCreatorRole
from student import auth
class CourseCreatorView(TestCase): class CourseCreatorView(TestCase):
...@@ -48,7 +48,7 @@ class CourseCreatorView(TestCase): ...@@ -48,7 +48,7 @@ class CourseCreatorView(TestCase):
def test_add_granted(self): def test_add_granted(self):
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
# Calling add_user_with_status_granted impacts is_user_in_course_group_role. # Calling add_user_with_status_granted impacts is_user_in_course_group_role.
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
add_user_with_status_granted(self.admin, self.user) add_user_with_status_granted(self.admin, self.user)
self.assertEqual('granted', get_course_creator_status(self.user)) self.assertEqual('granted', get_course_creator_status(self.user))
...@@ -57,15 +57,15 @@ class CourseCreatorView(TestCase): ...@@ -57,15 +57,15 @@ class CourseCreatorView(TestCase):
add_user_with_status_unrequested(self.user) add_user_with_status_unrequested(self.user)
self.assertEqual('granted', get_course_creator_status(self.user)) self.assertEqual('granted', get_course_creator_status(self.user))
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(auth.has_access(self.user, CourseCreatorRole()))
def test_update_creator_group(self): def test_update_creator_group(self):
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
update_course_creator_group(self.admin, self.user, True) update_course_creator_group(self.admin, self.user, True)
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(auth.has_access(self.user, CourseCreatorRole()))
update_course_creator_group(self.admin, self.user, False) update_course_creator_group(self.admin, self.user, False)
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
def test_user_requested_access(self): def test_user_requested_access(self):
add_user_with_status_unrequested(self.user) add_user_with_status_unrequested(self.user)
......
cms/djangoapps/course_creators/views.py
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
Methods for interacting programmatically with the user creator table. Methods for interacting programmatically with the user creator table.
""" """
from course_creators.models import CourseCreator from course_creators.models import CourseCreator
from student.roles import CourseCreatorRole
from auth.authz import add_user_to_creator_group, remove_user_from_creator_group from student import auth
def add_user_with_status_unrequested(user): def add_user_with_status_unrequested(user):
...@@ -43,9 +43,9 @@ def update_course_creator_group(caller, user, add): ...@@ -43,9 +43,9 @@ def update_course_creator_group(caller, user, add):
Caller must have staff permissions. Caller must have staff permissions.
""" """
if add: if add:
add_user_to_creator_group(caller, user) auth.add_users(caller, CourseCreatorRole(), user)
else: else:
remove_user_from_creator_group(caller, user) auth.remove_users(caller, CourseCreatorRole(), user)
def get_course_creator_status(user): def get_course_creator_status(user):
......
cms/envs/common.py
...@@ -203,7 +203,7 @@ TEMPLATE_DEBUG = False ...@@ -203,7 +203,7 @@ TEMPLATE_DEBUG = False
# Site info # Site info
SITE_ID = 1 SITE_ID = 1
SITE_NAME = "localhost:8000" SITE_NAME = "localhost:8001"
HTTPS = 'on' HTTPS = 'on'
ROOT_URLCONF = 'cms.urls' ROOT_URLCONF = 'cms.urls'
IGNORABLE_404_ENDS = ('favicon.ico') IGNORABLE_404_ENDS = ('favicon.ico')
...@@ -409,7 +409,6 @@ INSTALLED_APPS = ( ...@@ -409,7 +409,6 @@ INSTALLED_APPS = (
# For CMS # For CMS
'contentstore', 'contentstore',
'auth',
'course_creators', 'course_creators',
'student', # misleading name due to sharing with lms 'student', # misleading name due to sharing with lms
'course_groups', # not used in cms (yet), but tests run 'course_groups', # not used in cms (yet), but tests run
......
cms/templates/manage_users.html
<%! import json %>
<%! from django.utils.translation import ugettext as _ %> <%! from django.utils.translation import ugettext as _ %>
<%! from django.core.urlresolvers import reverse %> <%! from django.core.urlresolvers import reverse %>
<%! from auth.authz import is_user_in_course_group_role %> <%! from student.roles import CourseInstructorRole %>
<%! import json %>
<%! from xmodule.modulestore.django import loc_mapper %> <%! from xmodule.modulestore.django import loc_mapper %>
<%inherit file="base.html" /> <%inherit file="base.html" />
<%block name="title">${_("Course Team Settings")}</%block> <%block name="title">${_("Course Team Settings")}</%block>
...@@ -65,7 +65,7 @@ ...@@ -65,7 +65,7 @@
<li class="user-item" data-email="${user.email}" data-url="${new_location.url_reverse('course_team/', user.email) }"> <li class="user-item" data-email="${user.email}" data-url="${new_location.url_reverse('course_team/', user.email) }">
<% is_instuctor = is_user_in_course_group_role(user, context_course.location, 'instructor', check_staff=False) %> <% is_instuctor = CourseInstructorRole(context_course.location).has_user(user) %>
% if is_instuctor: % if is_instuctor:
<span class="wrapper-ui-badge"> <span class="wrapper-ui-badge">
<span class="flag flag-role flag-role-admin is-hanging"> <span class="flag flag-role flag-role-admin is-hanging">
...@@ -120,7 +120,7 @@ ...@@ -120,7 +120,7 @@
% endfor % endfor
</ol> </ol>
<% user_is_instuctor = is_user_in_course_group_role(request.user, context_course.location, 'instructor', check_staff=False) %> <% user_is_instuctor = CourseInstructorRole(context_course.location).has_user(request.user) %>
% if user_is_instuctor and len(staff) == 1: % if user_is_instuctor and len(staff) == 1:
<div class="notice notice-incontext notice-create has-actions"> <div class="notice notice-incontext notice-create has-actions">
<div class="msg"> <div class="msg">
......
common/djangoapps/student/auth.py 0 → 100644
"""
The application interface to roles which checks whether any user trying to change
authorization has authorization to do so, which infers authorization via role hierarchy
(GlobalStaff is superset of auths of course instructor, ...), which consults the config
to decide whether to check course creator role, and other such functions.
"""
from django.core.exceptions import PermissionDenied
from django.conf import settings
from student.roles import GlobalStaff, CourseCreatorRole, CourseStaffRole, CourseInstructorRole, CourseRole, \
CourseBetaTesterRole
def has_access(user, role):
"""
Check whether this user has access to this role (either direct or implied)
:param user:
:param role: an AccessRole
"""
if not user.is_active:
return False
# do cheapest check first even tho it's not the direct one
if GlobalStaff().has_user(user):
return True
# CourseCreator is odd b/c it can be disabled via config
if isinstance(role, CourseCreatorRole):
# completely shut down course creation setting
if settings.FEATURES.get('DISABLE_COURSE_CREATION', False):
return False
# wide open course creation setting
if not settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
return True
if role.has_user(user):
return True
# if not, then check inferred permissions
if (isinstance(role, (CourseStaffRole, CourseBetaTesterRole)) and
CourseInstructorRole(role.location).has_user(user)):
return True
return False
def add_users(caller, role, *users):
"""
The caller requests adding the given users to the role. Checks that the caller
has sufficient authority.
:param caller: a user
:param role: an AccessRole
"""
_check_caller_authority(caller, role)
role.add_users(*users)
def remove_users(caller, role, *users):
"""
The caller requests removing the given users from the role. Checks that the caller
has sufficient authority.
:param caller: a user
:param role: an AccessRole
"""
# can always remove self (at this layer)
if not(len(users) == 1 and caller == users[0]):
_check_caller_authority(caller, role)
role.remove_users(*users)
def _check_caller_authority(caller, role):
"""
Internal function to check whether the caller has authority to manipulate this role
:param caller: a user
:param role: an AccessRole
"""
if not (caller.is_authenticated and caller.is_active):
raise PermissionDenied
# superuser
if GlobalStaff().has_user(caller):
return
if isinstance(role, (GlobalStaff, CourseCreatorRole)):
raise PermissionDenied
elif isinstance(role, CourseRole): # instructors can change the roles w/in their course
if not has_access(caller, CourseInstructorRole(role.location)):
raise PermissionDenied
lms/djangoapps/courseware/roles.py → common/djangoapps/student/roles.py
...@@ -64,11 +64,13 @@ class GlobalStaff(AccessRole): ...@@ -64,11 +64,13 @@ class GlobalStaff(AccessRole):
def add_users(self, *users): def add_users(self, *users):
for user in users: for user in users:
user.is_staff = True if (user.is_authenticated and user.is_active):
user.save() user.is_staff = True
user.save()
def remove_users(self, *users): def remove_users(self, *users):
for user in users: for user in users:
# don't check is_authenticated nor is_active on purpose
user.is_staff = False user.is_staff = False
user.save() user.save()
...@@ -96,10 +98,10 @@ class GroupBasedRole(AccessRole): ...@@ -96,10 +98,10 @@ class GroupBasedRole(AccessRole):
""" """
Return whether the supplied django user has access to this role. Return whether the supplied django user has access to this role.
""" """
# pylint: disable=protected-access if not (user.is_authenticated and user.is_active):
if not user.is_authenticated():
return False return False
# pylint: disable=protected-access
if not hasattr(user, '_groups'): if not hasattr(user, '_groups'):
user._groups = set(name.lower() for name in user.groups.values_list('name', flat=True)) user._groups = set(name.lower() for name in user.groups.values_list('name', flat=True))
...@@ -109,8 +111,12 @@ class GroupBasedRole(AccessRole): ...@@ -109,8 +111,12 @@ class GroupBasedRole(AccessRole):
""" """
Add the supplied django users to this role. Add the supplied django users to this role.
""" """
# silently ignores anonymous and inactive users so that any that are
# legit get updated.
users = [user for user in users if user.is_authenticated and user.is_active]
group, _ = Group.objects.get_or_create(name=self._group_names[0]) group, _ = Group.objects.get_or_create(name=self._group_names[0])
group.user_set.add(*users) group.user_set.add(*users)
# remove cache
for user in users: for user in users:
if hasattr(user, '_groups'): if hasattr(user, '_groups'):
del user._groups del user._groups
...@@ -119,8 +125,10 @@ class GroupBasedRole(AccessRole): ...@@ -119,8 +125,10 @@ class GroupBasedRole(AccessRole):
""" """
Remove the supplied django users from this role. Remove the supplied django users from this role.
""" """
group, _ = Group.objects.get_or_create(name=self._group_names[0]) groups = Group.objects.filter(name__in=self._group_names)
group.user_set.remove(*users) for group in groups:
group.user_set.remove(*users)
# remove cache
for user in users: for user in users:
if hasattr(user, '_groups'): if hasattr(user, '_groups'):
del user._groups del user._groups
...@@ -144,31 +152,33 @@ class CourseRole(GroupBasedRole): ...@@ -144,31 +152,33 @@ class CourseRole(GroupBasedRole):
""" """
# TODO: figure out how to make the group name generation lazy so it doesn't force the # TODO: figure out how to make the group name generation lazy so it doesn't force the
# loc mapping? # loc mapping?
location = Locator.to_locator_or_location(location) self.location = Locator.to_locator_or_location(location)
self.role = role
# direct copy from auth.authz.get_all_course_role_groupnames will refactor to one impl asap # direct copy from auth.authz.get_all_course_role_groupnames will refactor to one impl asap
groupnames = [] groupnames = []
# pylint: disable=no-member # pylint: disable=no-member
if isinstance(location, Location): if isinstance(self.location, Location):
try: try:
groupnames.append('{0}_{1}'.format(role, location.course_id)) groupnames.append('{0}_{1}'.format(role, self.location.course_id))
course_context = self.location.course_id # course_id is valid for translation
except InvalidLocationError: # will occur on old locations where location is not of category course except InvalidLocationError: # will occur on old locations where location is not of category course
if course_context is None: if course_context is None:
raise CourseContextRequired() raise CourseContextRequired()
else: else:
groupnames.append('{0}_{1}'.format(role, course_context)) groupnames.append('{0}_{1}'.format(role, course_context))
try: try:
locator = loc_mapper().translate_location(location.course_id, location, False, False) locator = loc_mapper().translate_location(course_context, self.location, False, False)
groupnames.append('{0}_{1}'.format(role, locator.package_id)) groupnames.append('{0}_{1}'.format(role, locator.package_id))
except (InvalidLocationError, ItemNotFoundError): except (InvalidLocationError, ItemNotFoundError):
# if it's never been mapped, the auth won't be via the Locator syntax # if it's never been mapped, the auth won't be via the Locator syntax
pass pass
# least preferred legacy role_course format # least preferred legacy role_course format
groupnames.append('{0}_{1}'.format(role, location.course)) groupnames.append('{0}_{1}'.format(role, self.location.course))
elif isinstance(location, CourseLocator): elif isinstance(self.location, CourseLocator):
groupnames.append('{0}_{1}'.format(role, location.package_id)) groupnames.append('{0}_{1}'.format(role, self.location.package_id))
# handle old Location syntax # handle old Location syntax
old_location = loc_mapper().translate_locator_to_location(location, get_course=True) old_location = loc_mapper().translate_locator_to_location(self.location, get_course=True)
if old_location: if old_location:
# the slashified version of the course_id (myu/mycourse/myrun) # the slashified version of the course_id (myu/mycourse/myrun)
groupnames.append('{0}_{1}'.format(role, old_location.course_id)) groupnames.append('{0}_{1}'.format(role, old_location.course_id))
...@@ -191,20 +201,23 @@ class OrgRole(GroupBasedRole): ...@@ -191,20 +201,23 @@ class OrgRole(GroupBasedRole):
class CourseStaffRole(CourseRole): class CourseStaffRole(CourseRole):
"""A Staff member of a course""" """A Staff member of a course"""
ROLE = 'staff'
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(CourseStaffRole, self).__init__('staff', *args, **kwargs) super(CourseStaffRole, self).__init__(self.ROLE, *args, **kwargs)
class CourseInstructorRole(CourseRole): class CourseInstructorRole(CourseRole):
"""A course Instructor""" """A course Instructor"""
ROLE = 'instructor'
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(CourseInstructorRole, self).__init__('instructor', *args, **kwargs) super(CourseInstructorRole, self).__init__(self.ROLE, *args, **kwargs)
class CourseBetaTesterRole(CourseRole): class CourseBetaTesterRole(CourseRole):
"""A course Beta Tester""" """A course Beta Tester"""
ROLE = 'beta_testers'
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(CourseBetaTesterRole, self).__init__('beta_testers', *args, **kwargs) super(CourseBetaTesterRole, self).__init__(self.ROLE, *args, **kwargs)
class OrgStaffRole(OrgRole): class OrgStaffRole(OrgRole):
...@@ -217,3 +230,13 @@ class OrgInstructorRole(OrgRole): ...@@ -217,3 +230,13 @@ class OrgInstructorRole(OrgRole):
"""An organization instructor""" """An organization instructor"""
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(OrgInstructorRole, self).__init__('instructor', *args, **kwargs) super(OrgInstructorRole, self).__init__('instructor', *args, **kwargs)
class CourseCreatorRole(GroupBasedRole):
"""
This is the group of people who have permission to create new courses (we may want to eventually
make this an org based role).
"""
ROLE = "course_creator_group"
def __init__(self, *args, **kwargs):
super(CourseCreatorRole, self).__init__(self.ROLE, *args, **kwargs)
cms/djangoapps/auth/tests/test_authz.py → common/djangoapps/student/tests/test_authz.py
...@@ -8,10 +8,9 @@ from django.contrib.auth.models import User ...@@ -8,10 +8,9 @@ from django.contrib.auth.models import User
from xmodule.modulestore import Location from xmodule.modulestore import Location
from django.core.exceptions import PermissionDenied 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,\ from student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole
create_all_course_groups, add_user_to_course_group, STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,\ from student.tests.factories import AdminFactory
is_user_in_course_group_role, remove_user_from_course_group, get_users_with_staff_role,\ from student.auth import has_access, add_users, remove_users
get_users_with_instructor_role
class CreatorGroupTest(TestCase): class CreatorGroupTest(TestCase):
...@@ -27,98 +26,104 @@ class CreatorGroupTest(TestCase): ...@@ -27,98 +26,104 @@ class CreatorGroupTest(TestCase):
def test_creator_group_not_enabled(self): def test_creator_group_not_enabled(self):
""" """
Tests that is_user_in_creator_group always returns True if ENABLE_CREATOR_GROUP Tests that CourseCreatorRole().has_user always returns True if ENABLE_CREATOR_GROUP
and DISABLE_COURSE_CREATION are both not turned on. and DISABLE_COURSE_CREATION are both not turned on.
""" """
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(has_access(self.user, CourseCreatorRole()))
def test_creator_group_enabled_but_empty(self): def test_creator_group_enabled_but_empty(self):
""" Tests creator group feature on, but group empty. """ """ Tests creator group feature on, but group empty. """
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(has_access(self.user, CourseCreatorRole()))
# Make user staff. This will cause is_user_in_creator_group to return True. # Make user staff. This will cause CourseCreatorRole().has_user to return True.
self.user.is_staff = True self.user.is_staff = True
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(has_access(self.user, CourseCreatorRole()))
def test_creator_group_enabled_nonempty(self): def test_creator_group_enabled_nonempty(self):
""" Tests creator group feature on, user added. """ """ Tests creator group feature on, user added. """
with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
self.assertTrue(add_user_to_creator_group(self.admin, self.user)) add_users(self.admin, CourseCreatorRole(), self.user)
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(has_access(self.user, CourseCreatorRole()))
# check that a user who has not been added to the group still returns false # 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') user_not_added = User.objects.create_user('testuser2', 'test+courses2@edx.org', 'foo2')
self.assertFalse(is_user_in_creator_group(user_not_added)) self.assertFalse(has_access(user_not_added, CourseCreatorRole()))
# remove first user from the group and verify that is_user_in_creator_group now returns false # remove first user from the group and verify that CourseCreatorRole().has_user now returns false
remove_user_from_creator_group(self.admin, self.user) remove_users(self.admin, CourseCreatorRole(), self.user)
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(has_access(self.user, CourseCreatorRole()))
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): def test_course_creation_disabled(self):
""" Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """ """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """
with mock.patch.dict('django.conf.settings.FEATURES', with mock.patch.dict('django.conf.settings.FEATURES',
{'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}): {'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}):
# Add user to creator group. # Add user to creator group.
self.assertTrue(add_user_to_creator_group(self.admin, self.user)) add_users(self.admin, CourseCreatorRole(), self.user)
# DISABLE_COURSE_CREATION overrides (user is not marked as staff). # DISABLE_COURSE_CREATION overrides (user is not marked as staff).
self.assertFalse(is_user_in_creator_group(self.user)) self.assertFalse(has_access(self.user, CourseCreatorRole()))
# Mark as staff. Now is_user_in_creator_group returns true. # Mark as staff. Now CourseCreatorRole().has_user returns true.
self.user.is_staff = True self.user.is_staff = True
self.assertTrue(is_user_in_creator_group(self.user)) self.assertTrue(has_access(self.user, CourseCreatorRole()))
# Remove user from creator group. CourseCreatorRole().has_user still returns true because is_staff=True
remove_users(self.admin, CourseCreatorRole(), self.user)
self.assertTrue(has_access(self.user, CourseCreatorRole()))
def test_add_user_not_authenticated(self):
"""
Tests that adding to creator group fails if user is not authenticated
"""
with mock.patch.dict('django.conf.settings.FEATURES',
{'DISABLE_COURSE_CREATION': False, "ENABLE_CREATOR_GROUP": True}):
self.user.is_authenticated = False
add_users(self.admin, CourseCreatorRole(), self.user)
self.assertFalse(has_access(self.user, CourseCreatorRole()))
# Remove user from creator group. is_user_in_creator_group still returns true because is_staff=True def test_add_user_not_active(self):
remove_user_from_creator_group(self.admin, self.user) """
self.assertTrue(is_user_in_creator_group(self.user)) Tests that adding to creator group fails if user is not active
"""
with mock.patch.dict('django.conf.settings.FEATURES',
{'DISABLE_COURSE_CREATION': False, "ENABLE_CREATOR_GROUP": True}):
self.user.is_active = False
add_users(self.admin, CourseCreatorRole(), self.user)
self.assertFalse(has_access(self.user, CourseCreatorRole()))
def test_add_user_to_group_requires_staff_access(self): def test_add_user_to_group_requires_staff_access(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_staff = False self.admin.is_staff = False
add_user_to_creator_group(self.admin, self.user) add_users(self.admin, CourseCreatorRole(), self.user)
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
add_user_to_creator_group(self.user, self.user) add_users(self.user, CourseCreatorRole(), self.user)
def test_add_user_to_group_requires_active(self): def test_add_user_to_group_requires_active(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_active = False self.admin.is_active = False
add_user_to_creator_group(self.admin, self.user) add_users(self.admin, CourseCreatorRole(), self.user)
def test_add_user_to_group_requires_authenticated(self): def test_add_user_to_group_requires_authenticated(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_authenticated = False self.admin.is_authenticated = False
add_user_to_creator_group(self.admin, self.user) add_users(self.admin, CourseCreatorRole(), self.user)
def test_remove_user_from_group_requires_staff_access(self): def test_remove_user_from_group_requires_staff_access(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_staff = False self.admin.is_staff = False
remove_user_from_creator_group(self.admin, self.user) remove_users(self.admin, CourseCreatorRole(), self.user)
def test_remove_user_from_group_requires_active(self): def test_remove_user_from_group_requires_active(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_active = False self.admin.is_active = False
remove_user_from_creator_group(self.admin, self.user) remove_users(self.admin, CourseCreatorRole(), self.user)
def test_remove_user_from_group_requires_authenticated(self): def test_remove_user_from_group_requires_authenticated(self):
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
self.admin.is_authenticated = False self.admin.is_authenticated = False
remove_user_from_creator_group(self.admin, self.user) remove_users(self.admin, CourseCreatorRole(), self.user)
class CourseGroupTest(TestCase): class CourseGroupTest(TestCase):
...@@ -128,6 +133,7 @@ class CourseGroupTest(TestCase): ...@@ -128,6 +133,7 @@ class CourseGroupTest(TestCase):
def setUp(self): def setUp(self):
""" Test case setup """ """ Test case setup """
self.global_admin = AdminFactory()
self.creator = User.objects.create_user('testcreator', 'testcreator+courses@edx.org', 'foo') 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.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo')
self.location = Location('i4x', 'mitX', '101', 'course', 'test') self.location = Location('i4x', 'mitX', '101', 'course', 'test')
...@@ -137,67 +143,47 @@ class CourseGroupTest(TestCase): ...@@ -137,67 +143,47 @@ class CourseGroupTest(TestCase):
Tests adding user to course group (happy path). Tests adding user to course group (happy path).
""" """
# Create groups for a new course (and assign instructor role to the creator). # 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)) self.assertFalse(has_access(self.creator, CourseInstructorRole(self.location)))
create_all_course_groups(self.creator, self.location) add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
self.assertTrue(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME)) add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
self.assertTrue(has_access(self.creator, CourseInstructorRole(self.location)))
# Add another user to the staff role. # Add another user to the staff role.
self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) self.assertFalse(has_access(self.staff, CourseStaffRole(self.location)))
self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)) add_users(self.creator, CourseStaffRole(self.location), self.staff)
self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) self.assertTrue(has_access(self.staff, CourseStaffRole(self.location)))
def test_add_user_to_course_group_permission_denied(self): def test_add_user_to_course_group_permission_denied(self):
""" """
Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role. Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role.
""" """
create_all_course_groups(self.creator, self.location) add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
add_user_to_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME) add_users(self.staff, CourseStaffRole(self.location), self.staff)
def test_remove_user_from_course_group(self): def test_remove_user_from_course_group(self):
""" """
Tests removing user from course group (happy path). Tests removing user from course group (happy path).
""" """
create_all_course_groups(self.creator, self.location) add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
add_users(self.global_admin, CourseStaffRole(self.location), self.creator)
self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)) add_users(self.creator, CourseStaffRole(self.location), self.staff)
self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) self.assertTrue(has_access(self.staff, CourseStaffRole(self.location)))
remove_user_from_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME) remove_users(self.creator, CourseStaffRole(self.location), self.staff)
self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) self.assertFalse(has_access(self.staff, CourseStaffRole(self.location)))
remove_user_from_course_group(self.creator, self.creator, self.location, INSTRUCTOR_ROLE_NAME) remove_users(self.creator, CourseInstructorRole(self.location), self.creator)
self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME)) self.assertFalse(has_access(self.creator, CourseInstructorRole(self.location)))
def test_remove_user_from_course_group_permission_denied(self): def test_remove_user_from_course_group_permission_denied(self):
""" """
Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role. Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role.
""" """
create_all_course_groups(self.creator, self.location) add_users(self.global_admin, CourseInstructorRole(self.location), self.creator)
another_staff = User.objects.create_user('another', 'teststaff+anothercourses@edx.org', 'foo')
add_users(self.global_admin, CourseStaffRole(self.location), self.creator, self.staff, another_staff)
with self.assertRaises(PermissionDenied): with self.assertRaises(PermissionDenied):
remove_user_from_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME) remove_users(self.staff, CourseStaffRole(self.location), another_staff)
def test_get_staff(self):
# Do this test with staff in 2 different classes.
create_all_course_groups(self.creator, self.location)
add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)
location2 = Location('i4x', 'mitX', '103', 'course', 'test2')
staff2 = User.objects.create_user('teststaff2', 'teststaff2+courses@edx.org', 'foo')
create_all_course_groups(self.creator, location2)
add_user_to_course_group(self.creator, staff2, location2, STAFF_ROLE_NAME)
self.assertSetEqual({self.staff, staff2, self.creator}, get_users_with_staff_role())
def test_get_instructor(self):
# Do this test with creators in 2 different classes.
create_all_course_groups(self.creator, self.location)
add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)
location2 = Location('i4x', 'mitX', '103', 'course', 'test2')
creator2 = User.objects.create_user('testcreator2', 'testcreator2+courses@edx.org', 'foo')
staff2 = User.objects.create_user('teststaff2', 'teststaff2+courses@edx.org', 'foo')
create_all_course_groups(creator2, location2)
add_user_to_course_group(creator2, staff2, location2, STAFF_ROLE_NAME)
self.assertSetEqual({self.creator, creator2}, get_users_with_instructor_role())
lms/djangoapps/courseware/tests/test_roles.py → common/djangoapps/student/tests/test_roles.py
""" """
Tests of courseware.roles Tests of student.roles
""" """
from django.test import TestCase from django.test import TestCase
...@@ -8,12 +8,12 @@ from xmodule.modulestore import Location ...@@ -8,12 +8,12 @@ from xmodule.modulestore import Location
from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory
from student.tests.factories import AnonymousUserFactory from student.tests.factories import AnonymousUserFactory
from courseware.roles import GlobalStaff, CourseRole from student.roles import GlobalStaff, CourseRole
class RolesTestCase(TestCase): class RolesTestCase(TestCase):
""" """
Tests of courseware.roles Tests of student.roles
""" """
def setUp(self): def setUp(self):
......
lms/djangoapps/bulk_email/tasks.py
...@@ -37,7 +37,7 @@ from bulk_email.models import ( ...@@ -37,7 +37,7 @@ from bulk_email.models import (
SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS, SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS,
) )
from courseware.courses import get_course, course_image_url from courseware.courses import get_course, course_image_url
from courseware.roles import CourseStaffRole, CourseInstructorRole from student.roles import CourseStaffRole, CourseInstructorRole
from instructor_task.models import InstructorTask from instructor_task.models import InstructorTask
from instructor_task.subtasks import ( from instructor_task.subtasks import (
SubtaskStatus, SubtaskStatus,
......
lms/djangoapps/bulk_email/tests/test_email.py
...@@ -53,7 +53,8 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): ...@@ -53,7 +53,8 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase):
self.instructor = InstructorFactory(course=self.course.location) self.instructor = InstructorFactory(course=self.course.location)
# Create staff # Create staff
self.staff = [StaffFactory(course=self.course.location) for _ in xrange(STAFF_COUNT)] self.staff = [StaffFactory(course=self.course.location)
for _ in xrange(STAFF_COUNT)]
# Create students # Create students
self.students = [UserFactory() for _ in xrange(STUDENT_COUNT)] self.students = [UserFactory() for _ in xrange(STUDENT_COUNT)]
......
lms/djangoapps/course_wiki/tests/test_access.py
...@@ -25,7 +25,10 @@ class TestWikiAccessBase(ModuleStoreTestCase): ...@@ -25,7 +25,10 @@ class TestWikiAccessBase(ModuleStoreTestCase):
self.wiki = get_or_create_root() self.wiki = get_or_create_root()
self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course') self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course')
self.course_math101_staff = [InstructorFactory(course=self.course_math101.location), StaffFactory(course=self.course_math101.location)] self.course_math101_staff = [
InstructorFactory(course=self.course_math101.location),
StaffFactory(course=self.course_math101.location)
]
wiki_math101 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101)) wiki_math101 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101))
wiki_math101_page = self.create_urlpath(wiki_math101, 'Child') wiki_math101_page = self.create_urlpath(wiki_math101, 'Child')
...@@ -44,9 +47,15 @@ class TestWikiAccess(TestWikiAccessBase): ...@@ -44,9 +47,15 @@ class TestWikiAccess(TestWikiAccessBase):
super(TestWikiAccess, self).setUp() super(TestWikiAccess, self).setUp()
self.course_310b = CourseFactory.create(org='org', number='310b', display_name='Course') self.course_310b = CourseFactory.create(org='org', number='310b', display_name='Course')
self.course_310b_staff = [InstructorFactory(course=self.course_310b.location), StaffFactory(course=self.course_310b.location)] self.course_310b_staff = [
InstructorFactory(course=self.course_310b.location),
StaffFactory(course=self.course_310b.location)
]
self.course_310b_ = CourseFactory.create(org='org', number='310b_', display_name='Course') self.course_310b_ = CourseFactory.create(org='org', number='310b_', display_name='Course')
self.course_310b__staff = [InstructorFactory(course=self.course_310b_.location), StaffFactory(course=self.course_310b_.location)] self.course_310b__staff = [
InstructorFactory(course=self.course_310b_.location),
StaffFactory(course=self.course_310b_.location)
]
self.wiki_310b = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b)) self.wiki_310b = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b))
self.wiki_310b_ = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b_)) self.wiki_310b_ = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b_))
...@@ -105,7 +114,10 @@ class TestWikiAccessForNumericalCourseNumber(TestWikiAccessBase): ...@@ -105,7 +114,10 @@ class TestWikiAccessForNumericalCourseNumber(TestWikiAccessBase):
super(TestWikiAccessForNumericalCourseNumber, self).setUp() super(TestWikiAccessForNumericalCourseNumber, self).setUp()
self.course_200 = CourseFactory.create(org='org', number='200', display_name='Course') self.course_200 = CourseFactory.create(org='org', number='200', display_name='Course')
self.course_200_staff = [InstructorFactory(course=self.course_200.location), StaffFactory(course=self.course_200.location)] self.course_200_staff = [
InstructorFactory(course=self.course_200.location),
StaffFactory(course=self.course_200.location)
]
wiki_200 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_200)) wiki_200 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_200))
wiki_200_page = self.create_urlpath(wiki_200, 'Child') wiki_200_page = self.create_urlpath(wiki_200, 'Child')
...@@ -126,7 +138,10 @@ class TestWikiAccessForOldFormatCourseStaffGroups(TestWikiAccessBase): ...@@ -126,7 +138,10 @@ class TestWikiAccessForOldFormatCourseStaffGroups(TestWikiAccessBase):
self.course_math101c = CourseFactory.create(org='org', number='math101c', display_name='Course') self.course_math101c = CourseFactory.create(org='org', number='math101c', display_name='Course')
Group.objects.get_or_create(name='instructor_math101c') Group.objects.get_or_create(name='instructor_math101c')
self.course_math101c_staff = [InstructorFactory(course=self.course_math101c.location), StaffFactory(course=self.course_math101c.location)] self.course_math101c_staff = [
InstructorFactory(course=self.course_math101c.location),
StaffFactory(course=self.course_math101c.location)
]
wiki_math101c = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101c)) wiki_math101c = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101c))
wiki_math101c_page = self.create_urlpath(wiki_math101c, 'Child') wiki_math101c_page = self.create_urlpath(wiki_math101c, 'Child')
......
lms/djangoapps/courseware/access.py
...@@ -20,7 +20,7 @@ from external_auth.models import ExternalAuthMap ...@@ -20,7 +20,7 @@ from external_auth.models import ExternalAuthMap
from courseware.masquerade import is_masquerading_as_student from courseware.masquerade import is_masquerading_as_student
from django.utils.timezone import UTC from django.utils.timezone import UTC
from student.models import CourseEnrollment from student.models import CourseEnrollment
from courseware.roles import ( from student.roles import (
GlobalStaff, CourseStaffRole, CourseInstructorRole, GlobalStaff, CourseStaffRole, CourseInstructorRole,
OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole
) )
......
lms/djangoapps/courseware/tests/factories.py
...@@ -14,7 +14,7 @@ from student.tests.factories import RegistrationFactory # Imported to re-export ...@@ -14,7 +14,7 @@ from student.tests.factories import RegistrationFactory # Imported to re-export
from student.tests.factories import UserProfileFactory as StudentUserProfileFactory from student.tests.factories import UserProfileFactory as StudentUserProfileFactory
from courseware.models import StudentModule, XModuleUserStateSummaryField from courseware.models import StudentModule, XModuleUserStateSummaryField
from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField
from courseware.roles import ( from student.roles import (
CourseInstructorRole, CourseInstructorRole,
CourseStaffRole, CourseStaffRole,
CourseBetaTesterRole, CourseBetaTesterRole,
......
lms/djangoapps/courseware/tests/test_masquerade.py
...@@ -16,7 +16,7 @@ from django.contrib.auth.models import User ...@@ -16,7 +16,7 @@ from django.contrib.auth.models import User
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.roles import CourseStaffRole from student.roles import CourseStaffRole
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores
from lms.lib.xblock.runtime import quote_slashes from lms.lib.xblock.runtime import quote_slashes
......
lms/djangoapps/courseware/tests/test_view_authentication.py
...@@ -3,7 +3,6 @@ import pytz ...@@ -3,7 +3,6 @@ import pytz
from mock import patch from mock import patch
from django.contrib.auth.models import User
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.test.utils import override_settings from django.test.utils import override_settings
...@@ -128,6 +127,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -128,6 +127,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
display_name='Welcome' display_name='Welcome'
) )
self.global_staff_user = GlobalStaffFactory()
self.unenrolled_user = UserFactory(last_name="Unenrolled") self.unenrolled_user = UserFactory(last_name="Unenrolled")
self.enrolled_user = UserFactory(last_name="Enrolled") self.enrolled_user = UserFactory(last_name="Enrolled")
...@@ -135,10 +135,11 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -135,10 +135,11 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
CourseEnrollmentFactory(user=self.enrolled_user, course_id=self.test_course.id) CourseEnrollmentFactory(user=self.enrolled_user, course_id=self.test_course.id)
self.staff_user = StaffFactory(course=self.course.location) self.staff_user = StaffFactory(course=self.course.location)
self.instructor_user = InstructorFactory(course=self.course.location) self.instructor_user = InstructorFactory(
course=self.course.location)
self.org_staff_user = OrgStaffFactory(course=self.course.location) self.org_staff_user = OrgStaffFactory(course=self.course.location)
self.org_instructor_user = OrgInstructorFactory(course=self.course.location) self.org_instructor_user = OrgInstructorFactory(
self.global_staff_user = GlobalStaffFactory() course=self.course.location)
def test_redirection_unenrolled(self): def test_redirection_unenrolled(self):
""" """
......
lms/djangoapps/dashboard/sysadmin.py
...@@ -29,9 +29,9 @@ from edxmako.shortcuts import render_to_response ...@@ -29,9 +29,9 @@ from edxmako.shortcuts import render_to_response
import mongoengine import mongoengine
from courseware.courses import get_course_by_id from courseware.courses import get_course_by_id
from courseware.roles import CourseStaffRole, CourseInstructorRole
import dashboard.git_import as git_import import dashboard.git_import as git_import
from dashboard.git_import import GitImportError from dashboard.git_import import GitImportError
from student.roles import CourseStaffRole, CourseInstructorRole
from dashboard.models import CourseImportLog from dashboard.models import CourseImportLog
from external_auth.models import ExternalAuthMap from external_auth.models import ExternalAuthMap
from external_auth.views import generate_password from external_auth.views import generate_password
......
lms/djangoapps/dashboard/tests/test_sysadmin.py
...@@ -17,7 +17,7 @@ from django.utils.html import escape ...@@ -17,7 +17,7 @@ from django.utils.html import escape
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
import mongoengine import mongoengine
from courseware.roles import CourseStaffRole, GlobalStaff from student.roles import CourseStaffRole, GlobalStaff
from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
from dashboard.models import CourseImportLog from dashboard.models import CourseImportLog
from dashboard.sysadmin import Users from dashboard.sysadmin import Users
......
lms/djangoapps/instructor/access.py
...@@ -12,7 +12,7 @@ TO DO sync instructor and staff flags ...@@ -12,7 +12,7 @@ TO DO sync instructor and staff flags
import logging import logging
from django_comment_common.models import Role from django_comment_common.models import Role
from courseware.roles import CourseBetaTesterRole, CourseInstructorRole, CourseStaffRole from student.roles import CourseBetaTesterRole, CourseInstructorRole, CourseStaffRole
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
......
lms/djangoapps/instructor/tests/test_access.py
...@@ -9,7 +9,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase ...@@ -9,7 +9,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from django.test.utils import override_settings from django.test.utils import override_settings
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.roles import CourseBetaTesterRole, CourseStaffRole from student.roles import CourseBetaTesterRole, CourseStaffRole
from django_comment_common.models import (Role, from django_comment_common.models import (Role,
FORUM_ROLE_MODERATOR) FORUM_ROLE_MODERATOR)
......
lms/djangoapps/instructor/tests/test_legacy_anon_csv.py
...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse ...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.roles import CourseStaffRole from student.roles import CourseStaffRole
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores
...@@ -44,11 +44,7 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas ...@@ -44,11 +44,7 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas
self.activate_user(self.student) self.activate_user(self.student)
self.activate_user(self.instructor) self.activate_user(self.instructor)
def make_instructor(course): CourseStaffRole(self.toy.location).add_users(User.objects.get(email=self.instructor))
""" Create an instructor for the course."""
CourseStaffRole(course.location).add_users(User.objects.get(email=self.instructor))
make_instructor(self.toy)
self.logout() self.logout()
self.login(self.instructor, self.password) self.login(self.instructor, self.password)
......
lms/djangoapps/instructor/tests/test_legacy_download_csv.py
...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse ...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.roles import CourseStaffRole from student.roles import CourseStaffRole
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores
...@@ -41,11 +41,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme ...@@ -41,11 +41,7 @@ class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollme
self.activate_user(self.student) self.activate_user(self.student)
self.activate_user(self.instructor) self.activate_user(self.instructor)
def make_instructor(course): CourseStaffRole(self.toy.location).add_users(User.objects.get(email=self.instructor))
""" Create an instructor for the course. """
CourseStaffRole(course.location).add_users(User.objects.get(email=self.instructor))
make_instructor(self.toy)
self.logout() self.logout()
self.login(self.instructor, self.password) self.login(self.instructor, self.password)
......
lms/djangoapps/instructor/tests/test_legacy_forum_admin.py
...@@ -15,7 +15,7 @@ from django_comment_client.utils import has_forum_access ...@@ -15,7 +15,7 @@ from django_comment_client.utils import has_forum_access
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.roles import CourseStaffRole from student.roles import CourseStaffRole
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
......
lms/djangoapps/instructor/views/legacy.py
...@@ -34,7 +34,7 @@ from bulk_email.models import CourseEmail, CourseAuthorization ...@@ -34,7 +34,7 @@ from bulk_email.models import CourseEmail, CourseAuthorization
from courseware import grades from courseware import grades
from courseware.access import has_access from courseware.access import has_access
from courseware.courses import get_course_with_access, get_cms_course_link from courseware.courses import get_course_with_access, get_cms_course_link
from courseware.roles import ( from student.roles import (
CourseStaffRole, CourseInstructorRole, CourseBetaTesterRole, GlobalStaff CourseStaffRole, CourseInstructorRole, CourseBetaTesterRole, GlobalStaff
) )
from courseware.models import StudentModule from courseware.models import StudentModule
...@@ -1029,7 +1029,7 @@ def _role_members_table(role, title, course_id): ...@@ -1029,7 +1029,7 @@ def _role_members_table(role, title, course_id):
Return a data table of usernames and names of users in group_name. Return a data table of usernames and names of users in group_name.
Arguments: Arguments:
role -- a courseware.roles.AccessRole role -- a student.roles.AccessRole
title -- a descriptive title to show the user title -- a descriptive title to show the user
Returns: Returns:
...@@ -1107,7 +1107,7 @@ def remove_user_from_role(request, username_or_email, role, group_title, event_n ...@@ -1107,7 +1107,7 @@ def remove_user_from_role(request, username_or_email, role, group_title, event_n
Arguments: Arguments:
request: django request--used for tracking log request: django request--used for tracking log
username_or_email: who to remove. Decide if it's an email by presense of an '@' username_or_email: who to remove. Decide if it's an email by presense of an '@'
role: A courseware.roles.AccessRole role: A student.roles.AccessRole
group_title: what to call this group in messages to user--e.g. "beta-testers". group_title: what to call this group in messages to user--e.g. "beta-testers".
event_name: what to call this event when logging to tracking logs. event_name: what to call this event when logging to tracking logs.
......
lms/djangoapps/open_ended_grading/tests.py
...@@ -26,7 +26,7 @@ from courseware.tests import factories ...@@ -26,7 +26,7 @@ from courseware.tests import factories
from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code, check_for_post_code from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code, check_for_post_code
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from lms.lib.xblock.runtime import LmsModuleSystem from lms.lib.xblock.runtime import LmsModuleSystem
from courseware.roles import CourseStaffRole from student.roles import CourseStaffRole
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from student.models import unique_id_for_user from student.models import unique_id_for_user
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment