diff --git a/Palto/Palto/api/v1/permissions.py b/Palto/Palto/api/v1/permissions.py index c0bb9ee..a64c0e0 100644 --- a/Palto/Palto/api/v1/permissions.py +++ b/Palto/Palto/api/v1/permissions.py @@ -27,8 +27,6 @@ def permission_from_helper_class(model: Type[models.ModelPermissionHelper]) -> T # for writing, only allowed users return True - return False - def has_object_permission(self, request, view, obj: models.User) -> bool: if request.method in permissions.SAFE_METHODS: # for reading, only allow if the user can see the object diff --git a/Palto/Palto/api/v1/serializers.py b/Palto/Palto/api/v1/serializers.py index e5fcd45..f0aadef 100644 --- a/Palto/Palto/api/v1/serializers.py +++ b/Palto/Palto/api/v1/serializers.py @@ -5,6 +5,7 @@ A serializers tell the API how should a model should be serialized to be used by """ from typing import Type +from django.forms import model_to_dict from rest_framework import serializers from rest_framework.exceptions import PermissionDenied @@ -25,31 +26,31 @@ class ModelSerializerContrains(serializers.ModelSerializer): def create(self, validated_data): # get the fields that this user can modify - field_contrains = self.Meta.model.user_fields_contraints(self.context["request"].user) + field_contraints = self.Meta.model.user_fields_contraints(self.context["request"].user) - # for every constraints - for field, constraints in field_contrains.items(): + # for every constraint + for field, constraints in field_contraints.items(): # check if the value is in the constraints. value = validated_data.get(field) - if value is not None and value not in constraints: + if value in constraints(validated_data): raise PermissionDenied(f"You are not allowed to use this value for the field {field}.") return super().create(validated_data) def update(self, instance, validated_data): # get the fields that this user can modify - field_contrains = self.Meta.model.user_fields_contraints(self.context["request"].user) + field_constraints = self.Meta.model.user_fields_contraints(self.context["request"].user) - # for every constraints - for field, constraints in field_contrains.items(): + # for every constraint + for field, constraints in field_constraints.items(): # check if the value of the request is in the constraints. value = validated_data.get(field) - if value is not None and value not in constraints: + if value in constraints(validated_data): raise PermissionDenied(f"You are not allowed to use this value for the field {field}.") # check if the value of the already existing instance is in the constraints. value = getattr(instance, field, None) - if value is not None and value not in constraints: + if value in constraints(model_to_dict(instance)): raise PermissionDenied(f"You are not allowed to use this value for the field {field}.") # check that the user is managing the department diff --git a/Palto/Palto/api/v1/tests.py b/Palto/Palto/api/v1/tests.py index 81f7c62..f391479 100644 --- a/Palto/Palto/api/v1/tests.py +++ b/Palto/Palto/api/v1/tests.py @@ -119,54 +119,6 @@ class DepartmentApiTestCase(test.APITestCase): response = self.client.post("/api/v1/departments/", data=self.DEPARTMENT_CREATION_DATA) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - def test_permission_anonymous(self): - """ Test the API permission for an anonymous user """ - - # TODO: use reverse to get the url ? - # TODO: use api endpoint as class attribute ? - self.client.logout() - - # check for a get request - response = self.client.get("/api/v1/departments/") - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - - # check for a post request - response = self.client.post("/api/v1/departments/", data=self.DEPARTMENT_CREATION_DATA) - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - - def test_permission_unrelated(self): - """ Test the API permission for an unrelated user """ - - # TODO: use reverse to get the url ? - self.client.force_login(self.user_other) - - # check for a get request and that he can't see anything - response = self.client.get("/api/v1/departments/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json()["count"], 0) - - # check for a post request - response = self.client.post("/api/v1/departments/", data=self.DEPARTMENT_CREATION_DATA) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_permission_related(self): - """ Test the API permission for a related user """ - - # TODO: use reverse to get the url ? - student1, student2 = factories.FakeUserFactory(), factories.FakeUserFactory() - department = factories.FakeDepartmentFactory(students=(student1, student2)) - - self.client.force_login(student1) - - # check for a get request and that he can see the other student - response = self.client.get("/api/v1/departments/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn(serializers.UserSerializer(student2).data, response.json()["results"]) - - # check for a post request - response = self.client.post("/api/v1/departments/", data=self.DEPARTMENT_CREATION_DATA) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - class StudentGroupApiTestCase(test.APITestCase): def setUp(self): @@ -225,58 +177,6 @@ class StudentGroupApiTestCase(test.APITestCase): response = self.client.post("/api/v1/student_groups/", data=self.student_group_creation_data) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - def test_permission_unrelated(self): - """ Test the API permission for an unrelated user """ - - # TODO: use reverse to get the url ? - for user in (self.user_other, *self.test_students_other): - self.client.force_login(user) - - # check for a get request and that he can't see anything - response = self.client.get("/api/v1/student_groups/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json()["count"], 0) - - # check for a post request - response = self.client.post("/api/v1/student_groups/", data=self.student_group_creation_data) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_permission_related(self): - """ Test the API permission for a related user """ - - for user in self.test_students_group: - # TODO: use reverse to get the url ? - self.client.force_login(user) - - # check for a get request and that he can see the students - response = self.client.get("/api/v1/student_groups/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual( - list(serializers.UserSerializer(student).data for student in self.test_students_group), - response.json()["results"]["students"] - ) - - # check for a post request - response = self.client.post("/api/v1/student_groups/", data=self.student_group_creation_data) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_permission_owner(self): - """ Test the API permission for the owner """ - - # TODO: use reverse to get the url ? - self.client.force_login(self.test_teacher_owner) - - # check for a get request and that he can see the students - response = self.client.get("/api/v1/student_groups/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual( - list(serializers.UserSerializer(student).data for student in self.test_students_group), - response.json()["results"]["students"] - ) - - # check for a post request - response = self.client.post("/api/v1/student_groups/", data=self.student_group_creation_data) - # TODO: autorisé ? class TeachingUnitApiTestCase(test.APITestCase): pass diff --git a/Palto/Palto/models.py b/Palto/Palto/models.py index 5399a6b..ac7f3d2 100644 --- a/Palto/Palto/models.py +++ b/Palto/Palto/models.py @@ -3,21 +3,19 @@ Models for the Palto project. Models are the class that represent and abstract the database. """ -import operator + import uuid from abc import abstractmethod from datetime import datetime, timedelta -from functools import reduce -from typing import Iterable +from typing import Iterable, Callable, Any -from django.contrib.auth import get_user_model from django.contrib.auth.models import AbstractUser +from django.core.exceptions import ValidationError from django.db import models from django.db.models import QuerySet, Q, F # TODO(Raphaël): split permissions from models for readability -# TODO(Raphaël): allow other function for permissions than in class ModelPermissionHelper: @@ -26,13 +24,14 @@ class ModelPermissionHelper: """ Return True if the user can create a new instance of this object """ - + return user.is_superuser @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[dict], QuerySet]]: """ - Return the list of fields in that model that the user can modify + Return a dictionary of field associated to a function giving all the allowed values for this field. + For example, this can be used to check that a user manage a department before allowing modification. """ return {} @@ -134,9 +133,9 @@ class Department(models.Model, ModelPermissionHelper): name: str = models.CharField(max_length=64, unique=True) email: str = models.EmailField() - managers = models.ManyToManyField(to=get_user_model(), blank=True, related_name="managing_departments") - teachers = models.ManyToManyField(to=get_user_model(), blank=True, related_name="teaching_departments") - students = models.ManyToManyField(to=get_user_model(), blank=True, related_name="studying_departments") + managers = models.ManyToManyField(to=User, blank=True, related_name="managing_departments") + teachers = models.ManyToManyField(to=User, blank=True, related_name="teaching_departments") + students = models.ManyToManyField(to=User, blank=True, related_name="studying_departments") def __repr__(self): return f"<{self.__class__.__name__} id={str(self.id)[:8]} name={self.name!r}>" @@ -205,9 +204,9 @@ class StudentGroup(models.Model, ModelPermissionHelper): id: uuid.UUID = models.UUIDField(default=uuid.uuid4, primary_key=True, editable=False, max_length=36) name: str = models.CharField(max_length=128) - owner = models.ForeignKey(to=get_user_model(), on_delete=models.CASCADE, related_name="owning_groups") department = models.ForeignKey(to=Department, on_delete=models.CASCADE, related_name="student_groups") - students = models.ManyToManyField(to=get_user_model(), blank=True, related_name="student_groups") + owner = models.ForeignKey(to=User, on_delete=models.CASCADE, related_name="owning_groups") + students = models.ManyToManyField(to=User, blank=True, related_name="student_groups") def __repr__(self): return f"<{self.__class__.__name__} id={str(self.id)[:8]} name={self.name!r}>" @@ -215,6 +214,19 @@ class StudentGroup(models.Model, ModelPermissionHelper): def __str__(self): return self.name + # validators + + def clean(self): + super().clean() + + # owner check + if self.department not in self.owner.teaching_departments: + raise ValidationError("The owner is not related to the department.") + + # students check + if not all(self.department in student.studying_departments for student in self.students.all()): + raise ValidationError("A student is not related to the department.") + # permissions @classmethod @@ -232,14 +244,16 @@ class StudentGroup(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[dict], QuerySet]]: # if the user is admin, no contrains if user.is_superuser: return {} return { - # the managers and teachers can only interact with their departments - "department": (user.managing_departments | user.teaching_departments).distinct() + # the user can only interact with a related departments + "department": lambda data: user.managing_departments | user.teaching_departments, + # the owner must be a teacher or a manager of this department + "owner": lambda data: data["department"].managers | data["department"].teachers, } @classmethod @@ -292,8 +306,8 @@ class TeachingUnit(models.Model, ModelPermissionHelper): department = models.ForeignKey(to=Department, on_delete=models.CASCADE, related_name="teaching_units") - managers = models.ManyToManyField(to=get_user_model(), blank=True, related_name="managing_units") - teachers = models.ManyToManyField(to=get_user_model(), blank=True, related_name="teaching_units") + managers = models.ManyToManyField(to=User, blank=True, related_name="managing_units") + teachers = models.ManyToManyField(to=User, blank=True, related_name="teaching_units") student_groups = models.ManyToManyField(to=StudentGroup, blank=True, related_name="studying_units") def __repr__(self): @@ -302,6 +316,23 @@ class TeachingUnit(models.Model, ModelPermissionHelper): def __str__(self): return self.name + # validations + + def clean(self): + super().clean() + + # managers check + if not all(self.department in manager.managing_departments for manager in self.managers.all()): + raise ValidationError("A manager is not related to the department.") + + # teachers check + if not all(self.department in teacher.teaching_departments for teacher in self.teachers.all()): + raise ValidationError("A teacher is not related to the department.") + + # student groups check + if not all(self.department in student_group.department for student_group in self.student_groups.all()): + raise ValidationError("A student group is not related to the department.") + # permissions @classmethod @@ -315,14 +346,14 @@ class TeachingUnit(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[dict], QuerySet]]: # if the user is admin, no contrains if user.is_superuser: return {} return { - # the managers can only interact with their departments - "department": user.managing_departments + # a user can only interact with a related departments + "department": lambda data: user.managing_departments | user.teaching_departments } @classmethod @@ -369,11 +400,20 @@ class StudentCard(models.Model, ModelPermissionHelper): uid: bytes = models.BinaryField(max_length=7) department: Department = models.ForeignKey(to=Department, on_delete=models.CASCADE, related_name="student_cards") - owner: User = models.ForeignKey(to=get_user_model(), on_delete=models.CASCADE, related_name="student_cards") + owner: User = models.ForeignKey(to=User, on_delete=models.CASCADE, related_name="student_cards") def __repr__(self): return f"<{self.__class__.__name__} id={str(self.id)[:8]} owner={self.owner.username!r}>" + # validations + + def clean(self): + super().clean() + + # owner check + if self.department not in self.owner.studying_departments: + raise ValidationError("The student is not related to the department.") + # permissions @classmethod @@ -386,19 +426,14 @@ class StudentCard(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[Any, dict], bool]]: # if the user is admin, no contrains if user.is_superuser: return {} return { - # the managers can only interact with their departments - "department": user.managing_departments, - # the owner of the card can be any students in a department that is managed by the user - "owner": reduce( - operator.or_, - (department.students.all() for department in user.managing_departments) - ) + # a user can only interact with a related departments + "department": lambda field, data: field in user.managing_departments, } @classmethod @@ -447,7 +482,7 @@ class TeachingSession(models.Model, ModelPermissionHelper): unit = models.ForeignKey(to=TeachingUnit, on_delete=models.CASCADE, related_name="sessions") group = models.ForeignKey(to=StudentGroup, on_delete=models.CASCADE, related_name="teaching_sessions") - teacher = models.ForeignKey(to=get_user_model(), on_delete=models.CASCADE, related_name="teaching_sessions") + teacher = models.ForeignKey(to=User, on_delete=models.CASCADE, related_name="teaching_sessions") def __repr__(self): return f"<{self.__class__.__name__} id={str(self.id)[:8]} unit={self.unit.name!r} start={self.start}>" @@ -459,6 +494,19 @@ class TeachingSession(models.Model, ModelPermissionHelper): def end(self) -> datetime: return self.start + self.duration + # validations + + def clean(self): + super().clean() + + # group check + if self.unit.department not in self.group.department: + raise ValidationError("The group is not related to the unit department.") + + # teacher check + if self.unit not in self.teacher.teaching_units: + raise ValidationError("The teacher is not related to the unit.") + # permissions @classmethod @@ -480,47 +528,21 @@ class TeachingSession(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[Any, dict], bool]]: # if the user is admin, no contrains if user.is_superuser: return {} return { - # the managers can only interact with their departments - "department": (user.managing_departments | user.teaching_departments).distinct(), - "teacher": - # the teacher can be any teacher in a department that the user is managing - reduce( - operator.or_, - (department.teachers.all() for department in user.managing_departments) - ) | reduce( - # or a teacher in a unit that the user is managing - operator.or_, - (department.teachers.all() for department in user.managing_units) - ) | ( - # or the user itself - User.objects.filter(pk=user.pk) - ), - "unit": - # the unit can be any unit in the department that the user is managing - reduce( - operator.or_, - (department.teaching_units.all() for department in user.managing_departments) - ) | ( - # or the units that the user is teaching - user.teaching_sessions - ), - "group": - # any group of a department where the user is a manager - reduce( - operator.or_, - (department.student_groups for department in user.managing_departments) - ) | - # any group where the user is a manager or a teacher of a unit - reduce( - operator.or_, - (unit.student_groups for unit in (user.managing_units | user.teaching_units)) - ) + # the managers can only interact with their units + "unit": lambda data: ( + # all the units the user is managing + user.managing_units | + # all the units the user is teaching + user.teaching_units | + # all the units of the department the user is managing + TeachingUnit.objects.filter(pk__in=user.managing_departments.values("teaching_units")) + ) } @classmethod @@ -571,7 +593,7 @@ class Attendance(models.Model, ModelPermissionHelper): date: datetime = models.DateTimeField() student: User = models.ForeignKey( - to=get_user_model(), + to=User, on_delete=models.CASCADE, related_name="attended_sessions" ) @@ -590,6 +612,15 @@ class Attendance(models.Model, ModelPermissionHelper): f">" ) + # validations + + def clean(self): + super().clean() + + # student check + if self.student not in self.session.group.students: + raise ValidationError("The student is not related to the student group.") + # permissions @classmethod @@ -611,50 +642,24 @@ class Attendance(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[Any, dict], bool]]: # if the user is admin, no contrains if user.is_superuser: return {} return { - # the managers can only interact with their departments - "department": user.managing_departments | user.teaching_departments, - "student": - # student can be any student from a department the user is managing or teaching - reduce( - operator.or_, - ( - department.students.all() - for department in (user.managing_departments | user.teaching_departments) - ) - ) | - # or any student from a unit the user is managing or teaching - reduce( - operator.or_, - ( - student_group.students.all() - for unit in (user.managing_units | user.teaching_units) - for student_group in unit.student_groups - ) - ), - "session": - # the session can be any session where the user is managing the department - reduce( - operator.or_, - ( - unit.sessions - for department in user.managing_departments - for unit in department.teaching_units - ) - ) | - # or where is the user is a teacher - reduce( - operator.or_, - ( - unit.sessions - for unit in (user.teaching_units | user.managing_units) - ) + "session": lambda data: ( + # the sessions that the user has taught + user.teaching_sessions | + # a session of a unit the user is managing + TeachingSession.objects.filter(pk__in=user.managing_units.values("sessions")) | + # all the sessions in a department the user is managing + TeachingSession.objects.filter( + pk__in=TeachingUnit.objects.filter( + pk__in=user.managing_departments.values("teaching_units") + ).values("sessions") ) + ) } @classmethod @@ -706,7 +711,7 @@ class Absence(models.Model, ModelPermissionHelper): message: str = models.TextField() department: Department = models.ForeignKey(to=Department, on_delete=models.CASCADE, related_name="absences") - student: User = models.ForeignKey(to=get_user_model(), on_delete=models.CASCADE, related_name="absences") + student: User = models.ForeignKey(to=User, on_delete=models.CASCADE, related_name="absences") start: datetime = models.DateTimeField() end: datetime = models.DateTimeField() @@ -724,6 +729,15 @@ class Absence(models.Model, ModelPermissionHelper): def __str__(self): return f"[{str(self.id)[:8]}] {self.student}" + # validations + + def clean(self): + super().clean() + + # student check + if self.department not in self.student.studying_departments: + raise ValidationError("The student is not related to the department.") + # properties def related_sessions(self) -> QuerySet[TeachingSession]: @@ -751,16 +765,14 @@ class Absence(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[dict], QuerySet]]: # if the user is admin, no contrains if user.is_superuser: return {} return { # all the departments the user is studying in - "department": user.studying_departments, - # the student itself - "student": User.objects.filter(pk=user.pk), + "department": lambda data: user.studying_departments, } @classmethod @@ -831,14 +843,14 @@ class AbsenceAttachment(models.Model, ModelPermissionHelper): return True @classmethod - def user_fields_contraints(cls, user: "User") -> dict[str, QuerySet]: + def user_fields_contraints(cls, user: "User") -> dict[str, Callable[[dict], QuerySet]]: # if the user is admin, no contrains if user.is_superuser: return {} return { # all the departments the user is studying in - "absence": user.absences, + "absence": lambda data: user.absences, } @classmethod