diff --git a/backend/Pipfile b/backend/Pipfile index 73e310ce..e1cecd9d 100644 --- a/backend/Pipfile +++ b/backend/Pipfile @@ -7,7 +7,7 @@ name = "pypi" # Before adding a new package, check it's not listed under [packages] at # https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile # Replace "{ref}" in the above URL with the ref set below. -codeforlife = {ref = "v0.13.7", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "reset_students_password", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} # TODO: check if we need the below packages whitenoise = "==6.5.0" django-pipeline = "==2.0.8" @@ -34,7 +34,7 @@ google-cloud-container = "==2.3.0" # Before adding a new package, check it's not listed under [dev-packages] at # https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile # Replace "{ref}" in the above URL with the ref set below. -codeforlife = {ref = "v0.13.7", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "reset_students_password", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} # TODO: check if we need the below packages django-selenium-clean = "==0.3.3" django-test-migrations = "==1.2.0" diff --git a/backend/Pipfile.lock b/backend/Pipfile.lock index d8f3d366..095d1acb 100644 --- a/backend/Pipfile.lock +++ b/backend/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "9e02bb3bd37675c13e20e4e4add09570c4ead5526ab3650ee2a3d47864023155" + "sha256": "894ad5f63407ce1f006a370c2c2487d8e3874f0d503bd5b61fa3c06eaab399fe" }, "pipfile-spec": 6, "requires": { @@ -168,7 +168,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "30953150f4d2762e421d8f798badb57ad0318fd6" + "ref": "78c5b17988d899b2677f4a7d3a12408077d83351" }, "codeforlife-portal": { "hashes": [ @@ -806,6 +806,7 @@ "sha256:a6f5977418eff3b2d5500d54d9db50c8277a368436f4e4f8ddb1be3422870184", "sha256:f91456ead12ab3c6c2e9491cf33ba6d08357d802192379bb482f1033ade496f5" ], + "markers": "python_version >= '3.6'", "version": "==3.1.2" }, "pandas": { @@ -1058,7 +1059,7 @@ "sha256:0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86", "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", "version": "==2.8.2" }, "pytz": { @@ -1209,7 +1210,7 @@ "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926", "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", "version": "==1.16.0" }, "sortedcontainers": { @@ -1304,6 +1305,7 @@ "sha256:6a33ee89877bd9abc1158129f6e94be74e2679636b8a205b43b85206c3f0bbdd", "sha256:f72f148f54442c6b056bf931dbc34f986fd0c3b0b6b5a58d013c9aef274d0c88" ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'", "version": "==2.0.1" }, "xlwt": { @@ -1512,7 +1514,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "30953150f4d2762e421d8f798badb57ad0318fd6" + "ref": "78c5b17988d899b2677f4a7d3a12408077d83351" }, "codeforlife-portal": { "hashes": [ @@ -2184,6 +2186,7 @@ "sha256:a6f5977418eff3b2d5500d54d9db50c8277a368436f4e4f8ddb1be3422870184", "sha256:f91456ead12ab3c6c2e9491cf33ba6d08357d802192379bb482f1033ade496f5" ], + "markers": "python_version >= '3.6'", "version": "==3.1.2" }, "packaging": { @@ -2492,7 +2495,6 @@ "sha256:c7c6ca206e93355074ae32f7403e8ea12163b1163c976fee7d4d84027c162be5", "sha256:d45e0952f3727241918b8fd0f376f5ff6b301cc0777c6f9a556935c92d8a7d42" ], - "markers": "python_version >= '3.7'", "version": "==7.2.1" }, "pytest-cov": { @@ -2551,7 +2553,7 @@ "sha256:0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86", "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", "version": "==2.8.2" }, "pytz": { @@ -2727,7 +2729,7 @@ "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926", "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'", + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'", "version": "==1.16.0" }, "snapshottest": { @@ -2895,6 +2897,7 @@ "sha256:6a33ee89877bd9abc1158129f6e94be74e2679636b8a205b43b85206c3f0bbdd", "sha256:f72f148f54442c6b056bf931dbc34f986fd0c3b0b6b5a58d013c9aef274d0c88" ], + "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'", "version": "==2.0.1" }, "xlwt": { diff --git a/backend/api/serializers/user.py b/backend/api/serializers/user.py index eaf43b32..ef726f6f 100644 --- a/backend/api/serializers/user.py +++ b/backend/api/serializers/user.py @@ -3,18 +3,22 @@ Created on 18/01/2024 at 15:14:32(+00:00). """ -import string import typing as t from itertools import groupby from codeforlife.serializers import ModelListSerializer -from codeforlife.user.models import Class, Student, Teacher, User, UserProfile +from codeforlife.user.models import ( + Class, + Student, + StudentUser, + Teacher, + User, + UserProfile, +) from codeforlife.user.serializers import UserSerializer as _UserSerializer -from django.contrib.auth.hashers import make_password from django.contrib.auth.password_validation import ( validate_password as _validate_password, ) -from django.utils.crypto import get_random_string from rest_framework import serializers from .student import StudentSerializer @@ -36,40 +40,15 @@ def create(self, validated_data): # TODO: replace this logic with bulk creates for each object when we # switch to PostgreSQL. - users: t.List[User] = [] - for user_fields in validated_data: - password = get_random_string( - length=6, - allowed_chars=string.ascii_lowercase, - ) - - user = User.objects.create_user( + return [ + StudentUser.objects.create_user( first_name=user_fields["first_name"], - username=get_random_string(length=30), - password=make_password(password), - ) - users.append(user) - - # pylint: disable-next=protected-access - user._password = password - - login_id = None - while ( - login_id is None - or Student.objects.filter(login_id=login_id).exists() - ): - login_id = get_random_string(length=64) - - Student.objects.create( - class_field=classes[ + klass=classes[ user_fields["new_student"]["class_field"]["access_code"] ], - user=UserProfile.objects.create(user=user), - new_user=user, - login_id=login_id, ) - - return users + for user_fields in validated_data + ] def validate(self, attrs): super().validate(attrs) diff --git a/backend/api/tests/views/test_user.py b/backend/api/tests/views/test_user.py index 41560f1c..1f798d77 100644 --- a/backend/api/tests/views/test_user.py +++ b/backend/api/tests/views/test_user.py @@ -7,10 +7,12 @@ from codeforlife.permissions import OR from codeforlife.tests import ModelViewSetTestCase +from codeforlife.types import JsonDict from codeforlife.user.models import ( AdminSchoolTeacherUser, Class, SchoolTeacherUser, + StudentUser, User, ) from codeforlife.user.permissions import IsTeacher @@ -56,7 +58,7 @@ def _get_pk_and_token_for_user(self, email: str): # test: get permissions def test_get_permissions__bulk(self): - """Only school-teachers can perform bulk actions.""" + """Only admin-teachers or class-teachers can perform bulk actions.""" self.assert_get_permissions( permissions=[ OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)) @@ -64,8 +66,19 @@ def test_get_permissions__bulk(self): action="bulk", ) + def test_get_permissions__students__reset_password(self): + """ + Only admin-teachers or class-teachers can reset students' passwords. + """ + self.assert_get_permissions( + permissions=[ + OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)) + ], + action="students__reset_password", + ) + def test_get_permissions__partial_update__teacher(self): - """Only admin-school-teachers can update a teacher.""" + """Only admin-teachers can update a teacher.""" self.assert_get_permissions( permissions=[IsTeacher(is_admin=True)], action="partial_update", @@ -73,7 +86,7 @@ def test_get_permissions__partial_update__teacher(self): ) def test_get_permissions__partial_update__student(self): - """Only school-teachers can update a student.""" + """Only admin-teachers or class-teachers can update a student.""" self.assert_get_permissions( permissions=[ OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)) @@ -84,11 +97,7 @@ def test_get_permissions__partial_update__student(self): # test: get queryset - def _test_get_queryset__bulk(self, request_method: str): - assert User.objects.filter( - new_teacher__school=self.admin_school_teacher_user.teacher.school - ).exists() - + def _test_get_queryset(self, action: str, request_method: str): student_users = list( User.objects.filter( new_student__class_field__teacher__school=( @@ -102,15 +111,21 @@ def _test_get_queryset__bulk(self, request_method: str): request_method, user=self.admin_school_teacher_user ) - self.assert_get_queryset(student_users, action="bulk", request=request) + self.assert_get_queryset(student_users, action=action, request=request) def test_get_queryset__bulk__patch(self): """Bulk partial-update can only target student-users.""" - self._test_get_queryset__bulk("patch") + self._test_get_queryset(action="bulk", request_method="patch") def test_get_queryset__bulk__delete(self): """Bulk destroy can only target student-users.""" - self._test_get_queryset__bulk("delete") + self._test_get_queryset(action="bulk", request_method="delete") + + def test_get_queryset__students__reset_password(self): + """Resetting student passwords can only target student-users.""" + self._test_get_queryset( + action="students__reset_password", request_method="patch" + ) # test: bulk actions @@ -275,6 +290,37 @@ def test_reset_password__patch__indy(self): self.client.patch(viewname, data={"password": "N3wPassword"}) self.client.login(email=self.indy_email, password="N3wPassword") + # test: students actions + + def test_students__reset_password(self): + """Teacher can bulk reset students' password.""" + self.client.login_as(self.admin_school_teacher_user) + + student_user_ids = list( + StudentUser.objects.filter( + new_student__class_field__teacher__school=( + self.admin_school_teacher_user.teacher.school + ) + ).values_list("id", flat=True) + ) + assert student_user_ids + + response = self.client.patch( + self.reverse_action("students--reset-password"), + student_user_ids, + content_type="application/json", + ) + + passwords: JsonDict = response.json() + assert all(isinstance(password, str) for password in passwords.values()) + + updated_student_user_ids = [ + int(student_user_id) for student_user_id in passwords.keys() + ] + student_user_ids.sort() + updated_student_user_ids.sort() + self.assertListEqual(student_user_ids, updated_student_user_ids) + # test: generic actions def test_partial_update__teacher(self): diff --git a/backend/api/views/user.py b/backend/api/views/user.py index cd0b80a4..45ad3f1d 100644 --- a/backend/api/views/user.py +++ b/backend/api/views/user.py @@ -7,7 +7,7 @@ from codeforlife.permissions import OR from codeforlife.request import Request -from codeforlife.user.models import User +from codeforlife.user.models import StudentUser, User from codeforlife.user.permissions import IsTeacher from codeforlife.user.views import UserViewSet as _UserViewSet from django.contrib.auth.tokens import ( @@ -31,7 +31,7 @@ class UserViewSet(_UserViewSet): serializer_class = UserSerializer def get_permissions(self): - if self.action == "bulk": + if self.action in ["bulk", "students__reset_password"]: return [OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))] if self.action == "partial_update": if "teacher" in self.request.data: @@ -43,7 +43,9 @@ def get_permissions(self): def get_queryset(self): queryset = super().get_queryset() - if self.action == "bulk" and self.request.method in ["PATCH", "DELETE"]: + if ( + self.action == "bulk" and self.request.method in ["PATCH", "DELETE"] + ) or self.action == "students__reset_password": queryset = queryset.filter( new_student__isnull=False, new_student__class_field__isnull=False, @@ -142,3 +144,20 @@ def request_password_reset(self, request: Request): "token": token, } ) + + @action(detail=False, methods=["patch"], url_path="students/reset-password") + def students__reset_password(self, request: Request): + """Bulk reset students' password.""" + queryset = self._get_bulk_queryset(request.data) + + passwords: t.Dict[int, str] = {} + for pk in queryset.values_list("pk", flat=True): + student_user = StudentUser(pk=pk) + student_user.set_password() + + # pylint: disable-next=protected-access + passwords[pk] = t.cast(str, student_user._password) + + student_user.save() # TODO: replace with bulk update + + return Response(passwords)