From 4ffe2b5a800a2934265e411d1db3acccb7636fd4 Mon Sep 17 00:00:00 2001 From: Finn Stutzenstein Date: Tue, 30 Aug 2016 09:16:47 +0200 Subject: [PATCH 1/2] Migration for new permission matrix --- openslides/users/migrations/0004_groups.py | 56 ++++++++++++++++++++ openslides/users/models.py | 19 ++++--- openslides/users/signals.py | 4 +- tests/unit/users/test_models.py | 59 ++++++++++++++++------ 4 files changed, 114 insertions(+), 24 deletions(-) create mode 100644 openslides/users/migrations/0004_groups.py diff --git a/openslides/users/migrations/0004_groups.py b/openslides/users/migrations/0004_groups.py new file mode 100644 index 000000000..99fefe4ec --- /dev/null +++ b/openslides/users/migrations/0004_groups.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.7 on 2016-08-01 14:54 +from __future__ import unicode_literals + +from django.core.exceptions import ObjectDoesNotExist +from django.db import migrations + + +def migrate_groups_and_user_permissions(apps, schema_editor): + """ + This function migrates the database to the new groups logic: + - Rename Group 'Anonymous' (pk=1) to 'Default' + - Rename Group 'Registered users' (pk=2) to 'Previous group Registered' + - Add all users who are not in any group to this group (pk=2) + - Add all permissions of 'Previous group Registered' to all other groups (exclude 'Default') + + But only do this migration if: + - there are groups in the database + - the name of the first group is 'Guests'. + """ + User = apps.get_model('users', 'User') + Group = apps.get_model('auth', 'Group') + + if Group.objects.exists(): + try: + group_default = Group.objects.filter(pk=1) + if group_default.get().name == 'Guests': + group_default.update(name='Default') + + group_old_registered = Group.objects.filter(pk=2) + group_old_registered.update(name='Previous group Registered') + group_old_registered = group_old_registered.get() + + users = User.objects.all() + for user in users: + if not user.groups.exists(): + user.groups.add(group_old_registered.pk) + + groups = Group.objects.filter(pk__gt=2) + for group in groups: + for permission in group_old_registered.permissions.all(): + group.permissions.add(permission) + except ObjectDoesNotExist: + # If the first or second group doesn't exists, just pass this migraition + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0003_user_number'), + ] + + operations = [ + migrations.RunPython(migrate_groups_and_user_permissions), + ] diff --git a/openslides/users/models.py b/openslides/users/models.py index f645dd7b4..816acec4f 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -5,16 +5,17 @@ from django.contrib.auth.models import ( AbstractBaseUser, BaseUserManager, Group, + Permission, PermissionsMixin, ) from django.db import models +from django.db.models import Q from openslides.utils.search import user_name_helper from ..core.config import config from ..utils.models import RESTModelMixin from .access_permissions import UserAccessPermissions -from .exceptions import UsersError class UserManager(BaseUserManager): @@ -35,13 +36,17 @@ class UserManager(BaseUserManager): """ Creates an user with the username 'admin'. If such a user already exists, resets it. The password is (re)set to 'admin'. The user - becomes member of the group 'Staff' (pk=3). + becomes member of the group 'Staff'. The two important permissions + 'users.can_see_name' and 'users.can_manage' are added to this group, + so that the admin can manage all other permissions. """ - try: - staff = Group.objects.get(pk=3) - except Group.DoesNotExist: - raise UsersError("Admin user can not be created or reset because " - "the group 'Staff' (pk=3) is not available.") + query_can_see_name = Q(content_type__app_label='users') & Q(codename='can_see_name') + query_can_manage = Q(content_type__app_label='users') & Q(codename='can_manage') + + staff, _ = Group.objects.get_or_create(name='Staff') + staff.permissions.add(Permission.objects.get(query_can_see_name)) + staff.permissions.add(Permission.objects.get(query_can_manage)) + admin, created = self.get_or_create( username='admin', defaults={'last_name': 'Administrator'}) diff --git a/openslides/users/signals.py b/openslides/users/signals.py index 483bd4869..9700f718c 100644 --- a/openslides/users/signals.py +++ b/openslides/users/signals.py @@ -10,8 +10,8 @@ def create_builtin_groups_and_admin(**kwargs): Creates the builtin user: admin. """ - # Check whether the group pk's 1 to 4 are free. - if Group.objects.filter(pk__in=range(1, 5)).exists(): + # Check whether there are groups in the database. + if Group.objects.exists(): # Do completely nothing if there are already some of our groups in the database. return diff --git a/tests/unit/users/test_models.py b/tests/unit/users/test_models.py index 0d005da24..5a4bbc23a 100644 --- a/tests/unit/users/test_models.py +++ b/tests/unit/users/test_models.py @@ -286,30 +286,26 @@ class UserManagerGeneratePassword(TestCase): for _ in range(8)]) +@patch('openslides.users.models.Permission') @patch('openslides.users.models.Group') class UserManagerCreateOrResetAdminUser(TestCase): - def test_get_admin_group(self, mock_group): + def test_add_admin_group(self, mock_group, mock_permission): """ - Tests that the Group with pk=3 is added to the admin. + Tests that the Group with name='Staff' is added to the admin. """ - def mock_side_effect(pk): - if pk == 3: - result = 'mock_staff' - else: - result = '' - return result - admin_user = MagicMock() manager = UserManager() manager.get_or_create = MagicMock(return_value=(admin_user, False)) - mock_group.objects.get.side_effect = mock_side_effect + + staff_group = MagicMock(name="Staff") + mock_group.objects.get_or_create = MagicMock(return_value=(staff_group, True)) + mock_permission.get = MagicMock() manager.create_or_reset_admin_user() - mock_group.objects.get.assert_called_once_with(pk=3) - admin_user.groups.add.assert_called_once_with('mock_staff') + admin_user.groups.add.assert_called_once_with(staff_group) - def test_password_set_to_admin(self, mock_group): + def test_password_set_to_admin(self, mock_group, mock_permission): """ Tests that the password of the admin is set to 'admin'. """ @@ -317,6 +313,10 @@ class UserManagerCreateOrResetAdminUser(TestCase): manager = UserManager() manager.get_or_create = MagicMock(return_value=(admin_user, False)) + staff_group = MagicMock(name="Staff") + mock_group.objects.get_or_create = MagicMock(return_value=(staff_group, True)) + mock_permission.get = MagicMock() + manager.create_or_reset_admin_user() self.assertEqual( @@ -325,7 +325,7 @@ class UserManagerCreateOrResetAdminUser(TestCase): admin_user.save.assert_called_once_with() @patch('openslides.users.models.User') - def test_return_value(self, mock_user, mock_group): + def test_return_value(self, mock_user, mock_group, mock_permission): """ Tests that the method returns True when a user is created. """ @@ -334,6 +334,10 @@ class UserManagerCreateOrResetAdminUser(TestCase): manager.get_or_create = MagicMock(return_value=(admin_user, True)) manager.model = mock_user + staff_group = MagicMock(name="Staff") + mock_group.objects.get_or_create = MagicMock(return_value=(staff_group, True)) + mock_permission.get = MagicMock() + self.assertEqual( manager.create_or_reset_admin_user(), True, @@ -341,7 +345,7 @@ class UserManagerCreateOrResetAdminUser(TestCase): "new user is created.") @patch('openslides.users.models.User') - def test_attributes_of_created_user(self, mock_user, mock_group): + def test_attributes_of_created_user(self, mock_user, mock_group, mock_permission): """ Tests username and last_name of the created admin user. """ @@ -350,6 +354,10 @@ class UserManagerCreateOrResetAdminUser(TestCase): manager.get_or_create = MagicMock(return_value=(admin_user, True)) manager.model = mock_user + staff_group = MagicMock(name="Staff") + mock_group.objects.get_or_create = MagicMock(return_value=(staff_group, True)) + mock_permission.get = MagicMock() + manager.create_or_reset_admin_user() self.assertEqual( @@ -360,3 +368,24 @@ class UserManagerCreateOrResetAdminUser(TestCase): admin_user.last_name, 'Administrator', "The last_name of a new created admin should be 'Administrator'.") + + def test_get_permissions(self, mock_group, mock_permission): + """ + Tests if two permissions are get + """ + admin_user = MagicMock() + manager = UserManager() + manager.get_or_create = MagicMock(return_value=(admin_user, True)) + + staff_group = MagicMock(name="Staff") + mock_group.objects.get_or_create = MagicMock(return_value=(staff_group, True)) + + permission_mock = MagicMock(name="test permission") + mock_permission.objects.get = MagicMock(return_value=permission_mock) + + manager.create_or_reset_admin_user() + + self.assertEqual( + mock_permission.objects.get.call_count, + 2, + "Permission.get should be called two times") From cd3c4709199d3bee7079a46d6d4d628d206ab2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Thu, 8 Sep 2016 11:40:58 +0200 Subject: [PATCH 2/2] Changed migration coding style. --- openslides/users/exceptions.py | 5 -- openslides/users/migrations/0004_groups.py | 62 ++++++++++++---------- openslides/users/signals.py | 2 +- 3 files changed, 35 insertions(+), 34 deletions(-) delete mode 100644 openslides/users/exceptions.py diff --git a/openslides/users/exceptions.py b/openslides/users/exceptions.py deleted file mode 100644 index 185aa383d..000000000 --- a/openslides/users/exceptions.py +++ /dev/null @@ -1,5 +0,0 @@ -from ..utils.exceptions import OpenSlidesError - - -class UsersError(OpenSlidesError): - pass diff --git a/openslides/users/migrations/0004_groups.py b/openslides/users/migrations/0004_groups.py index 99fefe4ec..262526b0d 100644 --- a/openslides/users/migrations/0004_groups.py +++ b/openslides/users/migrations/0004_groups.py @@ -2,47 +2,53 @@ # Generated by Django 1.9.7 on 2016-08-01 14:54 from __future__ import unicode_literals -from django.core.exceptions import ObjectDoesNotExist -from django.db import migrations +from django.db import migrations, models def migrate_groups_and_user_permissions(apps, schema_editor): """ This function migrates the database to the new groups logic: - - Rename Group 'Anonymous' (pk=1) to 'Default' - - Rename Group 'Registered users' (pk=2) to 'Previous group Registered' + - Rename group 'Anonymous' (pk=1) to 'Default' + - Rename group 'Registered users' (pk=2) to 'Previous group Registered' - Add all users who are not in any group to this group (pk=2) - - Add all permissions of 'Previous group Registered' to all other groups (exclude 'Default') + - Add all permissions of 'Previous group Registered' to all other groups (except 'Default') - But only do this migration if: - - there are groups in the database - - the name of the first group is 'Guests'. + But only run this migration if: + - there are groups in the database, + - the name of the first group is 'Guests', + - the name of the second group is 'Registered users'. """ + # Disconnect autoupdate. We do not want to trigger it here. + models.signals.post_save.disconnect(dispatch_uid='inform_changed_data_receiver') + User = apps.get_model('users', 'User') Group = apps.get_model('auth', 'Group') - if Group.objects.exists(): - try: - group_default = Group.objects.filter(pk=1) - if group_default.get().name == 'Guests': - group_default.update(name='Default') + try: + group_default = Group.objects.get(pk=1) + group_registered = Group.objects.get(pk=2) + except Group.DoesNotExist: + # One of the groups does not exist. Just do nothing. + pass + else: + if group_default.name == 'Guests' and group_registered.name == 'Registered users': + # Rename groups pk 1 and 2. + group_default.name = 'Default' + group_default.save() + group_registered.name = 'Previous group Registered' + group_registered.save() - group_old_registered = Group.objects.filter(pk=2) - group_old_registered.update(name='Previous group Registered') - group_old_registered = group_old_registered.get() + # Move users without groups to group pk 2. + users = User.objects.all() + for user in users: + if not user.groups.exists(): + user.groups.add(group_registered) - users = User.objects.all() - for user in users: - if not user.groups.exists(): - user.groups.add(group_old_registered.pk) - - groups = Group.objects.filter(pk__gt=2) - for group in groups: - for permission in group_old_registered.permissions.all(): - group.permissions.add(permission) - except ObjectDoesNotExist: - # If the first or second group doesn't exists, just pass this migraition - pass + # Copy permissions of group pk 2 to all other groups except pk 1. + groups = Group.objects.filter(pk__gt=2) + for group in groups: + for permission in group_registered.permissions.all(): + group.permissions.add(permission) class Migration(migrations.Migration): diff --git a/openslides/users/signals.py b/openslides/users/signals.py index 9700f718c..8d6437764 100644 --- a/openslides/users/signals.py +++ b/openslides/users/signals.py @@ -12,7 +12,7 @@ def create_builtin_groups_and_admin(**kwargs): """ # Check whether there are groups in the database. if Group.objects.exists(): - # Do completely nothing if there are already some of our groups in the database. + # Do completely nothing if there are already some groups in the database. return permission_strings = (