diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 830acb3c..2e927468 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,7 @@ name: Main on: + pull_request: push: paths-ignore: - "**/*.md" diff --git a/backend/Pipfile b/backend/Pipfile index 01c43834..06d3f99e 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.2", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "v0.13.4", 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.2", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "v0.13.4", 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 08a6ab89..5d6b7532 100644 --- a/backend/Pipfile.lock +++ b/backend/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "6ae7e27489a2d7bb282f88822980d097fd377b5ee247f05edd9b05c6654b3472" + "sha256": "a4f65f80c5f383809d2b3acdc82ede3c6cd65b81150d4d9bcd0a8a19a4d1942c" }, "pipfile-spec": 6, "requires": { @@ -168,7 +168,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9361a531c264e6d637798d93424d818cef6be042" + "ref": "e5801cf92dc7d3edb3e20aca76a031ca8d618f76" }, "codeforlife-portal": { "hashes": [ @@ -1260,11 +1260,11 @@ }, "tzdata": { "hashes": [ - "sha256:aa3ace4329eeacda5b7beb7ea08ece826c28d761cda36e747cfbf97996d39bf3", - "sha256:dd54c94f294765522c77399649b4fefd95522479a664a0cec87f41bebc6148c9" + "sha256:2674120f8d891909751c38abcdfd386ac0a5a1127954fbc332af6b5ceae07efd", + "sha256:9068bc196136463f5245e51efda838afa15aaeca9903f49050dfa2679db4d252" ], "markers": "python_version >= '2'", - "version": "==2023.4" + "version": "==2024.1" }, "urllib3": { "hashes": [ @@ -1512,7 +1512,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9361a531c264e6d637798d93424d818cef6be042" + "ref": "e5801cf92dc7d3edb3e20aca76a031ca8d618f76" }, "codeforlife-portal": { "hashes": [ @@ -2492,7 +2492,7 @@ "sha256:c7c6ca206e93355074ae32f7403e8ea12163b1163c976fee7d4d84027c162be5", "sha256:d45e0952f3727241918b8fd0f376f5ff6b301cc0777c6f9a556935c92d8a7d42" ], - "markers": "python_version >= '3.7'", + "markers": "python_version < '3.10'", "version": "==7.2.1" }, "pytest-cov": { @@ -2822,11 +2822,11 @@ }, "tzdata": { "hashes": [ - "sha256:aa3ace4329eeacda5b7beb7ea08ece826c28d761cda36e747cfbf97996d39bf3", - "sha256:dd54c94f294765522c77399649b4fefd95522479a664a0cec87f41bebc6148c9" + "sha256:2674120f8d891909751c38abcdfd386ac0a5a1127954fbc332af6b5ceae07efd", + "sha256:9068bc196136463f5245e51efda838afa15aaeca9903f49050dfa2679db4d252" ], "markers": "python_version >= '2'", - "version": "==2023.4" + "version": "==2024.1" }, "urllib3": { "hashes": [ diff --git a/backend/api/fixtures/school_1.json b/backend/api/fixtures/school_1.json index 578938c8..c72454e6 100644 --- a/backend/api/fixtures/school_1.json +++ b/backend/api/fixtures/school_1.json @@ -108,5 +108,47 @@ "access_code": "ZZ222", "teacher": 7 } + }, + { + "model": "common.schoolteacherinvitation", + "pk": 1, + "fields": { + "token": "pbkdf2_sha256$260000$hbsAadmrRo744BTM6NofUb$ePs/7vi6sSzOPpiWxNhXMZnNnE7aXOpzIhxrAa/rdiU=", + "school": 2, + "from_teacher": 7, + "invited_teacher_first_name": "Invited", + "invited_teacher_last_name": "Teacher", + "invited_teacher_email": "invited@teacher.com", + "invited_teacher_is_admin": false, + "expiry": "2024-02-09 20:26:08.298402+00:00" + } + }, + { + "model": "common.schoolteacherinvitation", + "pk": 2, + "fields": { + "token": "pbkdf2_sha256$260000$hbsAadmrRo744BTM6NofUb$ePs/7vi6sSzOPpiWxNhXMZnNnE7aXOpzIhxrAa/rdiU=", + "school": 2, + "from_teacher": 7, + "invited_teacher_first_name": "Invited", + "invited_teacher_last_name": "Teacher", + "invited_teacher_email": "invited@teacher.com", + "invited_teacher_is_admin": false, + "expiry": "9999-02-09 20:26:08.298402+00:00" + } + }, + { + "model": "common.schoolteacherinvitation", + "pk": 3, + "fields": { + "token": "pbkdf2_sha256$260000$hbsAadmrRo744BTM6NofUb$ePs/7vi6sSzOPpiWxNhXMZnNnE7aXOpzIhxrAa/rdiU=", + "school": 2, + "from_teacher": 7, + "invited_teacher_first_name": "Invited", + "invited_teacher_last_name": "Teacher", + "invited_teacher_email": "teacher@school1.com", + "invited_teacher_is_admin": false, + "expiry": "9999-02-09 20:26:08.298402+00:00" + } } -] \ No newline at end of file +] diff --git a/backend/api/models/__init__.py b/backend/api/models/__init__.py new file mode 100644 index 00000000..c45ded7b --- /dev/null +++ b/backend/api/models/__init__.py @@ -0,0 +1,6 @@ +""" +© Ocado Group +Created on 06/02/2024 at 15:13:00(+00:00). +""" + +from common.models import SchoolTeacherInvitation diff --git a/backend/api/serializers/__init__.py b/backend/api/serializers/__init__.py index 2d829ea6..5f37dfae 100644 --- a/backend/api/serializers/__init__.py +++ b/backend/api/serializers/__init__.py @@ -6,6 +6,7 @@ from .auth_factor import AuthFactorSerializer from .klass import ClassSerializer from .school import SchoolSerializer +from .school_teacher_invitation import SchoolTeacherInvitationSerializer from .student import StudentSerializer from .teacher import TeacherSerializer from .user import UserSerializer diff --git a/backend/api/serializers/school_teacher_invitation.py b/backend/api/serializers/school_teacher_invitation.py new file mode 100644 index 00000000..3f94a961 --- /dev/null +++ b/backend/api/serializers/school_teacher_invitation.py @@ -0,0 +1,57 @@ +""" +© Ocado Group +Created on 09/02/2024 at 16:14:00(+00:00). +""" + +from datetime import timedelta + +from codeforlife.serializers import ModelSerializer +from django.contrib.auth.hashers import make_password +from django.utils import timezone +from django.utils.crypto import get_random_string +from rest_framework import serializers + +from ..models import SchoolTeacherInvitation + + +# pylint: disable-next=missing-class-docstring,too-many-ancestors +class SchoolTeacherInvitationSerializer( + ModelSerializer[SchoolTeacherInvitation] +): + first_name = serializers.CharField(source="invited_teacher_first_name") + last_name = serializers.CharField(source="invited_teacher_last_name") + email = serializers.EmailField(source="invited_teacher_email") + is_admin = serializers.BooleanField(source="invited_teacher_is_admin") + + class Meta: + model = SchoolTeacherInvitation + fields = [ + "id", + "first_name", + "last_name", + "email", + "is_admin", + "expiry", + ] + extra_kwargs = { + "id": {"read_only": True}, + "expiry": {"read_only": True}, + } + + def create(self, validated_data): + user = self.request_admin_school_teacher_user + + token = get_random_string(length=32) + validated_data["token"] = make_password(token) + validated_data["school"] = user.teacher.school + validated_data["from_teacher"] = user.teacher + validated_data["expiry"] = timezone.now() + timedelta(days=30) + + invitation = super().create(validated_data) + invitation._token = token + return invitation + + def update(self, instance, validated_data): + instance.expiry = timezone.now() + timedelta(days=30) + instance.save() + return instance diff --git a/backend/api/serializers/user.py b/backend/api/serializers/user.py index 580ebb94..eaf43b32 100644 --- a/backend/api/serializers/user.py +++ b/backend/api/serializers/user.py @@ -8,7 +8,7 @@ from itertools import groupby from codeforlife.serializers import ModelListSerializer -from codeforlife.user.models import Class, Student, User, UserProfile +from codeforlife.user.models import Class, Student, 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 ( @@ -119,31 +119,82 @@ class UserSerializer(_UserSerializer): class Meta(_UserSerializer.Meta): fields = [ *_UserSerializer.Meta.fields, + "student", + "teacher", "password", "current_password", ] extra_kwargs = { **_UserSerializer.Meta.extra_kwargs, "first_name": {"read_only": False}, + "last_name": {"read_only": False, "required": False}, + "email": {"read_only": False}, "password": {"write_only": True, "required": False}, } list_serializer_class = UserListSerializer def validate(self, attrs): - if self.view.action != "reset-password": - pass + if self.instance is not None and self.view.action != "reset-password": # TODO: make current password required when changing self-profile. + pass + + if "new_teacher" in attrs and "last_name" not in attrs: + raise serializers.ValidationError( + "Last name is required.", code="last_name_required" + ) return attrs def validate_password(self, value: str): """ Validate the new password depending on user type. - :param value: the new password """ - _validate_password(value, self.instance) + + # If we're creating a new user, we do not yet have the user object. + # Therefore, we need to create a dummy user and pass it to the password + # validators so they know what type of user we have. + instance = self.instance + if not instance: + instance = User() + + user_type: str = self.context["user_type"] + if user_type == "teacher": + Teacher(new_user=instance) + elif user_type == "student": + Student(new_user=instance) + + _validate_password(value, instance) + return value + def create(self, validated_data): + user = User.objects.create_user( + username=validated_data["email"], + email=validated_data["email"], + password=validated_data["password"], + first_name=validated_data["first_name"], + last_name=validated_data.get("last_name"), + ) + + user_profile = UserProfile.objects.create( + user=user, + is_verified=self.context.get("is_verified", False), + ) + + if "new_teacher" in validated_data: + Teacher.objects.create( + user=user_profile, + new_user=user, + is_admin=validated_data["new_teacher"]["is_admin"], + school=self.context.get("school"), + ) + elif "new_student" in validated_data: + pass # TODO + + # TODO: Handle signing new user up to newsletter if checkbox ticked + + return user + def update(self, instance, validated_data): password = validated_data.get("password") diff --git a/backend/api/signals/school_teacher_invitation.py b/backend/api/signals/school_teacher_invitation.py new file mode 100644 index 00000000..af8c8fe1 --- /dev/null +++ b/backend/api/signals/school_teacher_invitation.py @@ -0,0 +1,22 @@ +""" +© Ocado Group +Created on 09/02/2024 at 17:02:00(+00:00). + +All signals for the SchoolTeacherInvitation model. +""" + +from django.db.models.signals import post_save +from django.dispatch import receiver + +from ..models import SchoolTeacherInvitation + + +# pylint: disable=unused-argument +@receiver(post_save, sender=SchoolTeacherInvitation) +def school_teacher_invitation__post_save( + sender, instance: SchoolTeacherInvitation, *args, **kwargs +): + """Send invitation email to invited teacher.""" + + instance._token # TODO: send email to invited teacher with differing + # content based on whether the email is already linked to an account or not. diff --git a/backend/api/tests/serializers/test_school_teacher_invitation.py b/backend/api/tests/serializers/test_school_teacher_invitation.py new file mode 100644 index 00000000..ca35323a --- /dev/null +++ b/backend/api/tests/serializers/test_school_teacher_invitation.py @@ -0,0 +1,67 @@ +""" +© Ocado Group +Created on 13/02/2024 at 13:44:00(+00:00). +""" +import datetime +from unittest.mock import patch + +from codeforlife.tests import ModelSerializerTestCase +from codeforlife.user.models import AdminSchoolTeacherUser +from django.utils import timezone + +from ...models import SchoolTeacherInvitation +from ...serializers import SchoolTeacherInvitationSerializer + + +# pylint: disable-next=missing-class-docstring +class TestSchoolTeacherInvitationSerializer( + ModelSerializerTestCase[SchoolTeacherInvitation] +): + model_serializer_class = SchoolTeacherInvitationSerializer + fixtures = ["school_1"] + + def setUp(self): + self.admin_school_teacher_user = AdminSchoolTeacherUser.objects.get( + email="admin.teacher@school1.com" + ) + self.invitation = SchoolTeacherInvitation.objects.get(pk=1) + + @patch( + "backend.api.serializers.school_teacher_invitation.make_password", + return_value="token", + ) + def test_create(self, _): + """Can successfully create.""" + now = timezone.now() + with patch.object(timezone, "now", return_value=now): + self.assert_create( + { + "invited_teacher_first_name": "NewTeacher", + "invited_teacher_last_name": "NewTeacher", + "invited_teacher_email": "invited@teacher.com", + "invited_teacher_is_admin": False, + }, + new_data={ + "token": "token", + "school": self.admin_school_teacher_user.teacher.school.id, + "from_teacher": self.admin_school_teacher_user.teacher.id, + "expiry": now + datetime.timedelta(days=30), + }, + context={ + "request": self.request_factory.post( + user=self.admin_school_teacher_user + ) + }, + ) + + def test_update(self): + """Can successfully update.""" + now = timezone.now() + with patch.object(timezone, "now", return_value=now): + self.assert_update( + self.invitation, + {}, + new_data={ + "expiry": now + datetime.timedelta(days=30), + }, + ) diff --git a/backend/api/tests/serializers/test_user.py b/backend/api/tests/serializers/test_user.py index aa45f8f2..08768f96 100644 --- a/backend/api/tests/serializers/test_user.py +++ b/backend/api/tests/serializers/test_user.py @@ -15,10 +15,7 @@ class TestUserSerializer(ModelSerializerTestCase[User]): fixtures = ["school_1"] def test_validate__first_name_not_unique_per_class_in_data(self): - """ - First name must be unique per class in data. - """ - + """First name must be unique per class in data.""" self.assert_validate( attrs=[ { @@ -43,10 +40,7 @@ def test_validate__first_name_not_unique_per_class_in_data(self): ) def test_validate__first_name_not_unique_per_class_in_db(self): - """ - First name must be unique per class in database. - """ - + """First name must be unique per class in database.""" klass = Class.objects.get(name="Class 1 @ School 1") assert klass is not None diff --git a/backend/api/tests/signals/test_school_teacher_invitation.py b/backend/api/tests/signals/test_school_teacher_invitation.py new file mode 100644 index 00000000..38e51948 --- /dev/null +++ b/backend/api/tests/signals/test_school_teacher_invitation.py @@ -0,0 +1,12 @@ +""" +© Ocado Group +Created on 09/02/2024 at 19:45:00(+00:00). +""" + +from django.test import TestCase + + +class TestSchoolTeacherInvitation(TestCase): + def test_post_save(self): + """Creating a teacher invitation sends a verification email.""" + raise NotImplementedError() # TODO: implement diff --git a/backend/api/tests/views/test_school_teacher_invitation.py b/backend/api/tests/views/test_school_teacher_invitation.py new file mode 100644 index 00000000..01bf2b25 --- /dev/null +++ b/backend/api/tests/views/test_school_teacher_invitation.py @@ -0,0 +1,222 @@ +""" +© Ocado Group +Created on 09/02/2024 at 17:18:00(+00:00). +""" + +from codeforlife.permissions import AllowNone +from codeforlife.tests import ModelViewSetTestCase +from codeforlife.user.models import User +from codeforlife.user.permissions import InSchool, IsTeacher +from rest_framework import status + +from ...models import SchoolTeacherInvitation +from ...views import SchoolTeacherInvitationViewSet + + +# pylint: disable-next=missing-class-docstring +class TestSchoolTeacherInvitationViewSet( + ModelViewSetTestCase[SchoolTeacherInvitation] +): + basename = "school-teacher-invitation" + model_view_set_class = SchoolTeacherInvitationViewSet + fixtures = ["non_school_teacher", "school_1"] + school_admin_teacher_email = "admin.teacher@school1.com" + non_school_teacher_email = "teacher@noschool.com" + + def _login_admin_school_teacher(self): + return self.client.login_school_teacher( + email=self.school_admin_teacher_email, + password="password", + is_admin=True, + ) + + def setUp(self): + self.expired_invitation = SchoolTeacherInvitation.objects.get(pk=1) + assert self.expired_invitation.is_expired + + self.new_user_invitation = SchoolTeacherInvitation.objects.get(pk=2) + assert not self.new_user_invitation.is_expired + + with self.assertRaises(User.DoesNotExist): + User.objects.get( + email__iexact=self.new_user_invitation.invited_teacher_email + ) + + self.existing_user_invitation = SchoolTeacherInvitation.objects.get( + pk=3 + ) + assert not self.existing_user_invitation.is_expired + User.objects.get( + email__iexact=self.existing_user_invitation.invited_teacher_email + ) + + def test_get_permissions__bulk(self): + """No one is allowed to perform bulk actions.""" + self.assert_get_permissions( + permissions=[AllowNone()], + action="bulk", + ) + + def test_get_permissions__create(self): + """Only admin-teachers can create an invitation.""" + self.assert_get_permissions( + permissions=[IsTeacher(is_admin=True), InSchool()], + action="create", + ) + + def test_get_permissions__partial_update(self): + """Only admin-teachers can update an invitation.""" + self.assert_get_permissions( + permissions=[IsTeacher(is_admin=True), InSchool()], + action="partial_update", + ) + + def test_get_permissions__retrieve(self): + """Only admin-teachers can retrieve an invitation.""" + self.assert_get_permissions( + permissions=[IsTeacher(is_admin=True), InSchool()], + action="retrieve", + ) + + def test_get_permissions__list(self): + """Only admin-teachers can list invitations.""" + self.assert_get_permissions( + permissions=[IsTeacher(is_admin=True), InSchool()], + action="list", + ) + + def test_get_permissions__destroy(self): + """Only admin-teachers can destroy an invitation.""" + self.assert_get_permissions( + permissions=[IsTeacher(is_admin=True), InSchool()], + action="destroy", + ) + + def test_create(self): + """Can successfully create an invitation.""" + self._login_admin_school_teacher() + + self.client.create( + { + "first_name": "Invited", + "last_name": "Teacher", + "email": "invited@teacher.com", + "is_admin": "False", + }, + ) + + def test_partial_update(self): + """Can successfully update an invitation's expiry.""" + self._login_admin_school_teacher() + + invitation = self.expired_invitation + + self.client.partial_update(invitation, {}) + + invitation.refresh_from_db() + + assert not invitation.is_expired + + def test_destroy(self): + """Can successfully destroy an invitation.""" + self._login_admin_school_teacher() + + invitation = self.new_user_invitation + + self.client.destroy(invitation) + + with self.assertRaises(invitation.DoesNotExist): + invitation.refresh_from_db() + + def test_accept__get__invalid_token(self): + """Accept invite raises 400 on GET with invalid token""" + invitation = self.new_user_invitation + + viewname = self.reverse_action( + "accept", + # pylint: disable-next=protected-access + kwargs={"pk": invitation.pk, "token": "whatever"}, + ) + + response = self.client.get( + viewname, status_code_assertion=status.HTTP_400_BAD_REQUEST + ) + + assert response.data["non_field_errors"] == ["Incorrect token."] + + def test_accept__get__expired(self): + """Accept invite raises 400 on GET with expired invite""" + invitation = self.expired_invitation + + viewname = self.reverse_action( + "accept", + # pylint: disable-next=protected-access + kwargs={"pk": invitation.pk, "token": "token"}, + ) + + response = self.client.get( + viewname, status_code_assertion=status.HTTP_400_BAD_REQUEST + ) + + assert response.data["non_field_errors"] == [ + "The invitation has expired." + ] + + def test_accept__get__existing_email(self): + """Accept invite raises 400 on GET with pre-existing email""" + invitation = self.existing_user_invitation + + viewname = self.reverse_action( + "accept", + # pylint: disable-next=protected-access + kwargs={"pk": invitation.pk, "token": "token"}, + ) + + response = self.client.get( + viewname, status_code_assertion=status.HTTP_400_BAD_REQUEST + ) + + assert response.data["non_field_errors"] == [ + "It looks like an account is already registered with this email " + "address. You will need to delete the other account first or " + "change the email associated with it in order to proceed. You will " + "then be able to access this page." + ] + + def test_accept__get(self): + """Accept invite GET succeeds""" + invitation = self.new_user_invitation + + viewname = self.reverse_action( + "accept", + # pylint: disable-next=protected-access + kwargs={"pk": invitation.pk, "token": "token"}, + ) + + self.client.get(viewname) + + def test_accept__post(self): + """Invited teacher can set password and their account is created""" + password = "InvitedPassword1!" + + invitation = self.new_user_invitation + + viewname = self.reverse_action( + "accept", + # pylint: disable-next=protected-access + kwargs={"pk": invitation.pk, "token": "token"}, + ) + + self.client.post( + viewname, data={"password": "InvitedPassword1!"}, format="json" + ) + + user = self.client.login( + email=invitation.invited_teacher_email, + password=password, + ) + assert user.teacher.school == invitation.school + assert user.teacher.is_admin == invitation.invited_teacher_is_admin + + with self.assertRaises(invitation.DoesNotExist): + invitation.refresh_from_db() diff --git a/backend/api/tests/views/test_user.py b/backend/api/tests/views/test_user.py index e80347b7..cb0d9bb4 100644 --- a/backend/api/tests/views/test_user.py +++ b/backend/api/tests/views/test_user.py @@ -27,12 +27,13 @@ class TestUserViewSet(ModelViewSetTestCase[User]): fixtures = ["independent", "non_school_teacher", "school_1"] non_school_teacher_email = "teacher@noschool.com" + school_teacher_email = "teacher@school1.com" indy_email = "indy@man.com" def _login_school_teacher(self): return self.client.login_school_teacher( - email="maxplanck@codeforlife.com", - password="Password1", + email=self.school_teacher_email, + password="password", is_admin=False, ) @@ -43,20 +44,14 @@ def _get_pk_and_token_for_user(self, email: str): return user.pk, token def test_get_permissions__bulk(self): - """ - Only school-teachers can perform bulk actions. - """ - + """Only school-teachers can perform bulk actions.""" self.assert_get_permissions( permissions=[IsTeacher(), InSchool()], action="bulk", ) def test_get_permissions__partial_update__teacher(self): - """ - Only admin-school-teachers can update a teacher. - """ - + """Only admin-school-teachers can update a teacher.""" self.assert_get_permissions( permissions=[IsTeacher(is_admin=True), InSchool()], action="partial_update", @@ -64,10 +59,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 school-teachers can update a student.""" self.assert_get_permissions( permissions=[IsTeacher(), InSchool()], action="partial_update", @@ -76,7 +68,6 @@ def test_get_permissions__partial_update__student(self): def test_bulk_create__students(self): """Teacher can bulk create students.""" - user = self._login_school_teacher() klass: t.Optional[Class] = user.teacher.class_teacher.first() @@ -144,8 +135,7 @@ def test_reset_password__invalid_pk(self): ) viewname = self.reverse_action( - "reset-password", - kwargs={"pk": "whatever", "token": token}, + "reset-password", kwargs={"pk": "whatever", "token": token} ) response = self.client.get( @@ -161,8 +151,7 @@ def test_reset_password__invalid_token(self): pk, _ = self._get_pk_and_token_for_user(self.non_school_teacher_email) viewname = self.reverse_action( - "reset-password", - kwargs={"pk": pk, "token": "whatever"}, + "reset-password", kwargs={"pk": pk, "token": "whatever"} ) response = self.client.get( @@ -180,8 +169,7 @@ def test_reset_password__get(self): ) viewname = self.reverse_action( - "reset-password", - kwargs={"pk": pk, "token": token}, + "reset-password", kwargs={"pk": pk, "token": token} ) self.client.get(viewname) @@ -193,8 +181,7 @@ def test_reset_password__patch__teacher(self): ) viewname = self.reverse_action( - "reset-password", - kwargs={"pk": pk, "token": token}, + "reset-password", kwargs={"pk": pk, "token": token} ) self.client.patch(viewname, data={"password": "N3wPassword!"}) @@ -216,10 +203,7 @@ def test_reset_password__patch__indy(self): self.client.login(email=self.indy_email, password="N3wPassword") def test_partial_update__teacher(self): - """ - Admin-school-teacher can update another teacher's profile. - """ - + """Admin-school-teacher can update another teacher's profile.""" admin_school_teacher_user = self.client.login_school_teacher( email="admin.teacher@school1.com", password="password", @@ -238,6 +222,7 @@ def test_partial_update__teacher(self): self.client.partial_update( other_school_teacher_user, { + "last_name": other_school_teacher_user.first_name, "teacher": { "is_admin": not other_school_teacher_user.teacher.is_admin, }, diff --git a/backend/api/urls.py b/backend/api/urls.py index ac99ff09..bc27d614 100644 --- a/backend/api/urls.py +++ b/backend/api/urls.py @@ -9,6 +9,7 @@ AuthFactorViewSet, ClassViewSet, OtpBypassTokenViewSet, + SchoolTeacherInvitationViewSet, SchoolViewSet, UserViewSet, ) @@ -29,6 +30,11 @@ OtpBypassTokenViewSet, basename="otp-bypass-token", ) +router.register( + "schools/teacher-invitations", + SchoolTeacherInvitationViewSet, + basename="school-teacher-invitation", +) router.register( "schools", SchoolViewSet, diff --git a/backend/api/views/__init__.py b/backend/api/views/__init__.py index 61ecd06e..6cff35ae 100644 --- a/backend/api/views/__init__.py +++ b/backend/api/views/__init__.py @@ -7,4 +7,5 @@ from .klass import ClassViewSet from .otp_bypass_token import OtpBypassTokenViewSet from .school import SchoolViewSet +from .school_teacher_invitation import SchoolTeacherInvitationViewSet from .user import UserViewSet diff --git a/backend/api/views/school_teacher_invitation.py b/backend/api/views/school_teacher_invitation.py new file mode 100644 index 00000000..2039981c --- /dev/null +++ b/backend/api/views/school_teacher_invitation.py @@ -0,0 +1,114 @@ +""" +© Ocado Group +Created on 09/02/2024 at 16:14:00(+00:00). +""" + +from codeforlife.permissions import AllowNone +from codeforlife.request import Request +from codeforlife.user.models import User +from codeforlife.user.permissions import InSchool, IsTeacher +from codeforlife.views import ModelViewSet +from django.contrib.auth.hashers import check_password +from rest_framework import status +from rest_framework.decorators import action +from rest_framework.permissions import AllowAny +from rest_framework.response import Response + +from ..models import SchoolTeacherInvitation +from ..serializers import SchoolTeacherInvitationSerializer, UserSerializer + + +# pylint: disable-next=missing-class-docstring,too-many-ancestors +class SchoolTeacherInvitationViewSet(ModelViewSet[SchoolTeacherInvitation]): + http_method_names = ["get", "post", "patch", "delete"] + serializer_class = SchoolTeacherInvitationSerializer + + def get_permissions(self): + if self.action in [ + "retrieve", + "list", + "create", + "partial_update", + "destroy", + ]: + return [IsTeacher(is_admin=True), InSchool()] + if self.action == "bulk": + return [AllowNone()] + + return super().get_permissions() + + def get_queryset(self): + queryset = SchoolTeacherInvitation.objects.all() + if self.action == "accept": + return queryset + + return queryset.filter( + school=self.request_admin_school_teacher_user.teacher.school + ) + + @action( + detail=True, + methods=["get", "post"], + url_path="accept/(?P.+)", + permission_classes=[AllowAny], + ) + def accept(self, request: Request, pk: str, token: str): + """ + Handles accepting a teacher's invitation to join their school. On GET, + checks validity of the invitation token. On PATCH, rechecks this + param, performs password validation and creates the new Teacher. + """ + invitation = self.get_object() + + if not check_password(token, invitation.token): + return Response( + {"non_field_errors": ["Incorrect token."]}, + status=status.HTTP_400_BAD_REQUEST, + ) + + if invitation.is_expired: + return Response( + {"non_field_errors": ["The invitation has expired."]}, + status=status.HTTP_400_BAD_REQUEST, + ) + + if User.objects.filter( + email__iexact=invitation.invited_teacher_email + ).exists(): + return Response( + { + "non_field_errors": [ + "It looks like an account is already registered with " + "this email address. You will need to delete the other " + "account first or change the email associated with it " + "in order to proceed. You will then be able to access " + "this page." + ] + }, + status=status.HTTP_400_BAD_REQUEST, + ) + + if request.method == "POST": + context = self.get_serializer_context() + context["is_verified"] = True + context["user_type"] = "teacher" + context["school"] = invitation.school + + data = { + "first_name": invitation.invited_teacher_first_name, + "last_name": invitation.invited_teacher_last_name, + "email": invitation.invited_teacher_email, + **request.data, + "teacher": { + "is_admin": invitation.invited_teacher_is_admin, + **request.data.get("teacher", {}), + }, + } + + serializer = UserSerializer(data=data, context=context) + serializer.is_valid(raise_exception=True) + serializer.save() + + invitation.delete() + + return Response() diff --git a/backend/api/views/user.py b/backend/api/views/user.py index 96c6a9ea..7a9346e1 100644 --- a/backend/api/views/user.py +++ b/backend/api/views/user.py @@ -84,7 +84,8 @@ def reset_password( serializer.is_valid(raise_exception=True) serializer.save() - # TODO: Check if need to handle resetting ratelimit & unblocking of user + # TODO: Check if need to handle resetting ratelimit & unblocking + # of user return Response() @@ -105,7 +106,8 @@ def request_password_reset(self, request: Request): try: user = User.objects.get(email__iexact=email) except User.DoesNotExist: - # NOTE: Always return a 200 here - a noticeable change in behaviour would allow email enumeration. + # NOTE: Always return a 200 here - a noticeable change in + # behaviour would allow email enumeration. return Response() token = default_token_generator.make_token(user)