Merge branch 'hotfix/bug-bounty' into develop

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()

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)

server/portfolio/models.py

@@ -1,19 +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 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)
@@ -29,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)

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):
@@ -24,23 +25,11 @@ class MutateProject(relay.ClientIDMutation):
 
     @classmethod
     def mutate_and_get_payload(cls, root, info, **kwargs):
-        data = kwargs.get('project')
+        raise Exception('Must be subclassed')
-        data['student'] = info.context.user.id
 
-        if data.get('id') is not None:
+    @classmethod
-            entity = get_object(Project, data['id'])
+    def create_error_response(cls, serializer):
-            serializer = ProjectSerializer(entity, data=data)
+        return cls(room=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()])
-        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()])
 
 
 class AddProject(MutateProject):
@@ -52,19 +41,45 @@ 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():
             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
+
+        entity = get_object(Project, data['id'])
+        serializer = ProjectSerializer(entity, 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:
+            raise PermissionDenied('not allowed')
+
 
 class MutateProjectEntry(relay.ClientIDMutation):
     errors = graphene.List(graphene.String)
@@ -73,7 +88,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'))

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
@@ -52,8 +54,8 @@ class PortfolioQuery(object):
         return Project.objects.filter(student=user)
 
     def resolve_project(self, info, **kwargs):
-        user = info.context.user # type: User
+        user = info.context.user  # type: User
-        project = get_by_id_or_slug(Project, **kwargs) #type: Project
+        project = get_by_id_or_slug(Project, **kwargs)  # type: Project
 
         if project.is_viewable_by(user):
             return project

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,68 @@ 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('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):

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_can_view(self):
+        query = """
+query ProjectQuery($id: ID!) {
+  project(id: $id) {
+    id
+    student {
+        email
+    }
+  }
+}          
+        """
+        result = self.get_client(self.student1).get_result(query, variables={
+            'id': self.project1.graphql_id
+        })
+        self.assertEqual(result.data['project']['student']['email'], self.student1.email)

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'

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')
@@ -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_room_entry(info, room_entry_data)
+        else:
+            serializer = cls.add_room_entry(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_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):
@@ -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)
+            return 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)
 
-        if serializer.is_valid():
+    @classmethod
-            serializer.save()
+    def add_room_entry(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)

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 = """
+        self.assertEqual(Room.objects.count(), 0)
-mutation AddRoom($input: AddRoomInput!){
-  addRoom(input: $input) {
-    room {
-      id
-      slug
-      title
-      entryCount
-      appearance
-      description
-      schoolClass {
-        id
-        name
-      }
-    }
-  }
-}
-"""
         title = 'some title'
-        appearance='blue'
+        appearance = 'blue'
-        res = self.get_client().execute(mutation, variables={
+        res = self.get_client().execute(self.mutation, variables={
             'input': {
                 'room': {
                     'title': title,
@@ -68,5 +71,25 @@ 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.assertEqual(Room.objects.count(), 1)
+
+        room = Room.objects.all()[0]
+        self.assertEqual(room.school_class.id, self.teacher2.selected_class.id)
 

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,48 @@ 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('message' in result.errors[0])
+        self.assertEqual(result.errors[0]['message'], 'You are in the wrong class')

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')