From 5207e2fc9169a9533b41d89d9caeea3651394196 Mon Sep 17 00:00:00 2001 From: noraiz-anwar Date: Wed, 30 Nov 2016 15:42:59 +0500 Subject: [PATCH] Remove access for inactive users --- cms/djangoapps/contentstore/views/user.py | 16 ++++---- common/djangoapps/student/roles.py | 14 +++++-- common/djangoapps/student/views.py | 11 ++++-- .../test/acceptance/pages/studio/auto_auth.py | 12 +++++- .../tests/studio/test_studio_course_team.py | 39 +++++++++++++++++++ 5 files changed, 76 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index e5da0a8194..e74a7bac8f 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -147,13 +147,6 @@ def _course_team_user(request, course_key, email): if not ((requester_perms & STUDIO_EDIT_ROLES) or (user.id == request.user.id)): return permissions_error_response - # can't modify an inactive user - if not user.is_active: - msg = { - "error": _('User {email} has registered but has not yet activated his/her account.').format(email=email), - } - return JsonResponse(msg, 400) - if request.method == "DELETE": new_role = None else: @@ -165,6 +158,13 @@ def _course_team_user(request, course_key, email): else: return JsonResponse({"error": _("No `role` specified.")}, 400) + # can't modify an inactive user but can remove it + if not (user.is_active or new_role is None): + msg = { + "error": _('User {email} has registered but has not yet activated his/her account.').format(email=email), + } + return JsonResponse(msg, 400) + old_roles = set() role_added = False for role_type in role_hierarchy: @@ -177,7 +177,7 @@ def _course_team_user(request, course_key, email): role_added = True else: return permissions_error_response - elif role.has_user(user): + elif role.has_user(user, check_user_activation=False): # Remove the user from this old role: old_roles.add(role) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 8606e95063..dd89ccc186 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -130,11 +130,19 @@ class RoleBase(AccessRole): self.course_key = course_key self._role_name = role_name - def has_user(self, user): + # pylint: disable=arguments-differ + def has_user(self, user, check_user_activation=True): """ - Return whether the supplied django user has access to this role. + Check if the supplied django user has access to this role. + + Arguments: + user: user to check against access to role + check_user_activation: Indicating whether or not we need to check + user activation while checking user roles + Return: + bool identifying if user has that particular role or not """ - if not (user.is_authenticated() and user.is_active): + if check_user_activation and not (user.is_authenticated() and user.is_active): return False # pylint: disable=protected-access diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 3e231367e1..cd51c4a5c7 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -2001,6 +2001,7 @@ def auto_auth(request): * `redirect`: Set to "true" will redirect to the `redirect_to` value if set, or course home page if course_id is defined, otherwise it will redirect to dashboard * `redirect_to`: will redirect to to this url + * `is_active` : make/update account with status provided as 'is_active' If username, email, or password are not provided, use randomly generated credentials. """ @@ -2017,10 +2018,13 @@ def auto_auth(request): is_superuser = request.GET.get('superuser', None) course_id = request.GET.get('course_id', None) redirect_to = request.GET.get('redirect_to', None) + active_status = request.GET.get('is_active') # mode has to be one of 'honor'/'professional'/'verified'/'audit'/'no-id-professional'/'credit' enrollment_mode = request.GET.get('enrollment_mode', 'honor') + active_status = (not active_status or active_status == 'true') + course_key = None if course_id: course_key = CourseLocator.from_string(course_id) @@ -2048,6 +2052,7 @@ def auto_auth(request): user = User.objects.get(username=username) user.email = email user.set_password(password) + user.is_active = active_status user.save() profile = UserProfile.objects.get(user=user) reg = Registration.objects.get(user=user) @@ -2061,9 +2066,9 @@ def auto_auth(request): user.is_superuser = (is_superuser == "true") user.save() - # Activate the user - reg.activate() - reg.save() + if active_status: + reg.activate() + reg.save() # ensure parental consent threshold is met year = datetime.date.today().year diff --git a/common/test/acceptance/pages/studio/auto_auth.py b/common/test/acceptance/pages/studio/auto_auth.py index ef0347c6e2..cbc9f275e6 100644 --- a/common/test/acceptance/pages/studio/auto_auth.py +++ b/common/test/acceptance/pages/studio/auto_auth.py @@ -12,19 +12,24 @@ class AutoAuthPage(PageObject): """ The automatic authorization page. When allowed via the django settings file, visiting - this url will create a user and log them in. + this url will create/update a user and log them in. """ def __init__(self, browser, username=None, email=None, password=None, - staff=None, course_id=None, roles=None, no_login=None): + staff=None, course_id=None, roles=None, no_login=None, is_active=None): """ Auto-auth is an end-point for HTTP GET requests. By default, it will create accounts with random user credentials, but you can also specify credentials using querystring parameters. + Can be used to update an account, call to this end-point with already + made account's credentials along with values to update will result into + an account update. + `username`, `email`, and `password` are the user's credentials (strings) `staff` is a boolean indicating whether the user is global staff. `course_id` is the ID of the course to enroll the student in. + `is_active` activation status of user Currently, this has the form "org/number/run" Note that "global staff" is NOT the same as course staff. @@ -55,6 +60,9 @@ class AutoAuthPage(PageObject): if no_login: self._params['no_login'] = True + if is_active is not None: + self._params['is_active'] = 'true' if is_active else 'false' + @property def url(self): """ diff --git a/common/test/acceptance/tests/studio/test_studio_course_team.py b/common/test/acceptance/tests/studio/test_studio_course_team.py index 8079747124..dbe9592b4e 100644 --- a/common/test/acceptance/tests/studio/test_studio_course_team.py +++ b/common/test/acceptance/tests/studio/test_studio_course_team.py @@ -26,6 +26,17 @@ class CourseTeamPageTest(StudioCourseTest): ).visit() return user + def _update_user(self, user_info): + """ + Update user with provided `user_info` + + Arguments: + `user_info`: dictionary containing values of attributes to be updated + """ + AutoAuthPage( + self.browser, no_login=True, **user_info + ).visit() + def setUp(self, is_staff=False): """ Install a course with no content using a fixture. @@ -174,6 +185,34 @@ class CourseTeamPageTest(StudioCourseTest): self.log_in(self.other_user) self._assert_current_course(visible=False) + def test_admins_can_delete_other_inactive_users(self): + """ + Scenario: Admins can delete other inactive users + Given I have opened a new course in Studio + And I am viewing the course team settings. + When I add other user to the course team, + And then delete that other user from the course team. + And other user logs in + Then he/she does not see the course on page + """ + self.page.add_user_to_course(self.other_user.get('email')) + self._assert_user_present(self.other_user, present=True) + + # inactivate user + user_info = { + 'username': self.other_user.get('username'), + 'email': self.other_user.get('email'), + 'password': self.other_user.get('password'), + 'is_active': False + } + self._update_user(user_info) + + # go to course team page to perform delete operation + self._go_to_course_team_page() + self.page.delete_user_from_course(self.other_user.get('email')) + + self._assert_user_present(self.other_user, present=False) + def test_admins_cannot_add_users_that_do_not_exist(self): """ Scenario: Admins cannot add users that do not exist