From 5e13299ae32fb4cb042f7577121faf52db025677 Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Mon, 13 May 2019 10:17:24 +0200 Subject: [PATCH] Request for settings a permission in a group (closes #4698) --- .../users/group-repository.service.ts | 18 ++++- .../group-list/group-list.component.ts | 3 +- .../src/app/site/users/models/view-group.ts | 31 +------- openslides/users/views.py | 45 +++++++++++- tests/integration/users/test_viewset.py | 70 +++++++++++++++---- 5 files changed, 119 insertions(+), 48 deletions(-) diff --git a/client/src/app/core/repositories/users/group-repository.service.ts b/client/src/app/core/repositories/users/group-repository.service.ts index 093f68ff7..e7e4c98ee 100644 --- a/client/src/app/core/repositories/users/group-repository.service.ts +++ b/client/src/app/core/repositories/users/group-repository.service.ts @@ -9,6 +9,7 @@ import { Group } from 'app/shared/models/users/group'; import { ViewGroup } from 'app/site/users/models/view-group'; import { ViewModelStoreService } from 'app/core/core-services/view-model-store.service'; import { TranslateService } from '@ngx-translate/core'; +import { HttpService } from 'app/core/core-services/http.service'; /** * Shape of a permission @@ -53,7 +54,8 @@ export class GroupRepositoryService extends BaseRepository { mapperService: CollectionStringMapperService, viewModelStoreService: ViewModelStoreService, translate: TranslateService, - private constants: ConstantsService + private constants: ConstantsService, + private http: HttpService ) { super(DS, dataSend, mapperService, viewModelStoreService, translate, Group); this.sortPermsPerApp(); @@ -69,6 +71,20 @@ export class GroupRepositoryService extends BaseRepository { return viewGroup; } + /** + * Toggles the given permisson. + * + * @param group The group + * @param perm The permission to toggle + */ + public async togglePerm(group: ViewGroup, perm: string): Promise { + const set = !group.permissions.includes(perm); + return await this.http.post(`rest/${group.collectionString}/${group.id}/set_permission/`, { + perm: perm, + set: set + }); + } + /** * Add an entry to appPermissions * diff --git a/client/src/app/site/users/components/group-list/group-list.component.ts b/client/src/app/site/users/components/group-list/group-list.component.ts index fb548f988..74e592044 100644 --- a/client/src/app/site/users/components/group-list/group-list.component.ts +++ b/client/src/app/site/users/components/group-list/group-list.component.ts @@ -153,8 +153,7 @@ export class GroupListComponent extends BaseViewComponent implements OnInit { * @param perm */ public togglePerm(viewGroup: ViewGroup, perm: string): void { - const updateData = new Group({ permissions: viewGroup.getAlteredPermissions(perm) }); - this.repo.update(updateData, viewGroup).then(null, this.raiseError); + this.repo.togglePerm(viewGroup, perm); } /** diff --git a/client/src/app/site/users/models/view-group.ts b/client/src/app/site/users/models/view-group.ts index 28930404c..b482e34d9 100644 --- a/client/src/app/site/users/models/view-group.ts +++ b/client/src/app/site/users/models/view-group.ts @@ -41,33 +41,6 @@ export class ViewGroup extends BaseViewModel { this._group = group; } - /** - * Returns an array of permissions where the given perm is included - * or removed. - * - * Avoids touching the local DataStore. - * - * @param perm - */ - public getAlteredPermissions(perm: string): string[] { - // clone the array, avoids altering the local dataStore - const currentPermissions = this.permissions.slice(); - - if (this.hasPermission(perm)) { - // remove the permission from currentPermissions-List - const indexOfPerm = currentPermissions.indexOf(perm); - if (indexOfPerm !== -1) { - currentPermissions.splice(indexOfPerm, 1); - return currentPermissions; - } else { - return currentPermissions; - } - } else { - currentPermissions.push(perm); - return currentPermissions; - } - } - public hasPermission(perm: string): boolean { return this.permissions.includes(perm); } @@ -80,7 +53,5 @@ export class ViewGroup extends BaseViewModel { return this.group; } - public updateDependencies(update: BaseViewModel): void { - console.log('ViewGroups wants to update Values with : ', update); - } + public updateDependencies(update: BaseViewModel): void {} } diff --git a/openslides/users/views.py b/openslides/users/views.py index 72a102e83..7882c3386 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -10,6 +10,7 @@ from django.contrib.auth import ( update_session_auth_hash, ) from django.contrib.auth.forms import AuthenticationForm +from django.contrib.auth.models import Permission from django.contrib.auth.password_validation import validate_password from django.contrib.auth.tokens import default_token_generator from django.contrib.sites.shortcuts import get_current_site @@ -313,7 +314,13 @@ class GroupViewSet(ModelViewSet): # Every authenticated user can see the metadata. # Anonymous users can do so if they are enabled. result = self.request.user.is_authenticated or anonymous_is_enabled() - elif self.action in ("create", "partial_update", "update", "destroy"): + elif self.action in ( + "create", + "partial_update", + "update", + "destroy", + "set_permission", + ): # Users with all app permissions can edit groups. result = ( has_perm(self.request.user, "users.can_see_name") @@ -409,6 +416,42 @@ class GroupViewSet(ModelViewSet): inform_changed_data(affected_users) return Response(status=status.HTTP_204_NO_CONTENT) + @detail_route(methods=["post"]) + @transaction.atomic + def set_permission(self, request, *args, **kwargs): + """ + Send {perm: set: } to set or + remove the permission from a group + """ + perm = request.data.get("perm") + if not isinstance(perm, str): + raise ValidationError("You have to give a permission as string.") + set = request.data.get("set") + if not isinstance(set, bool): + raise ValidationError("You have to give a set value.") + + # check if perm is a valid permission + try: + app_label, codename = perm.split(".") + except ValueError: + raise ValidationError("Incorrect permission string") + try: + permission = Permission.objects.get( + content_type__app_label=app_label, codename=codename + ) + except Permission.DoesNotExist: + raise ValidationError("Incorrect permission string") + + # add/remove the permission + group = self.get_object() + if set: + group.permissions.add(permission) + else: + group.permissions.remove(permission) + inform_changed_data(group) + + return Response() + class PersonalNoteViewSet(ModelViewSet): """ diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index faf8db452..462f804ac 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -10,6 +10,7 @@ from openslides.users.serializers import UserFullSerializer from openslides.utils.autoupdate import inform_changed_data from openslides.utils.test import TestCase +from ...common_groups import GROUP_DEFAULT_PK, GROUP_DELEGATE_PK, GROUP_STAFF_PK from ..helpers import count_queries @@ -93,8 +94,7 @@ class UserCreate(TestCase): def test_creation_with_group(self): self.client.login(username="admin", password="admin") - # These are the builtin groups 'Delegates' and 'Staff'. The pks are valid. - group_pks = (2, 3) + group_pks = (GROUP_DELEGATE_PK, GROUP_STAFF_PK) self.client.post( reverse("user-list"), @@ -107,9 +107,7 @@ class UserCreate(TestCase): def test_creation_with_default_group(self): self.client.login(username="admin", password="admin") - # This is the builtin groups 'default'. - # The pk is valid. But this group can not be added to users. - group_pk = (1,) + group_pk = (GROUP_DEFAULT_PK,) response = self.client.post( reverse("user-list"), @@ -382,7 +380,7 @@ class GroupReceive(TestCase): user = User(username="test") user.set_password("test") user.save() - default_group = Group.objects.get(pk=1) + default_group = Group.objects.get(pk=GROUP_DEFAULT_PK) default_group.permissions.all().delete() self.client.login(username="test", password="test") @@ -463,8 +461,7 @@ class GroupUpdate(TestCase): def test_simple_update_via_patch(self): admin_client = APIClient() admin_client.login(username="admin", password="admin") - # This is the builtin group 'Delegates'. The pk is valid. - group_pk = 2 + group_pk = GROUP_DELEGATE_PK # This contains one valid permission of the users app. permissions = ("users.can_see_name",) @@ -485,13 +482,12 @@ class GroupUpdate(TestCase): def test_simple_update_via_put(self): admin_client = APIClient() admin_client.login(username="admin", password="admin") - # This is the builtin group 'Delegates'. The pk is valid. - group_pk = 2 # This contains one valid permission of the users app. permissions = ("users.can_see_name",) response = admin_client.put( - reverse("group-detail", args=[group_pk]), {"permissions": permissions} + reverse("group-detail", args=[GROUP_DELEGATE_PK]), + {"permissions": permissions}, ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -547,6 +543,54 @@ class GroupUpdate(TestCase): ) ) + def test_set_single_permission(self): + admin_client = APIClient() + admin_client.login(username="admin", password="admin") + + response = admin_client.post( + reverse("group-set-permission", args=[GROUP_DEFAULT_PK]), + {"perm": "users.can_manage", "set": True}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + group = Group.objects.get(pk=GROUP_DEFAULT_PK) + self.assertTrue( + group.permissions.get( + content_type__app_label="users", codename="can_manage" + ) + ) + + def test_add_single_permission_wrong_permission(self): + admin_client = APIClient() + admin_client.login(username="admin", password="admin") + + response = admin_client.post( + reverse("group-set-permission", args=[GROUP_DEFAULT_PK]), + {"perm": "not_existing.permission", "set": True}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_remove_single_permission(self): + admin_client = APIClient() + admin_client.login(username="admin", password="admin") + + response = admin_client.post( + reverse("group-set-permission", args=[GROUP_DEFAULT_PK]), + {"perm": "users.can_see_name", "set": False}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + group = Group.objects.get(pk=GROUP_DEFAULT_PK) + self.assertFalse( + group.permissions.filter( + content_type__app_label="users", codename="can_see" + ).exists() + ) + class GroupDelete(TestCase): """ @@ -568,10 +612,8 @@ class GroupDelete(TestCase): def test_delete_builtin_groups(self): admin_client = APIClient() admin_client.login(username="admin", password="admin") - # The pk of builtin group 'Default' - group_pk = 1 - response = admin_client.delete(reverse("group-detail", args=[group_pk])) + response = admin_client.delete(reverse("group-detail", args=[GROUP_DEFAULT_PK])) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)