Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow only enrolled users in course forum roles #544

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2212,11 +2212,16 @@ def setUp(self):
super().setUp()

self.instructor = InstructorFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.instructor, self.course.id)
self.client.login(username=self.instructor.username, password='test')

self.other_instructor = InstructorFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.other_instructor, self.course.id)
self.other_staff = StaffFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.other_staff, self.course.id)
self.other_user = UserFactory()
CourseEnrollment.enroll(self.other_user, self.course.id)
self.unenrolled_user = UserFactory()

def test_modify_access_noparams(self):
""" Test missing all query parameters. """
Expand Down Expand Up @@ -2432,6 +2437,34 @@ def assert_update_forum_role_membership(self, current_user, identifier, rolename
elif action == 'revoke':
assert rolename not in user_roles

def test_update_forum_role_with_unenrolled_user(self):
"""
Check response of update_forum_role_membership api with unenrolled user.
"""

def _check_allow_access_with_unenrolled_user(url, role):
"""
Helper function for testing api.
"""
response = self.client.post(url, {
'unique_student_identifier': self.unenrolled_user.username,
'rolename': role,
'action': 'allow',
})
assert response.status_code == 200
expected = {
'unique_student_identifier': self.unenrolled_user.username,
'userNotEnrolled': True,
}
res_json = json.loads(response.content.decode('utf-8'))
assert res_json == expected
user_roles = self.unenrolled_user.roles.filter(course_id=self.course.id).values_list("name", flat=True)
assert role not in user_roles

forum_role_url = reverse('update_forum_role_membership', kwargs={'course_id': str(self.course.id)})
_check_allow_access_with_unenrolled_user(forum_role_url, 'Community TA')
_check_allow_access_with_unenrolled_user(forum_role_url, 'Administrator')


@ddt.ddt
class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2807,6 +2807,12 @@ def update_forum_role_membership(request, course_id):
))

user = get_student_from_identifier(unique_student_identifier)
if action == 'allow' and not is_user_enrolled_in_course(user, course_id):
response_payload = {
'unique_student_identifier': unique_student_identifier,
'userNotEnrolled': True,
}
return JsonResponse(response_payload)

try:
update_forum_role(course_id, user, rolename, action)
Expand Down
5 changes: 5 additions & 0 deletions lms/static/js/instructor_dashboard/membership.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@ such that the value can be defined later than this assignment (file load order).
return this.show_errors(
gettext('Error: You cannot remove yourself from the Instructor group!')
);
} else if (data.userNotEnrolled) {
msg = gettext("Error: User '<%- username %>' is not enrolled in this course.")
return this.show_errors(_.template(msg)({
username: data.unique_student_identifier
}));
} else {
return this.reload_list();
}
Expand Down