From 6dc598800df7258d904162f1160f40bfb674dc45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Fri, 9 Dec 2016 18:00:45 +0100 Subject: [PATCH] Fixed state flag required permission to see. --- openslides/motions/access_permissions.py | 32 ++++++----- openslides/motions/models.py | 57 +------------------- openslides/motions/serializers.py | 12 +++++ openslides/motions/static/js/motions/base.js | 15 +++--- openslides/motions/views.py | 18 ++++--- tests/integration/motions/test_viewset.py | 31 +++++++++++ tests/old/motions/test_models.py | 2 - 7 files changed, 82 insertions(+), 85 deletions(-) diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 333bd4c06..59719bbf9 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -25,20 +25,28 @@ class MotionAccessPermissions(BaseAccessPermissions): def get_restricted_data(self, full_data, user): """ Returns the restricted serialized data for the instance prepared for - the user. Removes non public comment fields for some unauthorized - users. + the user. Removes motion if the user has not the permission to see + the motion in this state. Removes non public comment fields for + some unauthorized users. """ - if user.has_perm('motions.can_see_and_manage_comments') or not full_data.get('comments'): - data = full_data + required_permission_to_see = full_data.get('state_required_permission_to_see') + if (not required_permission_to_see or + user.has_perm(required_permission_to_see) or + user.has_perm('motions.can_manage') or + user.pk in full_data.get('submitters_id', [])): + if user.has_perm('motions.can_see_and_manage_comments') or not full_data.get('comments'): + data = full_data + else: + data = deepcopy(full_data) + for i, field in enumerate(config['motions_comments']): + if not field.get('public'): + try: + data['comments'][i] = None + except IndexError: + # No data in range. Just do nothing. + pass else: - data = deepcopy(full_data) - for i, field in enumerate(config['motions_comments']): - if not field.get('public'): - try: - data['comments'][i] = None - except IndexError: - # No data in range. Just do nothing. - pass + data = None return data def get_projector_data(self, full_data): diff --git a/openslides/motions/models.py b/openslides/motions/models.py index c731325ba..8018f6564 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -42,7 +42,7 @@ class MotionManager(models.Manager): join and prefetch all related models. """ return (self.get_queryset() - .select_related('active_version') + .select_related('active_version', 'state') .prefetch_related( 'versions', 'agenda_items', @@ -604,61 +604,6 @@ class Motion(RESTModelMixin, models.Model): """ return self.agenda_item.pk - def get_allowed_actions(self, person): - """ - Return a dictonary with all allowed actions for a specific person. - - The dictonary contains the following actions. - - * see - * update / edit - * delete - * create_poll - * support - * unsupport - * change_state - * reset_state - * change_recommendation - - NOTE: If you update this function please also update the - 'isAllowed' function on client side in motions/site.js. - """ - # TODO: Remove this method and implement these things in the views. - actions = { - 'see': (person.has_perm('motions.can_see') and - (not self.state.required_permission_to_see or - person.has_perm(self.state.required_permission_to_see) or - self.is_submitter(person))), - - 'update': (person.has_perm('motions.can_manage') or - (self.is_submitter(person) and - self.state.allow_submitter_edit)), - - 'delete': person.has_perm('motions.can_manage'), - - 'create_poll': (person.has_perm('motions.can_manage') and - self.state.allow_create_poll), - - 'support': (self.state.allow_support and - config['motions_min_supporters'] > 0 and - not self.is_submitter(person) and - not self.is_supporter(person)), - - 'unsupport': (self.state.allow_support and - self.is_supporter(person)), - - 'change_state': person.has_perm('motions.can_manage'), - - 'reset_state': person.has_perm('motions.can_manage'), - - 'change_recommendation': person.has_perm('motions.can_manage'), - - } - - actions['edit'] = actions['update'] - - return actions - def write_log(self, message_list, person=None, skip_autoupdate=False): """ Write a log message. diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 61b081eac..4d49c65a1 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -267,6 +267,7 @@ class MotionSerializer(ModelSerializer): log_messages = MotionLogSerializer(many=True, read_only=True) polls = MotionPollSerializer(many=True, read_only=True) reason = CharField(allow_blank=True, required=False, write_only=True) + state_required_permission_to_see = SerializerMethodField() text = CharField(write_only=True) title = CharField(max_length=255, write_only=True) versions = MotionVersionSerializer(many=True, read_only=True) @@ -294,6 +295,7 @@ class MotionSerializer(ModelSerializer): 'supporters', 'comments', 'state', + 'state_required_permission_to_see', 'workflow_id', 'recommendation', 'tags', @@ -366,3 +368,13 @@ class MotionSerializer(ModelSerializer): attr.add(*validated_data[key]) return motion + + def get_state_required_permission_to_see(self, motion): + """ + Returns the permission (as string) that is required for non + managers that are not submitters to see this motion in this state. + + Hint: Most states have and empty string here so this restriction is + disabled. + """ + return motion.state.required_permission_to_see diff --git a/openslides/motions/static/js/motions/base.js b/openslides/motions/static/js/motions/base.js index b50bd592e..4677e9df5 100644 --- a/openslides/motions/static/js/motions/base.js +++ b/openslides/motions/static/js/motions/base.js @@ -418,8 +418,8 @@ angular.module('OpenSlidesApp.motions', [ * - reset_state * - change_recommendation * - * NOTE: If you update this function please also update the - * 'get_allowed_actions' function on server side in motions/models.py. + * NOTE: If you update this function please think about + * server permissions, see motions/views.py. */ switch (action) { case 'see': @@ -435,7 +435,7 @@ angular.module('OpenSlidesApp.motions', [ return ( operator.hasPerms('motions.can_manage') || ( - ($.inArray(operator.user, this.submitters) != -1) && + (_.indexOf(this.submitters, operator.user) !== -1) && this.state.allow_submitter_edit ) ); @@ -453,14 +453,11 @@ angular.module('OpenSlidesApp.motions', [ operator.hasPerms('motions.can_support') && this.state.allow_support && Config.get('motions_min_supporters').value > 0 && - ($.inArray(operator.user, this.submitters) == -1) && - ($.inArray(operator.user, this.supporters) == -1) + (_.indexOf(this.submitters, operator.user) === -1) && + (_.indexOf(this.supporters, operator.user) === -1) ); case 'unsupport': - return ( - this.state.allow_support && - ($.inArray(operator.user, this.supporters) != -1) - ); + return this.state.allow_support && _.indexOf(this.supporters, operator.user) !== -1; case 'change_state': return operator.hasPerms('motions.can_manage'); case 'reset_state': diff --git a/openslides/motions/views.py b/openslides/motions/views.py index f5e26375e..29b33ade1 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -116,14 +116,16 @@ class MotionViewSet(ModelViewSet): Checks also whether the requesting user can update the motion. He needs at least the permissions 'motions.can_see' (see - self.check_view_permissions()). Also the instance method - get_allowed_actions() is evaluated. + self.check_view_permissions()). Also check manage permission or + submitter and state. """ # Get motion. motion = self.get_object() # Check permissions. - if not motion.get_allowed_actions(request.user)['update']: + if (not request.user.has_perm('motions.can_manage') and + not (motion.is_submitter(request.user) and + motion.state.allow_submitter_edit)): self.permission_denied(request) # Check permission to send only some data. @@ -215,12 +217,14 @@ class MotionViewSet(ModelViewSet): """ # Retrieve motion and allowed actions. motion = self.get_object() - allowed_actions = motion.get_allowed_actions(request.user) # Support or unsupport motion. if request.method == 'POST': # Support motion. - if not allowed_actions['support']: + if not (motion.state.allow_support and + config['motions_min_supporters'] > 0 and + not motion.is_submitter(request.user) and + not motion.is_supporter(request.user)): raise ValidationError({'detail': _('You can not support this motion.')}) motion.supporters.add(request.user) motion.write_log([ugettext_noop('Motion supported')], request.user) @@ -228,7 +232,7 @@ class MotionViewSet(ModelViewSet): else: # Unsupport motion. # request.method == 'DELETE' - if not allowed_actions['unsupport']: + if not motion.state.allow_support or not motion.is_supporter(request.user): raise ValidationError({'detail': _('You can not unsupport this motion.')}) motion.supporters.remove(request.user) motion.write_log([ugettext_noop('Motion unsupported')], request.user) @@ -320,6 +324,8 @@ class MotionViewSet(ModelViewSet): View to create a poll. It is a POST request without any data. """ motion = self.get_object() + if not motion.state.allow_create_poll: + raise ValidationError({'detail': 'You can not create a poll in this motion state.'}) try: with transaction.atomic(): motion.create_poll() diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index c0d165392..20b70ddc8 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -298,6 +298,37 @@ class RetrieveMotion(TestCase): with self.assertNumQueries(16): self.client.get(reverse('motion-detail', args=[self.motion.pk])) + def test__guest_state_with_required_permission_to_see(self): + config['general_system_enable_anonymous'] = True + guest_client = APIClient() + state = self.motion.state + state.required_permission_to_see = 'permission_that_the_user_does_not_have_leeceiz9hi7iuta4ahY2' + state.save() + response = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_admin_state_with_required_permission_to_see(self): + state = self.motion.state + state.required_permission_to_see = 'permission_that_the_user_does_not_have_coo1Iewu8Eing2xahfoo' + state.save() + response = self.client.get(reverse('motion-detail', args=[self.motion.pk])) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_submitter_state_with_required_permission_to_see(self): + state = self.motion.state + state.required_permission_to_see = 'permission_that_the_user_does_not_have_eiW8af9caizoh1thaece' + state.save() + user = get_user_model().objects.create_user( + username='username_ohS2opheikaSa5theijo', + password='password_kau4eequaisheeBateef') + self.motion.submitters.add(user) + submitter_client = APIClient() + submitter_client.login( + username='username_ohS2opheikaSa5theijo', + password='password_kau4eequaisheeBateef') + response = submitter_client.get(reverse('motion-detail', args=[self.motion.pk])) + self.assertEqual(response.status_code, status.HTTP_200_OK) + class UpdateMotion(TestCase): """ diff --git a/tests/old/motions/test_models.py b/tests/old/motions/test_models.py index 4a06ead1e..18ab6f696 100644 --- a/tests/old/motions/test_models.py +++ b/tests/old/motions/test_models.py @@ -73,8 +73,6 @@ class ModelTest(TestCase): self.motion.state = State.objects.get(pk=6) self.assertEqual(self.motion.state.name, 'permitted') self.assertEqual(self.motion.state.get_action_word(), 'Permit') - self.assertFalse(self.motion.get_allowed_actions(self.test_user)['support']) - self.assertFalse(self.motion.get_allowed_actions(self.test_user)['unsupport']) def test_new_states_or_workflows(self): workflow_1 = Workflow.objects.create(name='W1')