From 79cd70cbd8065ade12e9186978c9c98a8744c207 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 16 Nov 2021 14:10:15 +0100 Subject: [PATCH 1/7] Order instruments by name --- server/basicknowledge/queries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/basicknowledge/queries.py b/server/basicknowledge/queries.py index 38bf75a8..9faf1726 100644 --- a/server/basicknowledge/queries.py +++ b/server/basicknowledge/queries.py @@ -64,4 +64,4 @@ class InstrumentQuery(object): return BasicKnowledge.objects.all().live() def resolve_instrument_types(self, info, **kwargs): - return InstrumentType.objects.filter(instruments__isnull=False).distinct() + return InstrumentType.objects.filter(instruments__isnull=False).order_by('name').distinct() From 338e4cfcfc5bc54be261d8603b1da8ee879f8861 Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Tue, 2 Nov 2021 12:18:05 +0100 Subject: [PATCH 2/7] Add unit tests to check the issues found in the bug bounty report --- server/core/mixins.py | 10 ++++ server/portfolio/models.py | 10 +--- server/portfolio/schema.py | 2 + .../portfolio/tests/test_project_mutations.py | 39 +++++++++++- server/portfolio/tests/test_project_query.py | 21 ++++++- server/rooms/models.py | 3 +- server/rooms/tests/test_new_room_mutation.py | 59 +++++++++++++------ .../rooms/tests/test_room_entry_mutations.py | 49 ++++++++++++++- server/users/models.py | 3 +- 9 files changed, 162 insertions(+), 34 deletions(-) diff --git a/server/core/mixins.py b/server/core/mixins.py index f444085e..4fb87c0b 100644 --- a/server/core/mixins.py +++ b/server/core/mixins.py @@ -1,4 +1,5 @@ import graphene +from graphql_relay import to_global_id class HiddenForMixin: @@ -18,3 +19,12 @@ class VisibleForMixin: class HiddenAndVisibleForMixin(HiddenForMixin, VisibleForMixin): pass + + +class GraphqlNodeMixin: + def default_node_name(self): + return f'{self.__class__.__name__}Node' + + @property + def graphql_id(self): + return to_global_id(self.default_node_name(), self.id) diff --git a/server/portfolio/models.py b/server/portfolio/models.py index 7cae2516..17ad7177 100644 --- a/server/portfolio/models.py +++ b/server/portfolio/models.py @@ -3,17 +3,9 @@ from django.db import models from django_extensions.db.models import TitleSlugDescriptionModel from graphql_relay import to_global_id +from core.mixins import GraphqlNodeMixin from users.models import User - -class GraphqlNodeMixin: - def default_node_name(self): - return f'{self.__class__.__name__}Node' - - @property - def graphql_id(self): - return to_global_id(self.default_node_name(), self.id) - class Project(TitleSlugDescriptionModel, GraphqlNodeMixin): objectives = models.TextField(blank=True) appearance = models.CharField(blank=True, null=False, max_length=255) diff --git a/server/portfolio/schema.py b/server/portfolio/schema.py index c647b510..652f86fe 100644 --- a/server/portfolio/schema.py +++ b/server/portfolio/schema.py @@ -6,6 +6,7 @@ from graphene_django import DjangoObjectType from api.utils import get_by_id_or_slug from portfolio.models import Project, ProjectEntry from users.models import Role, UserRole, User +from users.schema import PublicUserNode class ProjectEntryNode(DjangoObjectType): @@ -19,6 +20,7 @@ class ProjectNode(DjangoObjectType): pk = graphene.Int() entries_count = graphene.Int() entries = graphene.List(ProjectEntryNode) + owner = graphene.Field(PublicUserNode) class Meta: model = Project diff --git a/server/portfolio/tests/test_project_mutations.py b/server/portfolio/tests/test_project_mutations.py index ddcd3638..ce583191 100644 --- a/server/portfolio/tests/test_project_mutations.py +++ b/server/portfolio/tests/test_project_mutations.py @@ -3,6 +3,7 @@ from graphene.test import Client from graphql_relay import to_global_id from api.schema import schema +from core.tests.base_test import SkillboxTestCase from portfolio.factories import ProjectFactory from users.factories import SchoolClassFactory from users.models import User @@ -10,7 +11,7 @@ from users.services import create_users from api.test_utils import create_client, DefaultUserTestCase from portfolio.models import Project -class ProjectQuery(TestCase): +class ProjectQuery(SkillboxTestCase): def setUp(self): create_users() self.teacher = User.objects.get(username='teacher') @@ -49,7 +50,6 @@ class ProjectQuery(TestCase): self.assertEqual(Project.objects.count(), 0) def test_should_not_be_able_to_delete_other_projects(self): - self.assertEqual(Project.objects.count(), 1) request = RequestFactory().get('/') request.user = self.student2 @@ -58,6 +58,41 @@ class ProjectQuery(TestCase): result = self.client.execute(self.mutation, variables=self.variables) self.assertEqual(result.get('errors')[0]['message'], 'Permission denied: Incorrect project') + def test_should_not_be_able_to_edit_other_projects(self): + self.assertEqual(Project.objects.count(), 1) + request = RequestFactory().get('/') + request.user = self.student2 + self.client = Client(schema=schema, context_value=request) + mutation = ''' +mutation UpdateProjectMutation($input: UpdateProjectInput!){ + updateProject(input: $input) { + project { + id + } + } +} +''' + # project: + # { + # title: String + # description: String + # objectives: String + # appearance: String + # id: ID! + # final: Boolean + # } + input = { + 'project': { + 'id': self.project1.graphql_id, + 'title': 'BAD! THIS IS BAD!' + } + } + result = self.get_client(self.student2).get_result(mutation, variables={ + 'input': input + }) + self.assertIsNotNone(result.errors) + self.assertTrue('Permission' in result.errors) + class ProjectMutationsTestCase(DefaultUserTestCase): def test_add_project(self): diff --git a/server/portfolio/tests/test_project_query.py b/server/portfolio/tests/test_project_query.py index b49c6452..b26e19d0 100644 --- a/server/portfolio/tests/test_project_query.py +++ b/server/portfolio/tests/test_project_query.py @@ -1,3 +1,5 @@ +from graphql_relay import to_global_id + from core.tests.base_test import SkillboxTestCase from portfolio.factories import ProjectFactory from portfolio.models import Project @@ -13,7 +15,7 @@ query ProjectQuery($id: ID!) { """ -class ProjectQueryTestCaswe(SkillboxTestCase): +class ProjectQueryTestCase(SkillboxTestCase): def _test_direct_project_access(self, user: User, should_have_access: bool): result = self.get_client(user).get_result(project_query, variables={ 'id': self.project1.graphql_id @@ -30,6 +32,7 @@ class ProjectQueryTestCaswe(SkillboxTestCase): school_class2 = SchoolClassFactory(users=[self.teacher2, self.student2]) self.project1 = ProjectFactory(student=self.student1) + self.project1_id = to_global_id('ProjectNode', self.project1.id) self.query = ''' query ProjectsQuery { projects { @@ -113,3 +116,19 @@ class ProjectQueryTestCaswe(SkillboxTestCase): self._test_direct_project_access(self.teacher2, False) # non-owner can't access project self._test_direct_project_access(self.student2, False) + + def test_project_owner(self): + query = """ +query ProjectQuery($id: ID!) { + project(id: $id) { + id + owner { + email + } + } +} + """ + result = self.get_client(self.student1).get_result(query, variables={ + 'id': self.project1.graphql_id + }) + self.assertIsNotNone(result.errors) diff --git a/server/rooms/models.py b/server/rooms/models.py index d8d7d0e8..68f8c902 100644 --- a/server/rooms/models.py +++ b/server/rooms/models.py @@ -5,10 +5,11 @@ from wagtail.core.fields import StreamField from books.blocks import DocumentBlock, ImageUrlBlock, LinkBlock, VideoBlock from books.models import TextBlock +from core.mixins import GraphqlNodeMixin from users.models import SchoolClass -class Room(TitleSlugDescriptionModel): +class Room(TitleSlugDescriptionModel, GraphqlNodeMixin): class Meta: verbose_name = 'Raum' verbose_name_plural = 'Räume' diff --git a/server/rooms/tests/test_new_room_mutation.py b/server/rooms/tests/test_new_room_mutation.py index 8dcfb9c1..bba8da28 100644 --- a/server/rooms/tests/test_new_room_mutation.py +++ b/server/rooms/tests/test_new_room_mutation.py @@ -2,6 +2,8 @@ from graphql_relay import from_global_id from core.tests.base_test import SkillboxTestCase from core.tests.helpers import GQLResult +from rooms.models import Room + class GQLRoom: def __init__(self, room_data): @@ -30,29 +32,30 @@ class AddRoomResult: class NewRoomMutationTestCase(SkillboxTestCase): def setUp(self) -> None: self.createDefault() + self.mutation = """ + mutation AddRoom($input: AddRoomInput!){ + addRoom(input: $input) { + room { + id + slug + title + entryCount + appearance + description + schoolClass { + id + name + } + } + } + } + """ def test_create_new_room(self): - mutation = """ -mutation AddRoom($input: AddRoomInput!){ - addRoom(input: $input) { - room { - id - slug - title - entryCount - appearance - description - schoolClass { - id - name - } - } - } -} -""" + self.assertEqual(Room.objects.count(), 0) title = 'some title' appearance='blue' - res = self.get_client().execute(mutation, variables={ + res = self.get_client().execute(self.mutation, variables={ 'input': { 'room': { 'title': title, @@ -68,5 +71,23 @@ mutation AddRoom($input: AddRoomInput!){ self.assertEqual(room.appearance, appearance) self.assertIsNone(room.description) self.assertEqual(int(from_global_id(room.school_class.get('id'))[1]), self.teacher.selected_class.id) + self.assertEqual(Room.objects.count(), 1) + def test_create_new_room_for_other_school_class(self): + self.assertEqual(Room.objects.count(), 0) + result = self.get_client(self.teacher2).get_result(self.mutation, variables={ + 'input': { + 'room': { + 'title': 'BIG NO NO!', + # description + 'schoolClass': { + 'id': self.school_class.graphql_id + }, + 'appearance': 'red' + } + } + }) + self.assertIsNotNone(result.errors) + self.assertTrue('Permission' in result.errors) + self.assertEqual(Room.objects.count(), 0) diff --git a/server/rooms/tests/test_room_entry_mutations.py b/server/rooms/tests/test_room_entry_mutations.py index d20f5c0a..e7b9f426 100644 --- a/server/rooms/tests/test_room_entry_mutations.py +++ b/server/rooms/tests/test_room_entry_mutations.py @@ -4,12 +4,13 @@ from graphql_relay import to_global_id from api.schema import schema from core.factories import UserFactory +from core.tests.base_test import SkillboxTestCase from rooms.factories import RoomEntryFactory, RoomFactory from rooms.models import RoomEntry from users.factories import SchoolClassFactory -class RoomEntryMutationsTestCase(TestCase): +class RoomEntryMutationsTestCase(SkillboxTestCase): def setUp(self): self.user = UserFactory(username='aschi') self.another_user = UserFactory(username='pesche') @@ -17,6 +18,8 @@ class RoomEntryMutationsTestCase(TestCase): s = SchoolClassFactory(users=[self.user, self.another_user]) s2 = SchoolClassFactory(users=[self.yet_another_user]) self.room_entry = RoomEntryFactory(author=self.user, room=RoomFactory(school_class=s)) + self.room = self.room_entry.room + self.first_school_class = s request = RequestFactory().get('/') request.user = self.user @@ -135,3 +138,47 @@ class RoomEntryMutationsTestCase(TestCase): entry = RoomEntry.objects.get(pk=self.room_entry.pk) self.assertIsNotNone(result.get('errors')) self.assertEqual(entry.title, self.room_entry.title) + + def test_add_room_entry_not_owner_from_other_class(self): + self.assertEqual(RoomEntry.objects.count(), 1) + mutation = """ +fragment RoomEntryParts on RoomEntryNode { + id + slug + title + contents + author { + id + firstName + lastName + avatarUrl + } +} + + +mutation AddRoomEntry($input: AddRoomEntryInput!){ + addRoomEntry(input: $input) { + roomEntry { + ...RoomEntryParts + } + errors + + } +} + """ + # input: + # title = graphene.String(required=True) + # contents = graphene.List(ContentElementInput) + # room = graphene.ID(required=True) + room_entry = { + 'title': 'Bad Actor!', + 'room': self.room.graphql_id + } + + result = self.get_client(self.yet_another_user).get_result(mutation, variables={ + 'input': { + 'roomEntry': room_entry + } + }) + self.assertIsNotNone(result.errors) + self.assertTrue('Permission' in result.errors) diff --git a/server/users/models.py b/server/users/models.py index 50a748ba..e87b88dd 100644 --- a/server/users/models.py +++ b/server/users/models.py @@ -14,6 +14,7 @@ from django.utils.functional import cached_property from django.utils.timezone import is_aware, make_aware from django.utils.translation import ugettext_lazy as _ +from core.mixins import GraphqlNodeMixin from users.licenses import MYSKILLBOX_LICENSES from users.managers import LicenseManager, RoleManager, UserManager, UserRoleManager @@ -176,7 +177,7 @@ class Team(GroupWithCode): return self.name -class SchoolClass(GroupWithCode): +class SchoolClass(GroupWithCode, GraphqlNodeMixin): users = models.ManyToManyField(get_user_model(), related_name='school_classes', blank=True, through='users.SchoolClassMember') From ee05ee79ba061ccf841cd8d81eecf665dcfd1e50 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 9 Nov 2021 09:09:27 +0100 Subject: [PATCH 3/7] Fix PHEP-7 (user can add room entry at other school) --- server/rooms/mutations.py | 35 ++++++++++++------- .../rooms/tests/test_room_entry_mutations.py | 3 +- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/server/rooms/mutations.py b/server/rooms/mutations.py index 91401fc5..6c4f9d37 100644 --- a/server/rooms/mutations.py +++ b/server/rooms/mutations.py @@ -85,12 +85,26 @@ class MutateRoomEntry(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): room_entry_data = kwargs.get('room_entry') + room = None if room_entry_data.get('room') is not None: - room_entry_data['room'] = get_object(Room, room_entry_data.get('room')).id + room = get_object(Room, room_entry_data.get('room')) + room_entry_data['room'] = room.id if room_entry_data.get('id') is not None: - # update path + serializer = cls.update_path(info, room_entry_data) + else: + serializer = cls.add_path(info, room_entry_data, room) + + if serializer.is_valid(): + serializer.save() + + return cls(room_entry=serializer.instance) + + return cls(room_entry=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + + @classmethod + def update_path(cls, info, room_entry_data): instance = get_object(RoomEntry, room_entry_data.get('id')) if not instance.room.school_class.is_user_in_schoolclass(info.context.user): @@ -99,18 +113,16 @@ class MutateRoomEntry(relay.ClientIDMutation): if instance.author.pk != info.context.user.pk: raise Exception('You are not the author') - serializer = RoomEntrySerializer(instance, data=room_entry_data, partial=True) - else: - # add path - room_entry_data['author'] = info.context.user.pk - serializer = RoomEntrySerializer(data=room_entry_data) + return RoomEntrySerializer(instance, data=room_entry_data, partial=True) - if serializer.is_valid(): - serializer.save() + @classmethod + def add_path(cls, info, room_entry_data, room): - return cls(room_entry=serializer.instance) + if not room or not room.school_class.is_user_in_schoolclass(info.context.user): + raise PermissionDenied('You are in the wrong class') - return cls(room_entry=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + room_entry_data['author'] = info.context.user.pk + return RoomEntrySerializer(data=room_entry_data) class AddRoomEntry(MutateRoomEntry): @@ -165,7 +177,6 @@ class UpdateRoomVisibility(relay.ClientIDMutation): return cls(success=True, room=room) - class AddComment(relay.ClientIDMutation): class Input: comment = graphene.String(required=True) diff --git a/server/rooms/tests/test_room_entry_mutations.py b/server/rooms/tests/test_room_entry_mutations.py index e7b9f426..76128b68 100644 --- a/server/rooms/tests/test_room_entry_mutations.py +++ b/server/rooms/tests/test_room_entry_mutations.py @@ -181,4 +181,5 @@ mutation AddRoomEntry($input: AddRoomEntryInput!){ } }) self.assertIsNotNone(result.errors) - self.assertTrue('Permission' in result.errors) + self.assertTrue('message' in result.errors[0]) + self.assertEqual(result.errors[0]['message'], 'You are in the wrong class') From 2a6993cad8552abb1429d25db671b3eb1dcd4fef Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 9 Nov 2021 13:53:53 +0100 Subject: [PATCH 4/7] Fix PHEP-3 (edit project as other user) --- server/portfolio/models.py | 3 +- server/portfolio/mutations.py | 47 ++++++++++++------- server/portfolio/schema.py | 4 +- .../portfolio/tests/test_project_mutations.py | 3 +- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/server/portfolio/models.py b/server/portfolio/models.py index 17ad7177..54565588 100644 --- a/server/portfolio/models.py +++ b/server/portfolio/models.py @@ -1,11 +1,11 @@ from django.contrib.auth import get_user_model from django.db import models from django_extensions.db.models import TitleSlugDescriptionModel -from graphql_relay import to_global_id from core.mixins import GraphqlNodeMixin from users.models import User + class Project(TitleSlugDescriptionModel, GraphqlNodeMixin): objectives = models.TextField(blank=True) appearance = models.CharField(blank=True, null=False, max_length=255) @@ -21,6 +21,7 @@ class Project(TitleSlugDescriptionModel, GraphqlNodeMixin): self.final and self.student.get_teacher().id == user.id ) + class ProjectEntry(models.Model): activity = models.TextField(blank=True) reflection = models.TextField(blank=True) diff --git a/server/portfolio/mutations.py b/server/portfolio/mutations.py index 66d29fba..3c46b364 100644 --- a/server/portfolio/mutations.py +++ b/server/portfolio/mutations.py @@ -24,23 +24,11 @@ class MutateProject(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): - data = kwargs.get('project') - data['student'] = info.context.user.id + raise Exception('Must be subclassed') - if data.get('id') is not None: - entity = get_object(Project, data['id']) - serializer = ProjectSerializer(entity, data=data) - else: - serializer = ProjectSerializer(data=data) - if serializer.is_valid(): - serializer.save() - props = { - 'project': serializer.instance, - 'errors': None - } - return cls(**props) - - return cls(errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + @classmethod + def create_error_response(cls, serializer): + return cls(room=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) class AddProject(MutateProject): @@ -58,13 +46,37 @@ class AddProject(MutateProject): serializer.save() return cls(project=serializer.instance) - return cls(room=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + return cls.create_error_response(serializer) class UpdateProject(MutateProject): class Input: project = graphene.Argument(UpdateProjectArgument) + @classmethod + def mutate_and_get_payload(cls, root, info, **kwargs): + data = kwargs.get('project') + + cls.user_is_owner(data, info.context.user) + data['student'] = info.context.user.id + + serializer = ProjectSerializer(data=data) + if serializer.is_valid(): + serializer.save() + props = { + 'project': serializer.instance, + 'errors': None + } + return cls(**props) + + return cls.create_error_response(serializer) + + @classmethod + def user_is_owner(cls, data, user): + project = get_object(Project, data['id']) + if not project or not project.student == user.id: + raise PermissionDenied('not allowed') + class MutateProjectEntry(relay.ClientIDMutation): errors = graphene.List(graphene.String) @@ -73,7 +85,6 @@ class MutateProjectEntry(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): data = kwargs.get('project_entry') - project = None if data.get('project') is not None: project = get_object(Project, data.get('project')) diff --git a/server/portfolio/schema.py b/server/portfolio/schema.py index 652f86fe..a578e41f 100644 --- a/server/portfolio/schema.py +++ b/server/portfolio/schema.py @@ -54,8 +54,8 @@ class PortfolioQuery(object): return Project.objects.filter(student=user) def resolve_project(self, info, **kwargs): - user = info.context.user # type: User - project = get_by_id_or_slug(Project, **kwargs) #type: Project + user = info.context.user # type: User + project = get_by_id_or_slug(Project, **kwargs) # type: Project if project.is_viewable_by(user): return project diff --git a/server/portfolio/tests/test_project_mutations.py b/server/portfolio/tests/test_project_mutations.py index ce583191..ce370a60 100644 --- a/server/portfolio/tests/test_project_mutations.py +++ b/server/portfolio/tests/test_project_mutations.py @@ -91,7 +91,8 @@ mutation UpdateProjectMutation($input: UpdateProjectInput!){ 'input': input }) self.assertIsNotNone(result.errors) - self.assertTrue('Permission' in result.errors) + self.assertTrue('message' in result.errors[0]) + self.assertEqual(result.errors[0]['message'], 'not allowed') class ProjectMutationsTestCase(DefaultUserTestCase): From c3274b4a65a88fa0e29fc67e5101856184ef9b13 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 9 Nov 2021 16:36:39 +0100 Subject: [PATCH 5/7] Fix test --- server/portfolio/mutations.py | 2 ++ server/rooms/tests/test_new_room_mutation.py | 10 ++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/portfolio/mutations.py b/server/portfolio/mutations.py index 3c46b364..b363e88d 100644 --- a/server/portfolio/mutations.py +++ b/server/portfolio/mutations.py @@ -8,6 +8,7 @@ from portfolio.inputs import AddProjectArgument, AddProjectEntryArgument, Update from portfolio.models import Project, ProjectEntry from portfolio.schema import ProjectEntryNode, ProjectNode from portfolio.serializers import ProjectEntrySerializer, ProjectSerializer +from users.models import UserSetting def check_owner(user, project): @@ -40,6 +41,7 @@ class AddProject(MutateProject): def mutate_and_get_payload(cls, root, info, **kwargs): data = kwargs.get('project') data['student'] = info.context.user.id + data['school_class'] = info.context.user.selected_class.id serializer = ProjectSerializer(data=data) if serializer.is_valid(): diff --git a/server/rooms/tests/test_new_room_mutation.py b/server/rooms/tests/test_new_room_mutation.py index bba8da28..7ed22dba 100644 --- a/server/rooms/tests/test_new_room_mutation.py +++ b/server/rooms/tests/test_new_room_mutation.py @@ -54,7 +54,7 @@ class NewRoomMutationTestCase(SkillboxTestCase): def test_create_new_room(self): self.assertEqual(Room.objects.count(), 0) title = 'some title' - appearance='blue' + appearance = 'blue' res = self.get_client().execute(self.mutation, variables={ 'input': { 'room': { @@ -87,7 +87,9 @@ class NewRoomMutationTestCase(SkillboxTestCase): } } }) - self.assertIsNotNone(result.errors) - self.assertTrue('Permission' in result.errors) - self.assertEqual(Room.objects.count(), 0) + + self.assertEqual(Room.objects.count(), 1) + + room = Room.objects.all()[0] + self.assertEqual(room.school_class.id, self.teacher2.selected_class.id) From a3fe315df3ab5b3f9154434d12dd154dd98c55c8 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Wed, 10 Nov 2021 10:51:55 +0100 Subject: [PATCH 6/7] Refactor mutations --- server/rooms/mutations.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/rooms/mutations.py b/server/rooms/mutations.py index 6c4f9d37..fa25f783 100644 --- a/server/rooms/mutations.py +++ b/server/rooms/mutations.py @@ -39,7 +39,7 @@ class AddRoom(MutateRoom): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): user = info.context.user - if not user.has_perm('users.can_manage_school_class_content'): + if not user.is_teacher(): return cls(room=None, errors=['not allowed']) room_data = kwargs.get('room') @@ -92,9 +92,9 @@ class MutateRoomEntry(relay.ClientIDMutation): room_entry_data['room'] = room.id if room_entry_data.get('id') is not None: - serializer = cls.update_path(info, room_entry_data) + serializer = cls.update_room_entry(info, room_entry_data) else: - serializer = cls.add_path(info, room_entry_data, room) + serializer = cls.add_room_entry(info, room_entry_data, room) if serializer.is_valid(): serializer.save() @@ -104,7 +104,7 @@ class MutateRoomEntry(relay.ClientIDMutation): return cls(room_entry=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) @classmethod - def update_path(cls, info, room_entry_data): + def update_room_entry(cls, info, room_entry_data): instance = get_object(RoomEntry, room_entry_data.get('id')) if not instance.room.school_class.is_user_in_schoolclass(info.context.user): @@ -116,7 +116,7 @@ class MutateRoomEntry(relay.ClientIDMutation): return RoomEntrySerializer(instance, data=room_entry_data, partial=True) @classmethod - def add_path(cls, info, room_entry_data, room): + def add_room_entry(cls, info, room_entry_data, room): if not room or not room.school_class.is_user_in_schoolclass(info.context.user): raise PermissionDenied('You are in the wrong class') From b9a8b3ff383142d0dbe0927a7e89cf0b2918e247 Mon Sep 17 00:00:00 2001 From: Christian Cueni Date: Tue, 16 Nov 2021 15:36:26 +0100 Subject: [PATCH 7/7] Fix bug, add happy path test --- server/portfolio/mutations.py | 5 ++-- .../portfolio/tests/test_project_mutations.py | 26 +++++++++++++++++++ server/portfolio/tests/test_project_query.py | 6 ++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/server/portfolio/mutations.py b/server/portfolio/mutations.py index b363e88d..8ee1cdfd 100644 --- a/server/portfolio/mutations.py +++ b/server/portfolio/mutations.py @@ -62,7 +62,8 @@ class UpdateProject(MutateProject): cls.user_is_owner(data, info.context.user) data['student'] = info.context.user.id - serializer = ProjectSerializer(data=data) + entity = get_object(Project, data['id']) + serializer = ProjectSerializer(entity, data=data) if serializer.is_valid(): serializer.save() props = { @@ -76,7 +77,7 @@ class UpdateProject(MutateProject): @classmethod def user_is_owner(cls, data, user): project = get_object(Project, data['id']) - if not project or not project.student == user.id: + if not project or not project.student == user: raise PermissionDenied('not allowed') diff --git a/server/portfolio/tests/test_project_mutations.py b/server/portfolio/tests/test_project_mutations.py index ce370a60..7008a521 100644 --- a/server/portfolio/tests/test_project_mutations.py +++ b/server/portfolio/tests/test_project_mutations.py @@ -94,6 +94,32 @@ mutation UpdateProjectMutation($input: UpdateProjectInput!){ self.assertTrue('message' in result.errors[0]) self.assertEqual(result.errors[0]['message'], 'not allowed') + def test_owner_can_edit(self): + self.assertEqual(Project.objects.count(), 1) + request = RequestFactory().get('/') + request.user = self.student + self.client = Client(schema=schema, context_value=request) + mutation = ''' +mutation UpdateProjectMutation($input: UpdateProjectInput!){ + updateProject(input: $input) { + project { + id + } + } +} +''' + + input = { + 'project': { + 'id': self.project1.graphql_id, + 'title': 'Good! THIS IS good!' + } + } + result = self.get_client(self.student).get_result(mutation, variables={ + 'input': input + }) + self.assertIsNone(result.errors) + class ProjectMutationsTestCase(DefaultUserTestCase): def test_add_project(self): diff --git a/server/portfolio/tests/test_project_query.py b/server/portfolio/tests/test_project_query.py index b26e19d0..9c66de0c 100644 --- a/server/portfolio/tests/test_project_query.py +++ b/server/portfolio/tests/test_project_query.py @@ -117,12 +117,12 @@ class ProjectQueryTestCase(SkillboxTestCase): # non-owner can't access project self._test_direct_project_access(self.student2, False) - def test_project_owner(self): + def test_project_owner_can_view(self): query = """ query ProjectQuery($id: ID!) { project(id: $id) { id - owner { + student { email } } @@ -131,4 +131,4 @@ query ProjectQuery($id: ID!) { result = self.get_client(self.student1).get_result(query, variables={ 'id': self.project1.graphql_id }) - self.assertIsNotNone(result.errors) + self.assertEqual(result.data['project']['student']['email'], self.student1.email)