diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 056b514180..7ef8e5a945 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -38,13 +38,16 @@ from importlib import import_module from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore import Location -from xmodule.modulestore.django import modulestore +from opaque_keys import InvalidKeyError import lms.lib.comment_client as cc from util.query import use_read_replica_if_available from xmodule_django.models import CourseKeyField, NoneToEmptyManager +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.django import modulestore from opaque_keys.edx.keys import CourseKey from functools import total_ordering +from courseware.access import has_access from certificates.models import GeneratedCertificate from course_modes.models import CourseMode @@ -818,31 +821,23 @@ class CourseEnrollment(models.Model): # All the server-side checks for whether a user is allowed to enroll. try: - course_id = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get("course_id")) - except InvalidKeyError: - log.warning( - "User {username} tried to {action} with invalid course id: {course_id}".format( - username=user.username, - action=action, - course_id=request.POST.get("course_id") - ) - ) - raise CourseEnrollmentException - try: - course = modulestore().get_course(course_id) + course = modulestore().get_course(course_key) except ItemNotFoundError: log.warning( "User {0} failed to enroll in non-existent course {1}".format( user.username, - course_id + course_key.to_deprecated_string() ) ) raise NonExistentCourseError + if course is None: + raise NonExistentCourseError + if not has_access(user, 'enroll', course): log.warning( "User {0} failed to enroll in course {1} because enrollment is closed".format( user.username, - course_id + course_key.to_deprecated_string() ) ) raise EnrollmentClosedError @@ -850,15 +845,15 @@ class CourseEnrollment(models.Model): log.warning( "User {0} failed to enroll in full course {1}".format( user.username, - course_id + course_key.to_deprecated_string() ) ) raise CourseFullError - if CourseEnrollment.is_enrolled(user, course_id): + if CourseEnrollment.is_enrolled(user, course_key): log.warning( "User {0} attempted to enroll in {1}, but they were already enrolled".format( user.username, - course_id + course_key.to_deprecated_string() ) ) raise AlreadyEnrolledError diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 6061ce1ea2..6594248595 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -6,7 +6,7 @@ adding users, removing users, and listing members from abc import ABCMeta, abstractmethod from django.contrib.auth.models import User -from student.models import CourseAccessRole + from xmodule_django.models import CourseKeyField @@ -15,6 +15,7 @@ class RoleCache(object): A cache of the CourseAccessRoles held by a particular user """ def __init__(self, user): + from student.models import CourseAccessRole self._roles = set( CourseAccessRole.objects.filter(user=user).all() ) @@ -127,6 +128,7 @@ class RoleBase(AccessRole): """ # silently ignores anonymous and inactive users so that any that are # legit get updated. + from student.models import CourseAccessRole for user in users: if user.is_authenticated and user.is_active and not self.has_user(user): entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org) @@ -138,6 +140,7 @@ class RoleBase(AccessRole): """ Remove the supplied django users from this role. """ + from student.models import CourseAccessRole entries = CourseAccessRole.objects.filter( user__in=users, role=self._role_name, org=self.org, course_id=self.course_key ) @@ -174,6 +177,7 @@ class CourseRole(RoleBase): @classmethod def course_group_already_exists(self, course_key): + from student.models import CourseAccessRole return CourseAccessRole.objects.filter(org=course_key.org, course_id=course_key).exists() @@ -267,6 +271,7 @@ class UserBasedRole(object): """ Grant this object's user the object's role for the supplied courses """ + from student.models import CourseAccessRole if self.user.is_authenticated and self.user.is_active: for course_key in course_keys: entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org) @@ -280,6 +285,7 @@ class UserBasedRole(object): """ Remove the supplied courses from this user's configured role. """ + from student.models import CourseAccessRole entries = CourseAccessRole.objects.filter(user=self.user, role=self.role, course_id__in=course_keys) entries.delete() if hasattr(self.user, '_roles'): @@ -294,4 +300,5 @@ class UserBasedRole(object): * course_id * role (will be self.role--thus uninteresting) """ + from student.models import CourseAccessRole return CourseAccessRole.objects.filter(role=self.role, user=self.user) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index c43ae54f6f..dc2cbfb69e 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -669,6 +669,18 @@ def change_enrollment(request, auto_register=False): if 'course_id' not in request.POST: return HttpResponseBadRequest(_("Course id not specified")) + try: + course_id = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get("course_id")) + except InvalidKeyError: + log.warning( + "User {username} tried to {action} with invalid course id: {course_id}".format( + username=user.username, + action=action, + course_id=request.POST.get("course_id") + ) + ) + return HttpResponseBadRequest(_("Invalid course id")) + # Ensure the user is authenticated if not user.is_authenticated(): return HttpResponseForbidden() @@ -721,7 +733,7 @@ def change_enrollment(request, auto_register=False): # for no such model to exist, even though we've set the enrollment type # to "honor". try: - CourseEnrollment.enroll(user, course.id, mode=current_mode.slug) + CourseEnrollment.enroll(user, course_id, mode=current_mode.slug) except Exception: return HttpResponseBadRequest(_("Could not enroll")) @@ -757,7 +769,7 @@ def change_enrollment(request, auto_register=False): ) try: - CourseEnrollment.enroll(user, course.id, mode=current_mode.slug) + CourseEnrollment.enroll(user, course_id, mode=current_mode.slug) except Exception: return HttpResponseBadRequest(_("Could not enroll")) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index dc9d021e5e..bbb481d560 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -399,3 +399,10 @@ def login_analytics(*args, **kwargs): } } ) + +@partial.partial +def change_enrollment(*args, **kwargs): + try: + CourseEnrollment.enroll(user, 'foo') + except: + pass diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index 07e46a3bed..fdd93e92cb 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/common/djangoapps/third_party_auth/settings.py @@ -110,6 +110,7 @@ def _set_global_settings(django_settings): 'social.pipeline.social_auth.load_extra_data', 'social.pipeline.user.user_details', 'third_party_auth.pipeline.login_analytics', + 'third_party_auth.pipeline.change_enrollment', ) # We let the user specify their email address during signup. diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 9c900185ab..428a54d925 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -14,11 +14,9 @@ from xmodule.x_module import XModule from xblock.core import XBlock -from student.models import CourseEnrollmentAllowed from external_auth.models import ExternalAuthMap from courseware.masquerade import is_masquerading_as_student from django.utils.timezone import UTC -from student.models import CourseEnrollment from student.roles import ( GlobalStaff, CourseStaffRole, CourseInstructorRole, OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole @@ -125,6 +123,7 @@ def _has_access_course_desc(user, action, course): """ Can this user access the forums in this course? """ + from student.models import CourseEnrollment return ( can_load() and ( @@ -149,6 +148,7 @@ def _has_access_course_desc(user, action, course): """ # if using registration method to restrict (say shibboleth) + from student.models import CourseEnrollmentAllowed if settings.FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain: if user is not None and user.is_authenticated() and \ ExternalAuthMap.objects.filter(user=user, external_domain=course.enrollment_domain):