From 6f24b7c169688a2e9b737c81905d04f42c32863d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Wed, 20 Mar 2019 17:21:01 +0100 Subject: [PATCH 1/2] Refactored state access level by renaming state field to restriction. --- CHANGELOG.rst | 5 ++- openslides/motions/access_permissions.py | 32 ++++++++++------ .../migrations/0023_state_restriction_1.py | 25 +++++++++++++ .../migrations/0023_state_restriction_2.py | 36 ++++++++++++++++++ .../migrations/0023_state_restriction_3.py | 10 +++++ openslides/motions/models.py | 37 ++++++++----------- openslides/motions/serializers.py | 37 ++++++++++++++++--- tests/integration/motions/test_viewset.py | 13 +++---- tests/unit/motions/test_projector.py | 14 +++---- 9 files changed, 153 insertions(+), 56 deletions(-) create mode 100644 openslides/motions/migrations/0023_state_restriction_1.py create mode 100644 openslides/motions/migrations/0023_state_restriction_2.py create mode 100644 openslides/motions/migrations/0023_state_restriction_3.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 02a7e0dfa..5343b65ab 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -35,8 +35,9 @@ Motions: follow recommendation, manage submitters and supporters, change motion category, motion block and origin and manage motion polls [#3913]. - Added new permission to create amendments [#4128]. - - Added new flag to motion state to control access for different users. Added - new permission to see motions in some internal state [#4235, #4518]. + - Added new flag to motion state to control access restrictions for different + users. Added new permission to see motions in some internal state + [#4235, #4518, #4521]. - Allowed submitters to set state of new motions in complex and customized workflow [#4236]. - Added multi select action to manage submitters, tags, states and diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 655a589a0..ddadaeac4 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -37,19 +37,27 @@ class MotionAccessPermissions(BaseAccessPermissions): is_submitter = False # Check see permission for this motion. - from .models import State + restriction = full["state_restriction"] - if await async_has_perm(user_id, "motions.can_manage"): - level = State.MANAGERS_ONLY - elif await async_has_perm( - user_id, "motions.can_manage_metadata" - ) or await async_has_perm(user_id, "motions.can_see_internal"): - level = State.EXTENDED_MANAGERS - elif is_submitter: - level = State.EXTENDED_MANAGERS_AND_SUBMITTER - else: - level = State.ALL - permission = level >= full["state_access_level"] + # Managers can see all motions. + permission = await async_has_perm(user_id, "motions.can_manage") + # If restriction field is an empty list, everybody can see the motion. + permission = permission or not restriction + + if not permission: + # Parse values of restriction field. + for value in restriction: + if value == "managers_only": + # permission remains false + break + elif value in ("motions.can_see_internal", "motions.can_manage_metadata"): + if await async_has_perm(user_id, value): + permission = True + break + elif value == "is_submitter": + if is_submitter: + permission = True + break # Parse single motion. if permission: diff --git a/openslides/motions/migrations/0023_state_restriction_1.py b/openslides/motions/migrations/0023_state_restriction_1.py new file mode 100644 index 000000000..59c3c0198 --- /dev/null +++ b/openslides/motions/migrations/0023_state_restriction_1.py @@ -0,0 +1,25 @@ +# Generated by Django 2.1.7 on 2019-03-20 15:49 + +from django.db import migrations +import jsonfield.encoder +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [("motions", "0022_auto_20190320_0840")] + + operations = [ + migrations.AddField( + model_name="state", + name="restriction", + field=jsonfield.fields.JSONField( + default=list, + dump_kwargs={ + "cls": jsonfield.encoder.JSONEncoder, + "separators": (",", ":"), + }, + load_kwargs={}, + ), + ) + ] diff --git a/openslides/motions/migrations/0023_state_restriction_2.py b/openslides/motions/migrations/0023_state_restriction_2.py new file mode 100644 index 000000000..46c483dbb --- /dev/null +++ b/openslides/motions/migrations/0023_state_restriction_2.py @@ -0,0 +1,36 @@ +# Generated by Django 2.1.7 on 2019-03-20 15:49 + +from django.db import migrations +import jsonfield.encoder +import jsonfield.fields + + +def copy_access_level(apps, schema_editor): + """ + Sets new restriction field of states according to access_level. + """ + # We get the model from the versioned app registry; + # if we directly import it, it will be the wrong version. + State = apps.get_model("motions", "State") + for state in State.objects.all(): + if state.access_level == 3: + state.restriction = ["managers_only"] + elif state.access_level == 2: + state.restriction = [ + "motions.can_see_internal", + "motions.can_manage_metadata", + ] + elif state.access_level == 1: + state.restriction = [ + "motions.can_see_internal", + "motions.can_manage_metadata", + "is_submitter", + ] + state.save(skip_autoupdate=True) + + +class Migration(migrations.Migration): + + dependencies = [("motions", "0023_state_restriction_1")] + + operations = [migrations.RunPython(copy_access_level)] diff --git a/openslides/motions/migrations/0023_state_restriction_3.py b/openslides/motions/migrations/0023_state_restriction_3.py new file mode 100644 index 000000000..152872f8a --- /dev/null +++ b/openslides/motions/migrations/0023_state_restriction_3.py @@ -0,0 +1,10 @@ +# Generated by Django 2.1.7 on 2019-03-20 15:55 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [("motions", "0023_state_restriction_2")] + + operations = [migrations.RemoveField(model_name="state", name="access_level")] diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 97d1e5ecd..e198ed4c8 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -985,24 +985,6 @@ class State(RESTModelMixin, models.Model): state. """ - ALL = 0 - EXTENDED_MANAGERS_AND_SUBMITTER = 1 - EXTENDED_MANAGERS = 2 - MANAGERS_ONLY = 3 - - ACCESS_LEVELS = ( - (ALL, "All users with permission to see motions"), - ( - EXTENDED_MANAGERS_AND_SUBMITTER, - "Submitters, authorized users (with permission to see internal motions), managers and users with permission to manage metadata", - ), - ( - EXTENDED_MANAGERS, - "Only authorized users (with permission to see internal motions), managers and users with permission to manage metadata", - ), - (MANAGERS_ONLY, "Only managers"), - ) - name = models.CharField(max_length=255) """A string representing the state.""" @@ -1024,11 +1006,22 @@ class State(RESTModelMixin, models.Model): Default value is 'primary' (blue). """ - access_level = models.IntegerField(choices=ACCESS_LEVELS, default=0) + restriction = JSONField(default=list) """ - Defines which users may see motions in this state e. g. only managers, - authorized users with permission to see internal motiosn, users with permission - to manage metadata and submitters. + Defines which users may see motions in this state: + + Contains a list of one or more of the following strings: + * motions.can_see_internal + * motions.can_manage_metadata + * is_submitter + * managers_only + + If the list is empty, everybody with the general permission to see motions + can see this motion. If the list contains 'managers_only', only managers with + motions.can_manage permission may see this motion. In all other cases the user + shall have one of the given permissions respectivly is submitter of the motion. + + Default: Empty list so everybody can see the motion. """ allow_support = models.BooleanField(default=False) diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 710314c29..cefd072e9 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -1,5 +1,6 @@ from typing import Dict, Optional +import jsonschema from django.db import transaction from ..core.config import config @@ -101,7 +102,7 @@ class StateSerializer(ModelSerializer): "name", "recommendation_label", "css_class", - "access_level", + "restriction", "allow_support", "allow_create_poll", "allow_submitter_edit", @@ -113,6 +114,30 @@ class StateSerializer(ModelSerializer): "workflow", ) + def validate_restriction(self, value): + """ + Ensures that the value is a list and only contains valid values. + """ + schema = { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Motion workflow state restriction field schema", + "description": "An array containing one or more explicit strings to control restriction for motions in this state.", + "type": "array", + "enum": [ + "motions.can_see_internal", + "motions.can_manage_metadata", + "is_submitter", + "managers_only", + ], + } + + # Validate value. + try: + jsonschema.validate(request.data, schema) + except jsonschema.ValidationError as err: + raise ValidationError({"detail": str(err)}) + return value + class WorkflowSerializer(ModelSerializer): """ @@ -369,7 +394,7 @@ class MotionSerializer(ModelSerializer): polls = MotionPollSerializer(many=True, read_only=True) modified_final_version = CharField(allow_blank=True, required=False) reason = CharField(allow_blank=True, required=False) - state_access_level = SerializerMethodField() + state_restriction = SerializerMethodField() text = CharField(allow_blank=True) title = CharField(max_length=255) amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False) @@ -404,7 +429,7 @@ class MotionSerializer(ModelSerializer): "supporters", "state", "state_extension", - "state_access_level", + "state_restriction", "statute_paragraph", "workflow_id", "recommendation", @@ -509,9 +534,9 @@ class MotionSerializer(ModelSerializer): return result - def get_state_access_level(self, motion): + def get_state_restriction(self, motion): """ - Returns the access level of this state. The default is 0 so everybody + Returns the restriction of this state. The default is an empty list so everybody with permission to see motions can see this motion. """ - return motion.state.access_level + return motion.state.restriction diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index a7741dfd1..08ea4ab75 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -477,11 +477,11 @@ class RetrieveMotion(TestCase): username=f"user_{index}", password="password" ) - def test_guest_state_with_access_level(self): + def test_guest_state_with_restriction(self): config["general_system_enable_anonymous"] = True guest_client = APIClient() state = self.motion.state - state.access_level = State.MANAGERS_ONLY + state.restriction = ["managers_only"] state.save() # The cache has to be cleared, see: # https://github.com/OpenSlides/OpenSlides/issues/3396 @@ -490,17 +490,16 @@ class RetrieveMotion(TestCase): response = guest_client.get(reverse("motion-detail", args=[self.motion.pk])) self.assertEqual(response.status_code, 404) - def test_admin_state_with_access_level(self): + def test_admin_state_with_restriction(self): state = self.motion.state - state.access_level = State.MANAGERS_ONLY - + state.restriction = ["managers_only"] 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_access_level(self): + def test_submitter_state_with_restriction(self): state = self.motion.state - state.access_level = State.EXTENDED_MANAGERS_AND_SUBMITTER + state.restriction = ["is_submitter"] state.save() user = get_user_model().objects.create_user( username="username_ohS2opheikaSa5theijo", diff --git a/tests/unit/motions/test_projector.py b/tests/unit/motions/test_projector.py index bb46b7564..a8b0eb1eb 100644 --- a/tests/unit/motions/test_projector.py +++ b/tests/unit/motions/test_projector.py @@ -29,7 +29,7 @@ def all_data(): "supporters_id": [], "state_id": 1, "state_extension": None, - "state_access_level": 0, + "state_restriction": [], "statute_paragraph_id": None, "workflow_id": 1, "recommendation_id": None, @@ -118,7 +118,7 @@ def all_data(): "supporters_id": [], "state_id": 1, "state_extension": None, - "state_access_level": 0, + "state_restriction": [], "statute_paragraph_id": None, "workflow_id": 1, "recommendation_id": None, @@ -151,7 +151,7 @@ def all_data(): "supporters_id": [], "state_id": 1, "state_extension": None, - "state_access_level": 0, + "state_restriction": [], "statute_paragraph_id": 1, "workflow_id": 1, "recommendation_id": None, @@ -178,7 +178,7 @@ def all_data(): "name": "submitted", "recommendation_label": None, "css_class": "primary", - "access_level": 0, + "restriction": [], "allow_support": True, "allow_create_poll": True, "allow_submitter_edit": True, @@ -194,7 +194,7 @@ def all_data(): "name": "accepted", "recommendation_label": "Acceptance", "css_class": "success", - "access_level": 0, + "restriction": [], "allow_support": False, "allow_create_poll": False, "allow_submitter_edit": False, @@ -210,7 +210,7 @@ def all_data(): "name": "rejected", "recommendation_label": "Rejection", "css_class": "danger", - "access_level": 0, + "restriction": [], "allow_support": False, "allow_create_poll": False, "allow_submitter_edit": False, @@ -226,7 +226,7 @@ def all_data(): "name": "not decided", "recommendation_label": "No decision", "css_class": "default", - "access_level": 0, + "restriction": [], "allow_support": False, "allow_create_poll": False, "allow_submitter_edit": False, From 23264849c9088c42268704895d956e8218630047 Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Mon, 1 Apr 2019 14:35:02 +0200 Subject: [PATCH 2/2] client and small changes in the serializer --- .../shared/models/motions/workflow-state.ts | 2 +- .../workflow-detail.component.html | 29 +++++++---- .../workflow-detail.component.ts | 50 ++++++++++++------- .../services/motion-filter-list.service.ts | 2 +- openslides/motions/access_permissions.py | 5 +- ...ction_1.py => 0024_state_restriction_1.py} | 4 +- ...ction_2.py => 0024_state_restriction_2.py} | 4 +- ...ction_3.py => 0024_state_restriction_3.py} | 2 +- openslides/motions/serializers.py | 20 +++++--- 9 files changed, 76 insertions(+), 42 deletions(-) rename openslides/motions/migrations/{0023_state_restriction_1.py => 0024_state_restriction_1.py} (90%) rename openslides/motions/migrations/{0023_state_restriction_2.py => 0024_state_restriction_2.py} (90%) rename openslides/motions/migrations/{0023_state_restriction_3.py => 0024_state_restriction_3.py} (77%) diff --git a/client/src/app/shared/models/motions/workflow-state.ts b/client/src/app/shared/models/motions/workflow-state.ts index 8c0040acc..8cb644f12 100644 --- a/client/src/app/shared/models/motions/workflow-state.ts +++ b/client/src/app/shared/models/motions/workflow-state.ts @@ -21,7 +21,7 @@ export class WorkflowState extends Deserializer { public name: string; public recommendation_label: string; public css_class: string; - public access_level: number; + public restriction: string[]; public allow_support: boolean; public allow_create_poll: boolean; public allow_submitter_edit: boolean; diff --git a/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.html b/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.html index 2bf76810f..8fbf43135 100644 --- a/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.html +++ b/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.html @@ -103,13 +103,24 @@
- {{ accessLevels[state.access_level].label | translate }} +
+ - +
+
+ + {{ getRestrictionLabel(restriction) | translate }},  + +
@@ -179,12 +190,12 @@ - - + + - diff --git a/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.ts b/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.ts index c92ad6f05..6a46aa1cf 100644 --- a/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.ts +++ b/client/src/app/site/motions/modules/motion-workflow/components/workflow-detail/workflow-detail.component.ts @@ -41,18 +41,18 @@ interface StatePerm { } /** - * Defines the structure of access levels + * Defines the structure of AmendmentIntoFinal */ -interface AccessLevel { - level: number; +interface AmendmentIntoFinal { + merge: MergeAmendment; label: string; } /** - * Defines the structure of access levels + * Defines the structre of restrictions */ -interface AmendmentIntoFinal { - merge: MergeAmendment; +interface Restriction { + key: string; label: string; } @@ -108,20 +108,20 @@ export class WorkflowDetailComponent extends BaseViewComponent implements OnInit type: 'check' }, { name: 'Show amendment in parent motoin', selector: 'merge_amendment_into_final', type: 'amendment' }, - { name: 'Access level', selector: 'access_level', type: 'accessLevel' }, + { name: 'Restrictions', selector: 'restriction', type: 'restriction' }, { name: 'Label color', selector: 'css_class', type: 'color' }, { name: 'Next states', selector: 'next_states_id', type: 'state' } ] as StatePerm[]; /** - * Determines possible access levels + * Determines possible restrictions */ - public accessLevels = [ - { level: 0, label: '0: All users' }, - { level: 1, label: '1: Submitters, authorized users and managers' }, - { level: 2, label: '2: Authorized users and managers for motions and metadata' }, - { level: 3, label: '3: Only managers for motions' } - ] as AccessLevel[]; + public restrictions = [ + { key: 'managers_only', label: 'Managers only' }, + { key: 'motions.can_see_internal', label: 'Can see internal (perm)' }, + { key: 'motions.can_manage_metadata', label: 'Can manage metadata (perm)' }, + { key: 'is_submitter', label: 'Is submitter' } + ] as Restriction[]; /** * Determines possible "Merge amendments into final" @@ -279,11 +279,27 @@ export class WorkflowDetailComponent extends BaseViewComponent implements OnInit /** * Sets an access level to the given workflow state * - * @param level The new access level + * @param restrictions The new restrictions * @param state the state to change */ - public onSetAccesLevel(level: number, state: WorkflowState): void { - this.workflowRepo.updateState({ access_level: level }, state).then(() => {}, this.raiseError); + public onSetRestriction(restriction: string, state: WorkflowState): void { + const restrictions = state.restriction.map(r => r); + const restrictionIndex = restrictions.findIndex(r => r === restriction); + + if (restrictionIndex < 0) { + restrictions.push(restriction); + } else { + restrictions.splice(restrictionIndex, 1); + } + this.workflowRepo.updateState({ restriction: restrictions }, state).then(() => {}, this.raiseError); + } + + /** + * @returns the restriction label for the given restriction + */ + public getRestrictionLabel(restriction: string): string { + const entry = this.restrictions.find(r => r.key === restriction); + return entry ? entry.label : ''; } /** diff --git a/client/src/app/site/motions/services/motion-filter-list.service.ts b/client/src/app/site/motions/services/motion-filter-list.service.ts index a992b9bf9..e6ce4543d 100644 --- a/client/src/app/site/motions/services/motion-filter-list.service.ts +++ b/client/src/app/site/motions/services/motion-filter-list.service.ts @@ -246,7 +246,7 @@ export class MotionFilterListService extends BaseFilterListService