fix unicode comparisons, fix 404 access exception, fix initial link handling, fix forum access modification permissions

lms/djangoapps/instructor/access.py

@@ -9,11 +9,14 @@ TODO sync instructor and staff flags
         {instructor: true, staff: true}
 """
 
+import logging
 from django.contrib.auth.models import Group
 from courseware.access import (get_access_group_name,
                                course_beta_test_group_name)
 from django_comment_common.models import Role
 
+log = logging.getLogger(__name__)
+
 
 def list_with_level(course, level):
     """
@@ -23,7 +26,7 @@ def list_with_level(course, level):
     There could be other levels specific to the course.
     If there is no Group for that course-level, returns an empty list
     """
-    if level is 'beta':
+    if level == 'beta':
         grpname = course_beta_test_group_name(course.location)
     else:
         grpname = get_access_group_name(course, level)
@@ -31,6 +34,7 @@ def list_with_level(course, level):
     try:
         return Group.objects.get(name=grpname).user_set.all()
     except Group.DoesNotExist:
+        log.info("list_with_level called with non-existant group named {}".format(grpname))
         return []
 
 
@@ -59,10 +63,10 @@ def _change_access(course, user, level, action):
     level is one of ['instructor', 'staff', 'beta']
     action is one of ['allow', 'revoke']
 
-    NOTE: will NOT create a group that does not yet exist.
+    NOTE: will create a group if it does not yet exist.
     """
 
-    if level is 'beta':
+    if level == 'beta':
         grpname = course_beta_test_group_name(course.location)
     elif level in ['instructor', 'staff']:
         grpname = get_access_group_name(course, level)

lms/djangoapps/instructor/views/api.py

@@ -17,6 +17,7 @@ from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbid
 from courseware.access import has_access
 from courseware.courses import get_course_with_access, get_course_by_id
 from django.contrib.auth.models import User
+from django_comment_client.utils import has_forum_access
 from django_comment_common.models import (Role,
                                           FORUM_ROLE_ADMINISTRATOR,
                                           FORUM_ROLE_MODERATOR,
@@ -33,6 +34,7 @@ import analytics.csvs
 
 log = logging.getLogger(__name__)
 
+
 def common_exceptions_400(func):
     """
     Catches common exceptions and renders matching 400 errors.
@@ -630,12 +632,33 @@ def list_forum_members(request, course_id):
     Lists forum members of a certain rolename.
     Limited to staff access.
 
-    Takes query parameter `rolename`
+    The requesting user must be at least staff.
+    Staff forum admins can access all roles EXCEPT for FORUM_ROLE_ADMINISTRATOR
+        which is limited to instructors.
+
+    Takes query parameter `rolename`.
     """
+    course = get_course_by_id(course_id)
+    has_instructor_access = has_access(request.user, course, 'instructor')
+    has_forum_admin = has_forum_access(
+        request.user, course_id, FORUM_ROLE_ADMINISTRATOR
+    )
+
     rolename = request.GET.get('rolename')
 
+    # default roles require either (staff & forum admin) or (instructor)
+    if not (has_forum_admin or has_instructor_access):
+        return HttpResponseBadRequest(
+            "Operation requires staff & forum admin or instructor access"
+        )
+
+    # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor)
+    if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access:
+        return HttpResponseBadRequest("Operation requires instructor access.")
+
+    # filter out unsupported for roles
     if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]:
-        return HttpResponseBadRequest()
+        return HttpResponseBadRequest("Unrecognized rolename '{}'.".format(rolename))
 
     try:
         role = Role.objects.get(name=rolename, course_id=course_id)
@@ -664,7 +687,7 @@ def list_forum_members(request, course_id):
 
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@require_level('instructor')
+@require_level('staff')
 @require_query_params(
     email="the target users email",
     rolename="the forum role",
@@ -675,20 +698,46 @@ def update_forum_role_membership(request, course_id):
     """
     Modify user's forum role.
 
+    The requesting user must be at least staff.
+    Staff forum admins can access all roles EXCEPT for FORUM_ROLE_ADMINISTRATOR
+        which is limited to instructors.
+    No one can revoke an instructors FORUM_ROLE_ADMINISTRATOR status.
+
     Query parameters:
-    email is the target users email
-    rolename is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]
-    action is one of ['allow', 'revoke']
+    - `email` is the target users email
+    - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]
+    - `action` is one of ['allow', 'revoke']
     """
+    course = get_course_by_id(course_id)
+    has_instructor_access = has_access(request.user, course, 'instructor')
+    has_forum_admin = has_forum_access(
+        request.user, course_id, FORUM_ROLE_ADMINISTRATOR
+    )
+
     email = request.GET.get('email')
     rolename = request.GET.get('rolename')
     action = request.GET.get('action')
 
+    # default roles require either (staff & forum admin) or (instructor)
+    if not (has_forum_admin or has_instructor_access):
+        return HttpResponseBadRequest(
+            "Operation requires staff & forum admin or instructor access"
+        )
+
+    # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor)
+    if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access:
+        return HttpResponseBadRequest("Operation requires instructor access.")
+
     if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]:
-        return HttpResponseBadRequest()
+        return HttpResponseBadRequest("Unrecognized rolename '{}'.".format(rolename))
+
+    user = User.objects.get(email=email)
+    target_is_instructor = has_access(user, course, 'instructor')
+    # cannot revoke instructor
+    if target_is_instructor and action == 'revoke' and rolename == FORUM_ROLE_ADMINISTRATOR:
+        return HttpResponseBadRequest("Cannot revoke instructor forum admin privelages.")
 
     try:
-        user = User.objects.get(email=email)
         access.update_forum_role_membership(course_id, user, rolename, action)
     except Role.DoesNotExist:
         return HttpResponseBadRequest("Role does not exist.")

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -40,7 +40,7 @@ def instructor_dashboard_2(request, course_id):
     }
 
     if not access['staff']:
-        raise Http404
+        raise Http404()
 
     sections = [
         _section_course_info(course_id),

lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee

@@ -74,18 +74,24 @@ setup_instructor_dashboard = (idash_content) =>
       # plantTimeout 0, -> section.data('wrapper')?.onExit?()
 
 
-  # activate an initial section by programmatically clicking on it.
+  # activate an initial section by 'clicking' on it.
   # check for a deep-link, or click the first link.
+  click_first_link = ->
+    link = links.eq(0)
+    link.click()
+    link.data('wrapper')?.onClickTitle?()
+
   if (new RegExp "^#{HASH_LINK_PREFIX}").test location.hash
     rmatch = (new RegExp "^#{HASH_LINK_PREFIX}(.*)").exec location.hash
     section_name = rmatch[1]
     link = links.filter "[data-section='#{section_name}']"
-    link.click()
-    link.data('wrapper')?.onClickTitle?()
+    if link.length == 1
+      link.click()
+      link.data('wrapper')?.onClickTitle?()
+    else
+      click_first_link()
   else
-    link = links.eq(0)
-    link.click()
-    link.data('wrapper')?.onClickTitle?()
+    click_first_link()
 
 
 

lms/templates/courseware/instructor_dashboard_2/membership.html

@@ -57,16 +57,18 @@
     </div>
   %endif
 
-  %if section_data['access']['forum_admin']:
-    <div class="auth-list-container" data-rolename="Administrator" data-display-name="Forum Admins">
-      <div class="auth-list-table" data-endpoint="${ section_data['list_forum_members_url'] }"></div>
-      <div class="auth-list-add" data-endpoint="${ section_data['update_forum_role_membership_url'] }">
-        <input type="text" name="email" placeholder="Enter Email" spellcheck="false">
-        <input type="button" name="allow" value="Grant Forum Admin">
+  %if section_data['access']['instructor']:
+      <div class="auth-list-container" data-rolename="Administrator" data-display-name="Forum Admins">
+        <div class="auth-list-table" data-endpoint="${ section_data['list_forum_members_url'] }"></div>
+        <div class="auth-list-add" data-endpoint="${ section_data['update_forum_role_membership_url'] }">
+          <input type="text" name="email" placeholder="Enter Email" spellcheck="false">
+          <input type="button" name="allow" value="Grant Forum Admin">
+        </div>
+        <div class="request-response-error"></div>
       </div>
-      <div class="request-response-error"></div>
-    </div>
+  %endif
 
+  %if section_data['access']['instructor'] or section_data['access']['forum_admin']:
     <div class="auth-list-container" data-rolename="Moderator" data-display-name="Forum Moderators">
       <div class="auth-list-table" data-endpoint="${ section_data['list_forum_members_url'] }"></div>
       <div class="auth-list-add" data-endpoint="${ section_data['update_forum_role_membership_url'] }">