From 141e2dbc9b751d174bb977dd2139a9de1e3bc547 Mon Sep 17 00:00:00 2001 From: lander Date: Sun, 17 Mar 2024 21:24:15 +0100 Subject: [PATCH 1/6] backend(api) chore: first set of changes to student_view permissions --- .../api/permissions/student_permissions.py | 21 +++++++++++++++++++ backend/api/views/student_view.py | 5 +++-- backend/authentication/models.py | 5 ++++- 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 backend/api/permissions/student_permissions.py diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py new file mode 100644 index 00000000..f3afa1aa --- /dev/null +++ b/backend/api/permissions/student_permissions.py @@ -0,0 +1,21 @@ +from rest_framework.permissions import BasePermission, SAFE_METHODS +from api.permissions.role_permissions import is_teacher +from authentication.models import User + +class StudentPermission(BasePermission): + + # Dit is garbage omdat altijd de has_permission eerst moet slagen. + + # IsAdminUser is already defined but because of DRF has_permissions must be present + # https://www.django-rest-framework.org/api-guide/permissions/#custom-permissions + def has_permission(self, request, view): + """Check if user has permission to view a general student endpoint.""" + user: User = request.user + if view.action in ['list', 'create', 'update', 'partial_update']: + return user.is_staff + return user.is_staff == True + + def has_object_permission(self, request, view, obj): + """Check if user has permission to view a detailed group endpoint""" + user: User = request.user + return request.method in SAFE_METHODS and (user.id == request.user.id or is_teacher(user)) diff --git a/backend/api/views/student_view.py b/backend/api/views/student_view.py index ef0c0289..28348cbf 100644 --- a/backend/api/views/student_view.py +++ b/backend/api/views/student_view.py @@ -2,6 +2,7 @@ from rest_framework.decorators import action from rest_framework.response import Response from rest_framework.permissions import IsAdminUser +from api.permissions.student_permissions import StudentPermission from api.permissions.role_permissions import IsSameUser, IsTeacher from api.models.student import Student from api.serializers.student_serializer import StudentSerializer @@ -12,12 +13,12 @@ class StudentViewSet(viewsets.ModelViewSet): queryset = Student.objects.all() serializer_class = StudentSerializer - permission_classes = [IsAdminUser | IsTeacher | IsSameUser] + permission_classes = [IsAdminUser | StudentPermission] @action(detail=True) def courses(self, request, **_): """Returns a list of courses for the given student""" - student = self.get_object() + student = self.get_object() courses = student.courses.all() # Serialize the course objects diff --git a/backend/authentication/models.py b/backend/authentication/models.py index 066d6fbb..adce2eb7 100644 --- a/backend/authentication/models.py +++ b/backend/authentication/models.py @@ -34,6 +34,10 @@ class User(AbstractBaseUser): USERNAME_FIELD = "username" EMAIL_FIELD = "email" + def make_admin(self): + self.is_staff = True + self.save() + @staticmethod def get_dummy_admin(): return User( @@ -45,7 +49,6 @@ def get_dummy_admin(): is_staff=True ) - class Faculty(models.Model): """This model represents a faculty.""" From 023099b66703eba7a36473dede8f3368b41c41ee Mon Sep 17 00:00:00 2001 From: Lander Maes Date: Sun, 17 Mar 2024 23:50:37 +0100 Subject: [PATCH 2/6] backend(api) chore: updated student_premissions --- backend/api/permissions/student_permissions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py index f3afa1aa..2086abfa 100644 --- a/backend/api/permissions/student_permissions.py +++ b/backend/api/permissions/student_permissions.py @@ -11,9 +11,9 @@ class StudentPermission(BasePermission): def has_permission(self, request, view): """Check if user has permission to view a general student endpoint.""" user: User = request.user - if view.action in ['list', 'create', 'update', 'partial_update']: + if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: return user.is_staff - return user.is_staff == True + return True def has_object_permission(self, request, view, obj): """Check if user has permission to view a detailed group endpoint""" From f30ef179ba5f27456411646c74d923c99bbdc63c Mon Sep 17 00:00:00 2001 From: lander Date: Mon, 18 Mar 2024 11:46:35 +0100 Subject: [PATCH 3/6] chore: update assistant_permissions --- backend/api/permissions/assistant_permissions.py | 2 +- backend/api/permissions/student_permissions.py | 2 +- backend/api/views/assistant_view.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/api/permissions/assistant_permissions.py b/backend/api/permissions/assistant_permissions.py index 5cb76fc1..77adb6f9 100644 --- a/backend/api/permissions/assistant_permissions.py +++ b/backend/api/permissions/assistant_permissions.py @@ -11,7 +11,7 @@ def has_permission(self, request: Request, view: ViewSet) -> bool: """Check if user has permission to view a general assistant endpoint.""" user = request.user - if view.action == "list": + if view.action in ['list', 'create', 'destroy']: # Only teachers can query the assistant list. return user.is_authenticated and is_teacher(user) diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py index 2086abfa..43d7bc59 100644 --- a/backend/api/permissions/student_permissions.py +++ b/backend/api/permissions/student_permissions.py @@ -12,7 +12,7 @@ def has_permission(self, request, view): """Check if user has permission to view a general student endpoint.""" user: User = request.user if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: - return user.is_staff + return False return True def has_object_permission(self, request, view, obj): diff --git a/backend/api/views/assistant_view.py b/backend/api/views/assistant_view.py index e20bec81..87328432 100644 --- a/backend/api/views/assistant_view.py +++ b/backend/api/views/assistant_view.py @@ -1,6 +1,6 @@ from rest_framework.decorators import action from rest_framework.response import Response -from rest_framework.viewsets import ReadOnlyModelViewSet +from rest_framework.viewsets import ModelViewSet from rest_framework.permissions import IsAdminUser from api.permissions.assistant_permissions import AssistantPermission from ..models.assistant import Assistant @@ -8,7 +8,7 @@ from ..serializers.course_serializer import CourseSerializer -class AssistantViewSet(ReadOnlyModelViewSet): +class AssistantViewSet(ModelViewSet): queryset = Assistant.objects.all() serializer_class = AssistantSerializer From 8cb3bcc3470e55767969abe5cd5a20c84db40858 Mon Sep 17 00:00:00 2001 From: lander Date: Mon, 18 Mar 2024 12:14:22 +0100 Subject: [PATCH 4/6] chore: updating teacher_view --- backend/api/permissions/student_permissions.py | 8 ++------ backend/api/permissions/teacher_permissions.py | 17 +++++++++++++++++ backend/api/views/teacher_view.py | 12 ++++++------ 3 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 backend/api/permissions/teacher_permissions.py diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py index 43d7bc59..abd1a596 100644 --- a/backend/api/permissions/student_permissions.py +++ b/backend/api/permissions/student_permissions.py @@ -1,13 +1,9 @@ -from rest_framework.permissions import BasePermission, SAFE_METHODS +from rest_framework.permissions import IsAuthenticated, SAFE_METHODS from api.permissions.role_permissions import is_teacher from authentication.models import User -class StudentPermission(BasePermission): +class StudentPermission(IsAuthenticated): - # Dit is garbage omdat altijd de has_permission eerst moet slagen. - - # IsAdminUser is already defined but because of DRF has_permissions must be present - # https://www.django-rest-framework.org/api-guide/permissions/#custom-permissions def has_permission(self, request, view): """Check if user has permission to view a general student endpoint.""" user: User = request.user diff --git a/backend/api/permissions/teacher_permissions.py b/backend/api/permissions/teacher_permissions.py new file mode 100644 index 00000000..7a31d3c6 --- /dev/null +++ b/backend/api/permissions/teacher_permissions.py @@ -0,0 +1,17 @@ +from rest_framework.permissions import IsAuthenticated, SAFE_METHODS +from authentication.models import User + +# (Almost) same as StudentPermission +class TeacherPermission(IsAuthenticated): + + def has_permission(self, request, view): + """Check if user has permission to view a general Teacher endpoint.""" + user: User = request.user + if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: + return False + return True + + def has_object_permission(self, request, view, obj): + """Check if user has permission to view a detailed group endpoint""" + user: User = request.user + return request.method in SAFE_METHODS and user.id == request.user.id diff --git a/backend/api/views/teacher_view.py b/backend/api/views/teacher_view.py index c16d4167..7189b880 100644 --- a/backend/api/views/teacher_view.py +++ b/backend/api/views/teacher_view.py @@ -1,22 +1,22 @@ from rest_framework import status from rest_framework.decorators import action from rest_framework.response import Response -from rest_framework.viewsets import ReadOnlyModelViewSet +from rest_framework.viewsets import ModelViewSet from rest_framework.permissions import IsAdminUser from api.models.course import Course from api.models.teacher import Teacher from api.serializers.teacher_serializer import TeacherSerializer from api.serializers.course_serializer import CourseSerializer -from api.permissions.role_permissions import IsSameUser +from api.permissions.teacher_permissions import TeacherPermission +from rest_framework.permissions import IsAuthenticated - -class TeacherViewSet(ReadOnlyModelViewSet): +class TeacherViewSet(ModelViewSet): queryset = Teacher.objects.all() serializer_class = TeacherSerializer - permission_classes = [IsAdminUser | IsSameUser] + permission_classes = [IsAdminUser | TeacherPermission] - @action(detail=True, methods=["get"]) + @action(detail=True, methods=["get"], permission_classes=[IsAuthenticated]) def courses(self, request, pk=None): """Returns a list of courses for the given teacher""" teacher = self.get_object() From 744f9946e508d512ba645ba93eaeeeffb9b74423 Mon Sep 17 00:00:00 2001 From: lander Date: Wed, 27 Mar 2024 15:57:22 +0100 Subject: [PATCH 5/6] fix: linting errors on #131 --- backend/api/permissions/student_permissions.py | 2 +- backend/api/permissions/teacher_permissions.py | 2 +- backend/api/views/student_view.py | 2 +- backend/api/views/teacher_view.py | 1 + backend/authentication/models.py | 1 + 5 files changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py index abd1a596..182a229c 100644 --- a/backend/api/permissions/student_permissions.py +++ b/backend/api/permissions/student_permissions.py @@ -2,11 +2,11 @@ from api.permissions.role_permissions import is_teacher from authentication.models import User + class StudentPermission(IsAuthenticated): def has_permission(self, request, view): """Check if user has permission to view a general student endpoint.""" - user: User = request.user if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: return False return True diff --git a/backend/api/permissions/teacher_permissions.py b/backend/api/permissions/teacher_permissions.py index 7a31d3c6..9a7c10ad 100644 --- a/backend/api/permissions/teacher_permissions.py +++ b/backend/api/permissions/teacher_permissions.py @@ -1,12 +1,12 @@ from rest_framework.permissions import IsAuthenticated, SAFE_METHODS from authentication.models import User + # (Almost) same as StudentPermission class TeacherPermission(IsAuthenticated): def has_permission(self, request, view): """Check if user has permission to view a general Teacher endpoint.""" - user: User = request.user if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: return False return True diff --git a/backend/api/views/student_view.py b/backend/api/views/student_view.py index 28348cbf..1261d6ab 100644 --- a/backend/api/views/student_view.py +++ b/backend/api/views/student_view.py @@ -18,7 +18,7 @@ class StudentViewSet(viewsets.ModelViewSet): @action(detail=True) def courses(self, request, **_): """Returns a list of courses for the given student""" - student = self.get_object() + student = self.get_object() courses = student.courses.all() # Serialize the course objects diff --git a/backend/api/views/teacher_view.py b/backend/api/views/teacher_view.py index 4d723d44..cdc6160d 100644 --- a/backend/api/views/teacher_view.py +++ b/backend/api/views/teacher_view.py @@ -8,6 +8,7 @@ from api.permissions.teacher_permissions import TeacherPermission from rest_framework.permissions import IsAuthenticated + class TeacherViewSet(ModelViewSet): queryset = Teacher.objects.all() serializer_class = TeacherSerializer diff --git a/backend/authentication/models.py b/backend/authentication/models.py index adce2eb7..dff350cb 100644 --- a/backend/authentication/models.py +++ b/backend/authentication/models.py @@ -49,6 +49,7 @@ def get_dummy_admin(): is_staff=True ) + class Faculty(models.Model): """This model represents a faculty.""" From dea315946fd0a35692daaaf752bc8831d3b75500 Mon Sep 17 00:00:00 2001 From: lander Date: Fri, 29 Mar 2024 14:25:17 +0100 Subject: [PATCH 6/6] cleanup for #131 --- backend/api/permissions/student_permissions.py | 4 +--- backend/api/permissions/teacher_permissions.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/backend/api/permissions/student_permissions.py b/backend/api/permissions/student_permissions.py index 182a229c..ab1afd57 100644 --- a/backend/api/permissions/student_permissions.py +++ b/backend/api/permissions/student_permissions.py @@ -7,9 +7,7 @@ class StudentPermission(IsAuthenticated): def has_permission(self, request, view): """Check if user has permission to view a general student endpoint.""" - if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: - return False - return True + return view.action == 'retrieve' def has_object_permission(self, request, view, obj): """Check if user has permission to view a detailed group endpoint""" diff --git a/backend/api/permissions/teacher_permissions.py b/backend/api/permissions/teacher_permissions.py index 9a7c10ad..22b8d96a 100644 --- a/backend/api/permissions/teacher_permissions.py +++ b/backend/api/permissions/teacher_permissions.py @@ -7,9 +7,7 @@ class TeacherPermission(IsAuthenticated): def has_permission(self, request, view): """Check if user has permission to view a general Teacher endpoint.""" - if view.action in ['list', 'create', 'update', 'partial_update', 'destroy']: - return False - return True + return view.action == 'retrieve' def has_object_permission(self, request, view, obj): """Check if user has permission to view a detailed group endpoint"""