diff --git a/backend/api/models/checks.py b/backend/api/models/checks.py index 7f5aa570..dfb7e2f0 100644 --- a/backend/api/models/checks.py +++ b/backend/api/models/checks.py @@ -14,8 +14,6 @@ class FileExtension(models.Model): unique=True ) -# TODO: Remove zip.* translations - class StructureCheck(models.Model): """Model that represents a structure check for a project. @@ -100,7 +98,6 @@ class ExtraCheck(models.Model): ) # Maximum memory the container uses in MB - # TODO: Set max and min memory_limit = models.PositiveSmallIntegerField( default=128, blank=False, diff --git a/backend/api/permissions/group_permissions.py b/backend/api/permissions/group_permissions.py index e75da21d..00f15f6a 100644 --- a/backend/api/permissions/group_permissions.py +++ b/backend/api/permissions/group_permissions.py @@ -1,8 +1,10 @@ +from api.models.group import Group from api.permissions.role_permissions import (is_assistant, is_student, is_teacher) from authentication.models import User from rest_framework.permissions import SAFE_METHODS, BasePermission from rest_framework.request import Request +from rest_framework.views import APIView from rest_framework.viewsets import ViewSet @@ -59,6 +61,23 @@ def has_object_permission(self, request: Request, view: ViewSet, group) -> bool: class GroupSubmissionPermission(BasePermission): """Permission class for submission related group endpoints""" + def has_permission(self, request: Request, view: APIView) -> bool: + user: User = request.user + group_id = view.kwargs.get('pk') + group: Group | None = Group.objects.get(id=group_id) if group_id else None + + if group is None: + return True + + # Teachers and assistants of that course can view all submissions + if is_teacher(user): + return group.project.course.teachers.filter(id=user.teacher.id).exists() + + if is_assistant(user): + return group.project.course.assistants.filter(id=user.assistant.id).exists() + + return is_student(user) and group.students.filter(id=user.student.id).exists() + def had_object_permission(self, request: Request, view: ViewSet, group) -> bool: user: User = request.user course = group.project.course diff --git a/backend/api/permissions/project_permissions.py b/backend/api/permissions/project_permissions.py index 55ff054a..a13b4946 100644 --- a/backend/api/permissions/project_permissions.py +++ b/backend/api/permissions/project_permissions.py @@ -13,11 +13,6 @@ def has_permission(self, request: Request, view: ViewSet) -> bool: """Check if user has permission to view a general project endpoint.""" user: User = request.user - # TODO: Sure return True corresponds with the comments made above - # The general project endpoint that lists all projects is not accessible for any role. - if request.method in SAFE_METHODS: - return True - # We only allow teachers and assistants to create new projects. return is_teacher(user) or is_assistant(user) diff --git a/backend/api/signals.py b/backend/api/signals.py index b705255a..82bc905c 100644 --- a/backend/api/signals.py +++ b/backend/api/signals.py @@ -7,24 +7,26 @@ from api.models.student import Student from api.models.submission import (ExtraCheckResult, StateEnum, StructureCheckResult, Submission) -from api.tasks.docker_image import task_docker_image_build +from api.tasks.docker_image import (task_docker_image_build, + task_docker_image_remove) from api.tasks.extra_check import task_extra_check_start from api.tasks.structure_check import task_structure_check_start from authentication.models import User from authentication.signals import user_created -from django.db.models.signals import post_delete, post_save +from django.db.models.signals import post_delete, post_save, pre_delete from django.dispatch import Signal, receiver -# Signals +# MARK: Signals run_docker_image_build = Signal() +run_docker_image_remove = Signal() run_all_checks = Signal() run_structure_checks = Signal() run_extra_checks = Signal() -# Receivers +# MARK: Receivers @receiver(user_created) @@ -44,6 +46,11 @@ def _run_docker_image_build(docker_image: DockerImage, **_): task_docker_image_build.apply_async((docker_image,)) +@receiver(run_docker_image_remove) +def _run_docker_image_remove(docker_image: DockerImage, **_): + task_docker_image_remove.apply_async((docker_image,)) + + @receiver(run_all_checks) def _run_all_checks(submission: Submission, **_): # Get all checks @@ -75,7 +82,7 @@ def _run_extra_checks(submission: Submission, **_): task_extra_check_start.apply_async((True, extra_check_result,)) -# Hooks +# MARK: Hooks @receiver(post_save, sender=StructureCheck) @@ -108,7 +115,6 @@ def hook_extra_check(sender, instance: ExtraCheck, **kwargs): @receiver(post_save, sender=Submission) def hook_submission(sender, instance: Submission, created: bool, **kwargs): - # TODO: Maybe remove the raw check if created and not kwargs.get('raw', False): run_all_checks.send(sender=Submission, submission=instance) pass @@ -116,11 +122,16 @@ def hook_submission(sender, instance: Submission, created: bool, **kwargs): @receiver(post_save, sender=DockerImage) def hook_docker_image(sender, instance: DockerImage, created: bool, **kwargs): - # Run when it's created if created: run_docker_image_build.send(sender=DockerImage, docker_image=instance) -# Helpers + +@receiver(pre_delete, sender=DockerImage) +def hook_docker_image_delete(sender, instance: DockerImage, **kwargs): + run_docker_image_remove.send(sender=DockerImage, docker_image=instance) + + +# MARK: Helpers # Get all structure checks and create a result for each one diff --git a/backend/api/tasks/docker_image.py b/backend/api/tasks/docker_image.py index cf614a0d..3636ef36 100644 --- a/backend/api/tasks/docker_image.py +++ b/backend/api/tasks/docker_image.py @@ -6,7 +6,6 @@ from ypovoli.settings import MEDIA_ROOT -# TODO: Remove built image when it's deleted from the database @shared_task def task_docker_image_build(docker_image: DockerImage): # Set state @@ -19,9 +18,18 @@ def task_docker_image_build(docker_image: DockerImage): client.images.build(path=MEDIA_ROOT, dockerfile=docker_image.file.path, tag=get_docker_image_tag(docker_image), rm=True, quiet=True, forcerm=True) docker_image.state = StateEnum.READY - except (docker.errors.APIError, docker.errors.BuildError, docker.errors.APIError, TypeError): + except (docker.errors.APIError, docker.errors.BuildError, TypeError): docker_image.state = StateEnum.ERROR # TODO: Sent notification # Update the state docker_image.save() + + +@shared_task +def task_docker_image_remove(docker_image: DockerImage): + try: + client = docker.from_env() + client.images.remove(get_docker_image_tag(docker_image)) + except docker.errors.APIError: + pass diff --git a/backend/api/views/course_view.py b/backend/api/views/course_view.py index c3f42786..3adc2e6c 100644 --- a/backend/api/views/course_view.py +++ b/backend/api/views/course_view.py @@ -10,9 +10,9 @@ from api.serializers.assistant_serializer import (AssistantIDSerializer, AssistantSerializer) from api.serializers.course_serializer import (CourseCloneSerializer, - SaveInvitationLinkSerializer, CourseSerializer, CreateCourseSerializer, + SaveInvitationLinkSerializer, StudentJoinSerializer, StudentLeaveSerializer, TeacherJoinSerializer, @@ -39,7 +39,6 @@ class CourseViewSet(viewsets.ModelViewSet): serializer_class = CourseSerializer permission_classes = [IsAdminUser | CoursePermission] - # TODO: Creating should return the info of the new object and not a message "created" (General TODO) def create(self, request: Request, *_): """Override the create method to add the teacher to the course""" serializer = CreateCourseSerializer(data=request.data, context={"request": request}) diff --git a/backend/api/views/docker_view.py b/backend/api/views/docker_view.py index 7792de1b..1e2da9d7 100644 --- a/backend/api/views/docker_view.py +++ b/backend/api/views/docker_view.py @@ -1,22 +1,19 @@ from api.models.docker import DockerImage from api.permissions.docker_permissions import DockerPermission from api.serializers.docker_serializer import DockerImageSerializer -from rest_framework.permissions import IsAdminUser +from api.views.pagination.basic_pagination import BasicPagination from django.db.models import Q from django.db.models.manager import BaseManager from rest_framework.decorators import action from rest_framework.mixins import (CreateModelMixin, DestroyModelMixin, RetrieveModelMixin, UpdateModelMixin) +from rest_framework.permissions import IsAdminUser from rest_framework.request import Request from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from api.views.pagination.basic_pagination import BasicPagination - - -# TODO: Remove update abilities, maybe? -class DockerImageViewSet(RetrieveModelMixin, CreateModelMixin, UpdateModelMixin, DestroyModelMixin, GenericViewSet): +class DockerImageViewSet(RetrieveModelMixin, UpdateModelMixin, CreateModelMixin, DestroyModelMixin, GenericViewSet): queryset = DockerImage.objects.all() serializer_class = DockerImageSerializer @@ -64,8 +61,6 @@ def patch_public(self, request: Request, **_) -> Response: return Response(serializer.data) - # TODO: Maybe not necessary - # https://www.django-rest-framework.org/api-guide/permissions/#overview-of-access-restriction-methods def list(self, request: Request) -> Response: images: BaseManager[DockerImage] = DockerImage.objects.all() if not request.user.is_staff: diff --git a/backend/api/views/group_view.py b/backend/api/views/group_view.py index c3595f4d..7c118bcd 100644 --- a/backend/api/views/group_view.py +++ b/backend/api/views/group_view.py @@ -40,7 +40,6 @@ def students(self, request, **_): ) return Response(serializer.data) - # TODO: I can access this endpoint unauthorized @action(detail=True, permission_classes=[IsAdminUser | GroupSubmissionPermission]) def submissions(self, request, **_): """Returns a list of submissions for the given group""" diff --git a/backend/api/views/submission_view.py b/backend/api/views/submission_view.py index f1f42a58..37911477 100644 --- a/backend/api/views/submission_view.py +++ b/backend/api/views/submission_view.py @@ -16,7 +16,6 @@ from rest_framework.viewsets import GenericViewSet -# TODO: Permission to ask for logs class SubmissionViewSet(RetrieveModelMixin, GenericViewSet): queryset = Submission.objects.all() serializer_class = SubmissionSerializer