From 0b9ebf9e215e7d377885c49d6fb99726b23cc39e Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 1 Oct 2024 09:18:18 +0200 Subject: [PATCH 1/4] Add documents permission --- server/vbv_lernwelt/course_session/views.py | 6 +- server/vbv_lernwelt/iam/permissions.py | 23 ++- .../iam/tests/test_permissions.py | 152 ++++++++++++++++++ 3 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 server/vbv_lernwelt/iam/tests/test_permissions.py diff --git a/server/vbv_lernwelt/course_session/views.py b/server/vbv_lernwelt/course_session/views.py index ac4af629..8839da3b 100644 --- a/server/vbv_lernwelt/course_session/views.py +++ b/server/vbv_lernwelt/course_session/views.py @@ -4,12 +4,14 @@ from rest_framework.response import Response from vbv_lernwelt.course.models import CircleDocument from vbv_lernwelt.course.serializers import CircleDocumentSerializer -from vbv_lernwelt.iam.permissions import has_course_session_access +from vbv_lernwelt.iam.permissions import ( + has_course_session_document_access, +) @api_view(["GET"]) def get_course_session_documents(request, course_session_id): - if not has_course_session_access(request.user, course_session_id): + if not has_course_session_document_access(request.user, course_session_id): raise PermissionDenied() circle_documents = CircleDocument.objects.filter( diff --git a/server/vbv_lernwelt/iam/permissions.py b/server/vbv_lernwelt/iam/permissions.py index 23a7cc2a..1a5971f9 100644 --- a/server/vbv_lernwelt/iam/permissions.py +++ b/server/vbv_lernwelt/iam/permissions.py @@ -44,6 +44,19 @@ def has_course_session_access(user, course_session_id: int): ).exists() +def has_course_session_document_access(user, course_session_id: int): + if user.is_superuser: + return True + + return ( + CourseSessionUser.objects.filter( + course_session_id=course_session_id, user=user + ).exists() + or is_course_session_berufsbildner(user, course_session_id) + or CourseSessionGroup.objects.filter(course_session=course_session_id, supervisor=user.id).exists() + ) + + def has_course_session_preview(user, course_session_id: int): if user.is_superuser: return True @@ -336,10 +349,10 @@ def can_view_course_completions( str(user.id) == target_user_id or is_course_session_expert(user=user, course_session_id=course_session_id) or is_agent_for_user( - agent=user, - participant_user_id=target_user_id, - course_session_id=course_session_id, - ) + agent=user, + participant_user_id=target_user_id, + course_session_id=course_session_id, + ) ) @@ -370,7 +383,7 @@ def course_session_permissions(user: User, course_session_id: int) -> list[str]: "learning-mentor": has_learning_mentor, "learning-mentor::edit-mentors": has_learning_mentor and is_member, "learning-mentor::guide-members": course_has_learning_mentor - and is_learning_mentor, + and is_learning_mentor, "preview": has_course_session_preview(user, course_session_id), "media-library": ( is_supervisor or is_expert or is_member or is_berufsbildner diff --git a/server/vbv_lernwelt/iam/tests/test_permissions.py b/server/vbv_lernwelt/iam/tests/test_permissions.py new file mode 100644 index 00000000..872fb5e7 --- /dev/null +++ b/server/vbv_lernwelt/iam/tests/test_permissions.py @@ -0,0 +1,152 @@ +from django.test import TestCase + +from vbv_lernwelt.course.creators.test_utils import ( + create_course, + create_course_session, + create_user, +) +from vbv_lernwelt.course.models import CourseSessionUser +from vbv_lernwelt.course_session_group.models import CourseSessionGroup +from vbv_lernwelt.iam.permissions import ( + has_course_session_document_access, +) +from vbv_lernwelt.learning_mentor.models import ( + AgentParticipantRelation, + AgentParticipantRoleType, +) + + +class PermissionsTestCase(TestCase): + def setUp(self): + self.course, _ = create_course("Test Course") + self.course_session = create_course_session( + course=self.course, title="Test Session" + ) + + self.other_course_session = create_course_session( + course=self.course, title="Other Session" + ) + + self.user = create_user("user") + + def test_regionenleiter_has_course_session_document_access(self): + # GIVEN + csg = CourseSessionGroup.objects.create(name="Test Group", course=self.course) + csg.course_session.add(self.course_session) + csg.supervisor.add(self.user) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + some = CourseSessionGroup.objects.filter(course_session=self.course_session.id, supervisor=self.user.id) + print(some) + + # THEN + self.assertTrue(has_access) + + def test_regionenleiter_has_no_course_session_document_access(self): + # GIVEN + csg = CourseSessionGroup.objects.create(name="Test Group", course=self.course) + csg.course_session.add(self.other_course_session) + csg.supervisor.add(self.user) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + some = CourseSessionGroup.objects.filter(course_session=self.course_session.id, supervisor=self.user.id) + print(some) + + # THEN + self.assertFalse(has_access) + + def test_expert_has_course_session_document_access(self): + # GIVEN + _csu = CourseSessionUser.objects.create( + course_session=self.course_session, + user=self.user, + role=CourseSessionUser.Role.EXPERT, + ) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + # THEN + self.assertTrue(has_access) + + def test_expert_has_no_course_session_document_access(self): + # GIVEN + _csu = CourseSessionUser.objects.create( + course_session=self.course_session, + user=self.user, + role=CourseSessionUser.Role.EXPERT, + ) + + # WHEN + has_access = has_course_session_document_access(self.user, self.other_course_session.id) + + # THEN + self.assertFalse(has_access) + + def test_member_has_course_session_document_access(self): + # GIVEN + _csu = CourseSessionUser.objects.create( + course_session=self.course_session, + user=self.user, + role=CourseSessionUser.Role.MEMBER, + ) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + # THEN + self.assertTrue(has_access) + + def test_member_has_no_course_session_document_access(self): + # GIVEN + _csu = CourseSessionUser.objects.create( + course_session=self.course_session, + user=self.user, + role=CourseSessionUser.Role.MEMBER, + ) + + # WHEN + has_access = has_course_session_document_access(self.user, self.other_course_session.id) + + # THEN + self.assertFalse(has_access) + + def test_berufsbildner_has_course_session_document_access(self): + # GIVEN + member = create_user("member") + _csu = CourseSessionUser.objects.create( + course_session=self.course_session, + user=member, + role=CourseSessionUser.Role.MEMBER, + ) + + AgentParticipantRelation.objects.create(agent=self.user, participant=_csu, + role=AgentParticipantRoleType.BERUFSBILDNER.value) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + # THEN + self.assertTrue(has_access) + + def test_berufsbildner_has_no_course_session_document_access(self): + # GIVEN + member = create_user("member") + _csu = CourseSessionUser.objects.create( + course_session=self.other_course_session, + user=member, + role=CourseSessionUser.Role.MEMBER, + ) + + AgentParticipantRelation.objects.create(agent=self.user, participant=_csu, + role=AgentParticipantRoleType.BERUFSBILDNER.value) + + # WHEN + has_access = has_course_session_document_access(self.user, self.course_session.id) + + # THEN + self.assertFalse(has_access) From 60e4de3d9e2bec56350a09751e8b4438512ea010 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 1 Oct 2024 16:08:37 +0200 Subject: [PATCH 2/4] Remove dead code --- server/vbv_lernwelt/course/serializers.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/server/vbv_lernwelt/course/serializers.py b/server/vbv_lernwelt/course/serializers.py index 0ff76b4f..d385287d 100644 --- a/server/vbv_lernwelt/course/serializers.py +++ b/server/vbv_lernwelt/course/serializers.py @@ -86,16 +86,10 @@ class CourseSessionSerializer(serializers.ModelSerializer): course = serializers.SerializerMethodField() actions = serializers.SerializerMethodField() - user_roles = serializers.SerializerMethodField() def get_course(self, obj): return CourseSerializer(obj.course).data - def get_user_roles(self, obj): - if hasattr(obj, "roles"): - return list(obj.roles) - return [] - class Meta: model = CourseSession fields = [ @@ -107,7 +101,6 @@ class CourseSessionSerializer(serializers.ModelSerializer): "start_date", "end_date", "actions", - "user_roles", ] read_only_fields = ["actions"] From 993c9bb536b37d0c3d01f516ee89cab5f8eca485 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Wed, 2 Oct 2024 07:41:15 +0200 Subject: [PATCH 3/4] Hide edit buttons based on role --- .../cockpit/documentPage/DocumentPage.vue | 32 +++++++++-- server/vbv_lernwelt/iam/permissions.py | 14 ++--- .../iam/tests/test_permissions.py | 54 ++++++++++++++----- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/client/src/pages/cockpit/documentPage/DocumentPage.vue b/client/src/pages/cockpit/documentPage/DocumentPage.vue index 249d8902..ad3c1a9d 100644 --- a/client/src/pages/cockpit/documentPage/DocumentPage.vue +++ b/client/src/pages/cockpit/documentPage/DocumentPage.vue @@ -2,7 +2,11 @@ import DocumentListItem from "@/components/circle/DocumentListItem.vue"; import ItDropdownSelect from "@/components/ui/ItDropdownSelect.vue"; import ItModal from "@/components/ui/ItModal.vue"; -import { useCourseData, useCurrentCourseSession } from "@/composables"; +import { + useCourseData, + useCourseSessionDetailQuery, + useCurrentCourseSession, +} from "@/composables"; import { useExpertCockpitPageData } from "@/pages/cockpit/cockpitPage/composables"; import DocumentUploadForm from "@/pages/cockpit/documentPage/DocumentUploadForm.vue"; import { @@ -12,6 +16,7 @@ import { } from "@/services/files"; import { useCourseSessionsStore } from "@/stores/courseSessions"; import { useExpertCockpitStore } from "@/stores/expertCockpit"; +import { useUserStore } from "@/stores/user"; import type { CircleDocument, DocumentUploadData } from "@/types"; import dialog from "@/utils/confirm-dialog"; import { useTranslation } from "i18next-vue"; @@ -21,6 +26,9 @@ import { computed, onMounted, ref, watch } from "vue"; const cockpitStore = useExpertCockpitStore(); const courseSession = useCurrentCourseSession(); const courseSessionsStore = useCourseSessionsStore(); +const courseSessionDetailResult = useCourseSessionDetailQuery(); +const userStore = useUserStore(); + const courseData = useCourseData(courseSession.value?.course.slug); const { t } = useTranslation(); @@ -71,6 +79,17 @@ const circleDocuments = computed(() => { ); }); +const canEditDocuments = computed(() => { + const circleExperts = courseSessionDetailResult.filterCircleExperts( + cockpitStore.currentCircle?.slug || "" + ); + return circleExperts.some( + (expert) => + expert.user_id === userStore.id && + expert.id.indexOf("as-ephemeral-supervisor") === -1 + ); +}); + const deleteDocument = async (doc: CircleDocument) => { const options = { title: t("circlePage.documents.deleteModalTitle"), @@ -134,18 +153,21 @@ async function uploadDocument(data: DocumentUploadData) { @update:model-value="cockpitStore.setCurrentCourseCircleFromEvent" > -
- -
    +
      diff --git a/server/vbv_lernwelt/iam/permissions.py b/server/vbv_lernwelt/iam/permissions.py index 1a5971f9..53ddbcd6 100644 --- a/server/vbv_lernwelt/iam/permissions.py +++ b/server/vbv_lernwelt/iam/permissions.py @@ -53,7 +53,9 @@ def has_course_session_document_access(user, course_session_id: int): course_session_id=course_session_id, user=user ).exists() or is_course_session_berufsbildner(user, course_session_id) - or CourseSessionGroup.objects.filter(course_session=course_session_id, supervisor=user.id).exists() + or CourseSessionGroup.objects.filter( + course_session=course_session_id, supervisor=user.id + ).exists() ) @@ -349,10 +351,10 @@ def can_view_course_completions( str(user.id) == target_user_id or is_course_session_expert(user=user, course_session_id=course_session_id) or is_agent_for_user( - agent=user, - participant_user_id=target_user_id, - course_session_id=course_session_id, - ) + agent=user, + participant_user_id=target_user_id, + course_session_id=course_session_id, + ) ) @@ -383,7 +385,7 @@ def course_session_permissions(user: User, course_session_id: int) -> list[str]: "learning-mentor": has_learning_mentor, "learning-mentor::edit-mentors": has_learning_mentor and is_member, "learning-mentor::guide-members": course_has_learning_mentor - and is_learning_mentor, + and is_learning_mentor, "preview": has_course_session_preview(user, course_session_id), "media-library": ( is_supervisor or is_expert or is_member or is_berufsbildner diff --git a/server/vbv_lernwelt/iam/tests/test_permissions.py b/server/vbv_lernwelt/iam/tests/test_permissions.py index 872fb5e7..2678502e 100644 --- a/server/vbv_lernwelt/iam/tests/test_permissions.py +++ b/server/vbv_lernwelt/iam/tests/test_permissions.py @@ -36,9 +36,13 @@ class PermissionsTestCase(TestCase): csg.supervisor.add(self.user) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) - some = CourseSessionGroup.objects.filter(course_session=self.course_session.id, supervisor=self.user.id) + some = CourseSessionGroup.objects.filter( + course_session=self.course_session.id, supervisor=self.user.id + ) print(some) # THEN @@ -51,9 +55,13 @@ class PermissionsTestCase(TestCase): csg.supervisor.add(self.user) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) - some = CourseSessionGroup.objects.filter(course_session=self.course_session.id, supervisor=self.user.id) + some = CourseSessionGroup.objects.filter( + course_session=self.course_session.id, supervisor=self.user.id + ) print(some) # THEN @@ -68,7 +76,9 @@ class PermissionsTestCase(TestCase): ) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) # THEN self.assertTrue(has_access) @@ -82,7 +92,9 @@ class PermissionsTestCase(TestCase): ) # WHEN - has_access = has_course_session_document_access(self.user, self.other_course_session.id) + has_access = has_course_session_document_access( + self.user, self.other_course_session.id + ) # THEN self.assertFalse(has_access) @@ -96,7 +108,9 @@ class PermissionsTestCase(TestCase): ) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) # THEN self.assertTrue(has_access) @@ -110,7 +124,9 @@ class PermissionsTestCase(TestCase): ) # WHEN - has_access = has_course_session_document_access(self.user, self.other_course_session.id) + has_access = has_course_session_document_access( + self.user, self.other_course_session.id + ) # THEN self.assertFalse(has_access) @@ -124,11 +140,16 @@ class PermissionsTestCase(TestCase): role=CourseSessionUser.Role.MEMBER, ) - AgentParticipantRelation.objects.create(agent=self.user, participant=_csu, - role=AgentParticipantRoleType.BERUFSBILDNER.value) + AgentParticipantRelation.objects.create( + agent=self.user, + participant=_csu, + role=AgentParticipantRoleType.BERUFSBILDNER.value, + ) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) # THEN self.assertTrue(has_access) @@ -142,11 +163,16 @@ class PermissionsTestCase(TestCase): role=CourseSessionUser.Role.MEMBER, ) - AgentParticipantRelation.objects.create(agent=self.user, participant=_csu, - role=AgentParticipantRoleType.BERUFSBILDNER.value) + AgentParticipantRelation.objects.create( + agent=self.user, + participant=_csu, + role=AgentParticipantRoleType.BERUFSBILDNER.value, + ) # WHEN - has_access = has_course_session_document_access(self.user, self.course_session.id) + has_access = has_course_session_document_access( + self.user, self.course_session.id + ) # THEN self.assertFalse(has_access) From f96733d9eaf2f12f1c36dcb4d90b987020b5d36e Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Wed, 2 Oct 2024 14:55:32 +0200 Subject: [PATCH 4/4] Add cypress tests --- .../components/circle/DocumentListItem.vue | 7 +++- .../cockpit/documentPage/DocumentPage.vue | 3 ++ .../circlePage/DocumentSection.vue | 6 ++- cypress/e2e/cockpit/cockpitDocuments.cy.js | 41 +++++++++++++++++++ .../course/creators/test_course.py | 25 +++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 cypress/e2e/cockpit/cockpitDocuments.cy.js diff --git a/client/src/components/circle/DocumentListItem.vue b/client/src/components/circle/DocumentListItem.vue index b11f8327..212fa10a 100644 --- a/client/src/components/circle/DocumentListItem.vue +++ b/client/src/components/circle/DocumentListItem.vue @@ -8,7 +8,12 @@

      {{ subtitle }}

      - + diff --git a/client/src/pages/cockpit/documentPage/DocumentPage.vue b/client/src/pages/cockpit/documentPage/DocumentPage.vue index ad3c1a9d..de48a353 100644 --- a/client/src/pages/cockpit/documentPage/DocumentPage.vue +++ b/client/src/pages/cockpit/documentPage/DocumentPage.vue @@ -83,6 +83,8 @@ const canEditDocuments = computed(() => { const circleExperts = courseSessionDetailResult.filterCircleExperts( cockpitStore.currentCircle?.slug || "" ); + // hack-ish way to check if the user is an expert for the circle + // supervistors are not allowd to edit documents if they are not experts in the circle return circleExperts.some( (expert) => expert.user_id === userStore.id && @@ -157,6 +159,7 @@ async function uploadDocument(data: DocumentUploadData) {
-
    +
      { + beforeEach(() => { + cy.manageCommand("cypress_reset"); + }); + + describe("Cockpit Document list", () => { + it("Trainer sees document mutation buttons", () => { + login("test-trainer1@example.com", "test"); + cy.visit("/course/test-lehrgang/cockpit/documents"); + + cy.get('[data-cy="document-upload-button"]').should("exist"); + cy.get('[data-cy="document-delete-button"]').should("exist"); + }); + + it("Supervisor does not see document mutation buttons", () => { + login("test-supervisor1@example.com", "test"); + cy.visit("/course/test-lehrgang/cockpit/documents"); + + cy.get('[data-cy="document-upload-button"]').should("not.exist"); + cy.get('[data-cy="document-delete-button"]').should("not.exist"); + }); + }); + + describe("Preview", () => { + it("Supervisor sees documents list", () => { + login("test-supervisor1@example.com", "test"); + cy.visit("/course/test-lehrgang/learn/fahrzeug"); + + cy.get('[data-cy="circle-page-documents"]').should("exist"); + }); + + it("Berufsbildner sees document list", () => { + login("test-berufsbildner1@example.com", "test"); + cy.visit("/course/test-lehrgang/learn/fahrzeug"); + + cy.get('[data-cy="circle-page-documents"]').should("exist"); + }); + }); +}); diff --git a/server/vbv_lernwelt/course/creators/test_course.py b/server/vbv_lernwelt/course/creators/test_course.py index 81f28c53..ebdd4f9e 100644 --- a/server/vbv_lernwelt/course/creators/test_course.py +++ b/server/vbv_lernwelt/course/creators/test_course.py @@ -2,6 +2,7 @@ from collections import deque from datetime import datetime from dateutil.relativedelta import MO, TH, TU, WE, relativedelta +from django.core.files.uploadedfile import SimpleUploadedFile from django.utils import timezone from slugify import slugify from wagtail.rich_text import RichText @@ -48,6 +49,7 @@ from vbv_lernwelt.core.utils import safe_deque_popleft from vbv_lernwelt.course.consts import COURSE_TEST_ID from vbv_lernwelt.course.factories import CoursePageFactory from vbv_lernwelt.course.models import ( + CircleDocument, Course, CourseCategory, CourseConfiguration, @@ -63,12 +65,14 @@ from vbv_lernwelt.course_session.models import ( ) from vbv_lernwelt.course_session_group.models import CourseSessionGroup from vbv_lernwelt.feedback.services import update_feedback_response +from vbv_lernwelt.files.models import UploadFile from vbv_lernwelt.learning_mentor.models import AgentParticipantRelation from vbv_lernwelt.learnpath.models import ( Circle, LearningContentAssignment, LearningContentAttendanceCourse, LearningContentEdoniqTest, + LearningSequence, ) from vbv_lernwelt.learnpath.tests.learning_path_factories import ( CircleFactory, @@ -187,6 +191,27 @@ def create_test_course( ) csac.due_date.save() + # create fake doc (will be uploaded to aws) + ls = LearningSequence.objects.get( + title="Vorbereitung", + slug="test-lehrgang-lp-circle-fahrzeug-ls-vorbereitung", + ) + fake_file = SimpleUploadedFile( + "fake_file.txt", b"file_content_here", content_type="text/plain" + ) + pseudo_file = UploadFile.objects.create( + file=fake_file, + original_file_name="Test Dokument", + file_name="Test Dokument", + file_type="txt", + ) + CircleDocument.objects.create( + file=pseudo_file, + name="Test Dokument)", + course_session=cs_bern, + learning_sequence=ls, + ) + if include_vv: csac = CourseSessionAttendanceCourse.objects.create( course_session=cs_bern,