Did a quick security scan through views.

* we appear to be fairly consistent in using is_staff and has_staff_access_to_course
* cleaned up some docstrings and little code things as I went
* fixed small bug in change_enrollment view (check for non-anon user)

common/djangoapps/student/views.py

@@ -94,8 +94,9 @@ def main_index(extra_context = {}, user=None):
     context.update(extra_context)
     return render_to_response('index.html', context)
 
-def course_from_id(id):
-    course_loc = CourseDescriptor.id_to_location(id)
+def course_from_id(course_id):
+    """Return the CourseDescriptor corresponding to this course_id"""
+    course_loc = CourseDescriptor.id_to_location(course_id)
     return modulestore().get_item(course_loc)
 
 
@@ -158,15 +159,19 @@ def try_change_enrollment(request):
 
 @login_required
 def change_enrollment_view(request):
+    """Delegate to change_enrollment to actually do the work."""
     return HttpResponse(json.dumps(change_enrollment(request)))
 
-
 def change_enrollment(request):
     if request.method != "POST":
         raise Http404
 
-    action = request.POST.get("enrollment_action", "")
     user = request.user
+    if not user.is_authenticated():
+        raise Http404
+
+    action = request.POST.get("enrollment_action", "")
+
     course_id = request.POST.get("course_id", None)
     if course_id == None:
         return HttpResponse(json.dumps({'success': False, 'error': 'There was an error receiving the course id.'}))
@@ -184,7 +189,7 @@ def change_enrollment(request):
         if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'):
             # require that user be in the staff_* group (or be an overall admin) to be able to enroll
             # eg staff_6.002x or staff_6.00x
-            if not has_staff_access_to_course(user,course):
+            if not has_staff_access_to_course(user, course):
                 staff_group = course_staff_group_name(course)
                 log.debug('user %s denied enrollment to %s ; not in %s' % (user,course.location.url(),staff_group))
                 return {'success': False, 'error' : '%s membership required to access course.' % staff_group}
@@ -264,6 +269,7 @@ def logout_user(request):
 def change_setting(request):
     ''' JSON call to change a profile setting: Right now, location
     '''
+    # TODO (vshnayder): location is no longer used
     up = UserProfile.objects.get(user=request.user)  # request.user.profile_cache
     if 'location' in request.POST:
         up.location = request.POST['location']

lms/djangoapps/courseware/views.py

@@ -69,12 +69,12 @@ def gradebook(request, course_id):
     if 'course_admin' not in user_groups(request.user):
         raise Http404
     course = check_course(course_id)
-        
+
     student_objects = User.objects.all()[:100]
     student_info = []
-    
+
     #TODO: Only select students who are in the course
-    for student in student_objects:        
+    for student in student_objects:
         student_info.append({
             'username': student.username,
             'id': student.id,
@@ -104,10 +104,10 @@ def profile(request, course_id, student_id=None):
 
     student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course)
     course_module = get_module(request.user, request, course.location, student_module_cache)
-    
+
     courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache)
     grade_summary = grades.grade(request.user, request, course, student_module_cache)
-    
+
     context = {'name': user_info.name,
                'username': student.username,
                'location': user_info.location,
@@ -233,12 +233,10 @@ def jump_to(request, location):
     '''
     Show the page that contains a specific location.
 
-    If the location is invalid, return a 404.
+    If the location is invalid or not in any class, return a 404.
 
-    If the location is valid, but not present in a course, ?
-
-    If the location is valid, but in a course the current user isn't registered for, ?
-        TODO -- let the index view deal with it?
+    Otherwise, delegates to the index view to figure out whether this user
+    has access, and what they should see.
     '''
     # Complain if the location isn't valid
     try:
@@ -254,16 +252,16 @@ def jump_to(request, location):
     except NoPathToItem:
         raise Http404("This location is not in any class: {0}".format(location))
 
-    # Rely on index to do all error handling
+    # Rely on index to do all error handling and access control.
     return index(request, course_id, chapter, section, position)
 
 @ensure_csrf_cookie
 def course_info(request, course_id):
-    '''
+    """
     Display the course's info.html, or 404 if there is no such course.
 
     Assumes the course_id is in a valid format.
-    '''
+    """
     course = check_course(course_id)
 
     return render_to_response('info.html', {'course': course})
@@ -289,7 +287,10 @@ def course_about(request, course_id):
 @ensure_csrf_cookie
 @cache_if_anonymous
 def university_profile(request, org_id):
-    all_courses = sorted(modulestore().get_courses(), key=lambda course: course.number)
+    """
+    Return the profile for the particular org_id.  404 if it's not valid.
+    """
+    all_courses = modulestore().get_courses()
     valid_org_ids = set(c.org for c in all_courses)
     if org_id not in valid_org_ids:
         raise Http404("University Profile not found for {0}".format(org_id))