From f65f36336179aa08aa90a75fe7fb778868c8cd67 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Tue, 29 Jan 2019 13:35:10 -0400 Subject: [PATCH] Refactor user management app Add keyword arguments. Update view resolutions and URL parameters to the '_id' form. Remove code from create and edit subclasses and user the super class error checking. Cache the view object instead of using .get_object() every time. Movernize tests. Update views to comply with MERCs 5 and 6. Split UserTestMixin into mixins for Groups and Users tests. Add super delete and detail tests. Remove redundant superuser filtering from views. Add transactions to views that also commit events. Signed-off-by: Roberto Rosario --- mayan/apps/user_management/api_views.py | 53 ++- mayan/apps/user_management/apps.py | 21 +- mayan/apps/user_management/events.py | 2 +- mayan/apps/user_management/icons.py | 2 +- mayan/apps/user_management/links.py | 37 ++- mayan/apps/user_management/methods.py | 4 +- mayan/apps/user_management/models.py | 5 +- mayan/apps/user_management/permissions.py | 20 +- mayan/apps/user_management/search.py | 3 +- mayan/apps/user_management/serializers.py | 73 ++--- mayan/apps/user_management/tests/__init__.py | 1 + mayan/apps/user_management/tests/literals.py | 20 +- mayan/apps/user_management/tests/mixins.py | 144 ++++++--- mayan/apps/user_management/tests/test_api.py | 297 ++++++++++------- .../apps/user_management/tests/test_events.py | 33 +- .../apps/user_management/tests/test_models.py | 2 +- .../apps/user_management/tests/test_views.py | 301 ++++++++++++++---- mayan/apps/user_management/urls.py | 111 ++++--- mayan/apps/user_management/utils.py | 18 +- mayan/apps/user_management/views.py | 290 +++++++++-------- 20 files changed, 886 insertions(+), 551 deletions(-) diff --git a/mayan/apps/user_management/api_views.py b/mayan/apps/user_management/api_views.py index 2d914424db..47b467fe72 100644 --- a/mayan/apps/user_management/api_views.py +++ b/mayan/apps/user_management/api_views.py @@ -7,6 +7,7 @@ from django.shortcuts import get_object_or_404 from rest_framework import generics from mayan.apps.acls.models import AccessControlList +from mayan.apps.common.mixins import ExternalObjectMixin from mayan.apps.rest_api.filters import MayanObjectPermissionsFilter from mayan.apps.rest_api.permissions import MayanPermission @@ -16,7 +17,7 @@ from .permissions import ( permission_user_edit, permission_user_view ) from .serializers import ( - GroupSerializer, UserSerializer, UserGroupListSerializer + GroupSerializer, UserSerializer#, UserGroupListSerializer ) @@ -53,6 +54,7 @@ class APIGroupView(generics.RetrieveUpdateDestroyAPIView): patch: Partially edit the selected group. put: Edit the selected group. """ + lookup_url_kwarg = 'group_pk' mayan_object_permissions = { 'GET': (permission_group_view,), 'PUT': (permission_group_edit,), @@ -84,6 +86,7 @@ class APIUserView(generics.RetrieveUpdateDestroyAPIView): patch: Partially edit the selected user. put: Edit the selected user. """ + lookup_url_kwarg = 'user_pk' mayan_object_permissions = { 'GET': (permission_user_view,), 'PUT': (permission_user_edit,), @@ -95,16 +98,28 @@ class APIUserView(generics.RetrieveUpdateDestroyAPIView): serializer_class = UserSerializer -class APIUserGroupList(generics.ListCreateAPIView): +class APIUserGroupList(ExternalObjectMixin, generics.ListCreateAPIView): """ get: Returns a list of all the groups to which an user belongs. post: Add a user to a list of groups. """ + external_object_pk_url_kwarg = 'user_pk' + filter_backends = (MayanObjectPermissionsFilter,) mayan_object_permissions = { - 'GET': (permission_user_view,), - 'POST': (permission_user_edit,) + 'GET': (permission_group_view,), + 'POST': (permission_group_edit,) } - permission_classes = (MayanPermission,) + + def get_external_object_permission(self): + if self.request.method == 'POST': + return permission_user_edit + else: + return permission_user_view + + def get_external_object_queryset(self): + return get_user_model().objects.exclude(is_staff=True).exclude( + is_superuser=True + ) def get_serializer(self, *args, **kwargs): if not self.request: @@ -113,10 +128,10 @@ class APIUserGroupList(generics.ListCreateAPIView): return super(APIUserGroupList, self).get_serializer(*args, **kwargs) def get_serializer_class(self): - if self.request.method == 'GET': + if self.request.method == 'POST': + return UserSerializer + else: return GroupSerializer - elif self.request.method == 'POST': - return UserGroupListSerializer def get_serializer_context(self): """ @@ -133,26 +148,10 @@ class APIUserGroupList(generics.ListCreateAPIView): return context def get_queryset(self): - user = self.get_user() - - return AccessControlList.objects.filter_by_access( - permission_group_view, self.request.user, - queryset=user.groups.order_by('id') - ) + return self.get_user().groups.order_by('id') def get_user(self): - if self.request.method == 'GET': - permission = permission_user_view - else: - permission = permission_user_edit - - user = get_object_or_404(klass=get_user_model(), pk=self.kwargs['pk']) - - AccessControlList.objects.check_access( - permissions=(permission,), user=self.request.user, - obj=user - ) - return user + return self.get_external_object() def perform_create(self, serializer): - serializer.save(user=self.get_user(), _user=self.request.user) + return serializer.save(user=self.get_object(), _user=self.request.user) diff --git a/mayan/apps/user_management/apps.py b/mayan/apps/user_management/apps.py index 875fc8066c..2ce6953c21 100644 --- a/mayan/apps/user_management/apps.py +++ b/mayan/apps/user_management/apps.py @@ -40,7 +40,7 @@ from .permissions import ( permission_user_view ) from .search import * # NOQA -from .utils import get_groups, get_users +from .utils import lookup_get_groups, lookup_get_users class UserManagementApp(MayanAppConfig): @@ -65,15 +65,15 @@ class UserManagementApp(MayanAppConfig): MetadataLookup( description=_('All the groups.'), name='groups', - value=get_groups + value=lookup_get_groups ) MetadataLookup( description=_('All the users.'), name='users', - value=get_users + value=lookup_get_users ) ModelEventType.register( - model=User, event_types=(event_user_edited,) + event_types=(event_user_edited,), model=User ) ModelPermission.register( @@ -129,7 +129,9 @@ class UserManagementApp(MayanAppConfig): menu_list_facet.bind_links( links=( - link_acl_list, link_group_members, + link_acl_list, link_events_for_object, + link_object_event_types_user_subcriptions_list, + link_group_members, ), sources=(Group,) ) @@ -138,8 +140,7 @@ class UserManagementApp(MayanAppConfig): link_acl_list, link_events_for_object, link_object_event_types_user_subcriptions_list, link_user_groups, - ), - sources=(User,) + ), sources=(User,) ) menu_multi_item.bind_links( @@ -147,12 +148,10 @@ class UserManagementApp(MayanAppConfig): sources=('user_management:user_list',) ) menu_object.bind_links( - links=(link_group_edit,), - sources=(Group,) + links=(link_group_edit,), sources=(Group,) ) menu_object.bind_links( - links=(link_group_delete,), position=99, - sources=(Group,) + links=(link_group_delete,), position=99, sources=(Group,) ) menu_object.bind_links( links=( diff --git a/mayan/apps/user_management/events.py b/mayan/apps/user_management/events.py index 571e1427aa..2d18763e36 100644 --- a/mayan/apps/user_management/events.py +++ b/mayan/apps/user_management/events.py @@ -5,7 +5,7 @@ from django.utils.translation import ugettext_lazy as _ from mayan.apps.events import EventTypeNamespace namespace = EventTypeNamespace( - name='user_management', label=_('User management') + label=_('User management'), name='user_management' ) event_group_created = namespace.add_event_type( diff --git a/mayan/apps/user_management/icons.py b/mayan/apps/user_management/icons.py index cf942721d5..934894b748 100644 --- a/mayan/apps/user_management/icons.py +++ b/mayan/apps/user_management/icons.py @@ -16,7 +16,7 @@ icon_user_delete = Icon(driver_name='fontawesome', symbol='times') icon_user_edit = Icon(driver_name='fontawesome', symbol='pencil-alt') icon_user_list = Icon(driver_name='fontawesome', symbol='user') icon_user_multiple_delete = icon_user_delete +icon_user_multiple_set_password = Icon(driver_name='fontawesome', symbol='key') icon_user_set_options = Icon(driver_name='fontawesome', symbol='cog') icon_user_set_password = Icon(driver_name='fontawesome', symbol='key') icon_user_setup = Icon(driver_name='fontawesome', symbol='user') -icon_user_multiple_set_password = icon_user_set_password diff --git a/mayan/apps/user_management/links.py b/mayan/apps/user_management/links.py index ed75f17ab8..bab25debcc 100644 --- a/mayan/apps/user_management/links.py +++ b/mayan/apps/user_management/links.py @@ -38,24 +38,23 @@ link_group_create = Link( text=_('Create new group'), view='user_management:group_create' ) link_group_delete = Link( - args='object.id', icon_class=icon_group_delete, + icon_class=icon_group_delete, kwargs={'group_id': 'object.pk'}, permission=permission_group_delete, tags='dangerous', - text=_('Delete'), view='user_management:group_delete', + text=_('Delete'), view='user_management:group_delete' ) link_group_edit = Link( - args='object.id', icon_class=icon_group_edit, + icon_class=icon_group_edit, kwargs={'group_id': 'object.pk'}, permission=permission_group_edit, text=_('Edit'), - view='user_management:group_edit', + view='user_management:group_edit' ) link_group_list = Link( icon_class=icon_group_list, permission=permission_group_view, - text=_('Groups'), - view='user_management:group_list' + text=_('Groups'), view='user_management:group_list' ) link_group_members = Link( - args='object.id', icon_class=icon_group_members, + icon_class=icon_group_members, kwargs={'group_id': 'object.pk'}, permission=permission_group_edit, text=_('Users'), - view='user_management:group_members', + view='user_management:group_members' ) link_group_setup = Link( icon_class=icon_group_setup, permission=permission_group_view, @@ -66,19 +65,19 @@ link_user_create = Link( text=_('Create new user'), view='user_management:user_create' ) link_user_delete = Link( - args='object.id', icon_class=icon_user_delete, + icon_class=icon_user_delete, kwargs={'user_id': 'object.pk'}, permission=permission_user_delete, tags='dangerous', text=_('Delete'), - view='user_management:user_delete', + view='user_management:user_delete' ) link_user_edit = Link( - args='object.id', icon_class=icon_user_edit, + icon_class=icon_user_edit, kwargs={'user_id': 'object.pk'}, permission=permission_user_edit, text=_('Edit'), - view='user_management:user_edit', + view='user_management:user_edit' ) link_user_groups = Link( - args='object.id', condition=condition_is_not_superuser, - icon_class=icon_group, permission=permission_user_edit, - text=_('Groups'), view='user_management:user_groups', + condition=condition_is_not_superuser, icon_class=icon_group, + kwargs={'user_id': 'object.pk'}, permission=permission_user_edit, + text=_('Groups'), view='user_management:user_groups' ) link_user_list = Link( icon_class=icon_user_list, permission=permission_user_view, @@ -95,14 +94,14 @@ link_user_multiple_set_password = Link( view='user_management:user_multiple_set_password' ) link_user_set_options = Link( - args='object.id', icon_class=icon_user_set_options, + icon_class=icon_user_set_options, kwargs={'user_id': 'object.pk'}, permission=permission_user_edit, text=_('User options'), - view='user_management:user_options', + view='user_management:user_options' ) link_user_set_password = Link( - args='object.id', icon_class=icon_user_set_password, + icon_class=icon_user_set_password, kwargs={'user_id': 'object.pk'}, permission=permission_user_edit, text=_('Set password'), - view='user_management:user_set_password', + view='user_management:user_set_password' ) link_user_setup = Link( icon_class=icon_user_setup, permission=permission_user_view, diff --git a/mayan/apps/user_management/methods.py b/mayan/apps/user_management/methods.py index 06baed3e71..23bd71b0a8 100644 --- a/mayan/apps/user_management/methods.py +++ b/mayan/apps/user_management/methods.py @@ -4,4 +4,6 @@ from django.shortcuts import reverse def method_get_absolute_url(self): - return reverse(viewname='user_management:user_details', args=(self.pk,)) + return reverse( + viewname='user_management:user_details', kwargs={'user_id': self.pk} + ) diff --git a/mayan/apps/user_management/models.py b/mayan/apps/user_management/models.py index 878a8c1bc6..a0a98a5713 100644 --- a/mayan/apps/user_management/models.py +++ b/mayan/apps/user_management/models.py @@ -18,8 +18,9 @@ class UserOptions(models.Model): to=settings.AUTH_USER_MODEL, unique=True, verbose_name=_('User') ) block_password_change = models.BooleanField( - default=False, - verbose_name=_('Forbid this user from changing their password.') + default=False, verbose_name=_( + 'Forbid this user from changing their password.' + ) ) objects = UserOptionsManager() diff --git a/mayan/apps/user_management/permissions.py b/mayan/apps/user_management/permissions.py index 944bbc3ef3..08328a0d24 100644 --- a/mayan/apps/user_management/permissions.py +++ b/mayan/apps/user_management/permissions.py @@ -4,29 +4,31 @@ from django.utils.translation import ugettext_lazy as _ from mayan.apps.permissions import PermissionNamespace -namespace = PermissionNamespace(label=_('User management'), name='user_management') +namespace = PermissionNamespace( + label=_('User management'), name='user_management' +) permission_group_create = namespace.add_permission( - name='group_create', label=_('Create new groups') + label=_('Create new groups'), name='group_create' ) permission_group_delete = namespace.add_permission( - name='group_delete', label=_('Delete existing groups') + label=_('Delete existing groups'), name='group_delete' ) permission_group_edit = namespace.add_permission( - name='group_edit', label=_('Edit existing groups') + label=_('Edit existing groups'), name='group_edit' ) permission_group_view = namespace.add_permission( - name='group_view', label=_('View existing groups') + label=_('View existing groups'), name='group_view' ) permission_user_create = namespace.add_permission( - name='user_create', label=_('Create new users') + label=_('Create new users'), name='user_create' ) permission_user_delete = namespace.add_permission( - name='user_delete', label=_('Delete existing users') + label=_('Delete existing users'), name='user_delete' ) permission_user_edit = namespace.add_permission( - name='user_edit', label=_('Edit existing users') + label=_('Edit existing users'), name='user_edit' ) permission_user_view = namespace.add_permission( - name='user_view', label=_('View existing users') + label=_('View existing users'), name='user_view' ) diff --git a/mayan/apps/user_management/search.py b/mayan/apps/user_management/search.py index 6ebf50546b..370b733fb7 100644 --- a/mayan/apps/user_management/search.py +++ b/mayan/apps/user_management/search.py @@ -32,8 +32,7 @@ user_search.add_model_field( ) group_search = SearchModel( - app_label='auth', model_name='Group', - permission=permission_group_view, + app_label='auth', model_name='Group', permission=permission_group_view, serializer_path='user_management.serializers.GroupSerializer' ) diff --git a/mayan/apps/user_management/serializers.py b/mayan/apps/user_management/serializers.py index 559b2c6969..6cd8205d46 100644 --- a/mayan/apps/user_management/serializers.py +++ b/mayan/apps/user_management/serializers.py @@ -11,7 +11,7 @@ from rest_framework.exceptions import ValidationError from mayan.apps.acls.models import AccessControlList -from .permissions import permission_group_view +from .permissions import permission_group_edit, permission_group_view class GroupSerializer(serializers.HyperlinkedModelSerializer): @@ -19,7 +19,10 @@ class GroupSerializer(serializers.HyperlinkedModelSerializer): class Meta: extra_kwargs = { - 'url': {'view_name': 'rest_api:group-detail'} + 'url': { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'group_pk', + 'view_name': 'rest_api:group-detail' + } } fields = ('id', 'name', 'url', 'users_count') model = Group @@ -28,41 +31,13 @@ class GroupSerializer(serializers.HyperlinkedModelSerializer): return instance.user_set.count() -class UserGroupListSerializer(serializers.Serializer): - group_pk_list = serializers.CharField( +class UserSerializer(serializers.HyperlinkedModelSerializer): + groups = GroupSerializer(many=True, read_only=True, required=False) + groups_pk_list = serializers.CharField( help_text=_( 'Comma separated list of group primary keys to assign this ' 'user to.' - ) - ) - - def create(self, validated_data): - validated_data['user'].groups.clear() - try: - pk_list = validated_data['group_pk_list'].split(',') - - for group in Group.objects.filter(pk__in=pk_list): - try: - AccessControlList.objects.check_access( - permissions=(permission_group_view,), - user=self.context['request'].user, obj=group - ) - except PermissionDenied: - pass - else: - validated_data['user'].groups.add(group) - except Exception as exception: - raise ValidationError(exception) - - return {'group_pk_list': validated_data['group_pk_list']} - - -class UserSerializer(serializers.HyperlinkedModelSerializer): - groups = GroupSerializer(many=True, read_only=True) - groups_pk_list = serializers.CharField( - help_text=_( - 'List of group primary keys to which to add the user.' - ), required=False + ), required=False, write_only=True ) password = serializers.CharField( required=False, style={'input_type': 'password'}, write_only=True @@ -70,7 +45,10 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): class Meta: extra_kwargs = { - 'url': {'view_name': 'rest_api:user-detail'} + 'url': { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'user_pk', + 'view_name': 'rest_api:user-detail' + } } fields = ( 'first_name', 'date_joined', 'email', 'groups', 'groups_pk_list', @@ -81,13 +59,19 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): read_only_fields = ('groups', 'is_active', 'last_login', 'date_joined') write_only_fields = ('password', 'group_pk_list') - def _add_groups(self, instance): - instance.groups.add( - *Group.objects.filter(pk__in=self.groups_pk_list.split(',')) + def _add_groups(self, instance, groups_pk_list): + instance.groups.clear() + + queryset = AccessControlList.objects.restrict_queryset( + permission=permission_group_edit, + queryset=Group.objects.filter(pk__in=groups_pk_list.split(',')), + user=self.context['request'].user ) + instance.groups.add(*queryset) + def create(self, validated_data): - self.groups_pk_list = validated_data.pop('groups_pk_list', '') + groups_pk_list = validated_data.pop('groups_pk_list', '') password = validated_data.pop('password', None) instance = super(UserSerializer, self).create(validated_data) @@ -95,13 +79,13 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): instance.set_password(password) instance.save() - if self.groups_pk_list: - self._add_groups(instance=instance) + if groups_pk_list: + self._add_groups(instance=instance, groups_pk_list=groups_pk_list) return instance def update(self, instance, validated_data): - self.groups_pk_list = validated_data.pop('groups_pk_list', '') + groups_pk_list = validated_data.pop('groups_pk_list', '') if 'password' in validated_data: instance.set_password(validated_data['password']) @@ -109,9 +93,8 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): instance = super(UserSerializer, self).update(instance, validated_data) - if self.groups_pk_list: - instance.groups.clear() - self._add_groups(instance=instance) + if groups_pk_list: + self._add_groups(instance=instance, groups_pk_list=groups_pk_list) return instance diff --git a/mayan/apps/user_management/tests/__init__.py b/mayan/apps/user_management/tests/__init__.py index 124ec460a6..33ff9e78ba 100644 --- a/mayan/apps/user_management/tests/__init__.py +++ b/mayan/apps/user_management/tests/__init__.py @@ -1 +1,2 @@ from .literals import * # NOQA +from .mixins import * # NOQA diff --git a/mayan/apps/user_management/tests/literals.py b/mayan/apps/user_management/tests/literals.py index 0cc6ddadc8..9b9b7db24a 100644 --- a/mayan/apps/user_management/tests/literals.py +++ b/mayan/apps/user_management/tests/literals.py @@ -5,26 +5,28 @@ __all__ = ( 'TEST_USER_PASSWORD', 'TEST_USER_PASSWORD_EDITED', 'TEST_USER_USERNAME' ) -TEST_CASE_ADMIN_EMAIL = 'admin@example.com' -TEST_CASE_ADMIN_PASSWORD = 'test admin password' -TEST_CASE_ADMIN_USERNAME = 'test_admin' +TEST_CASE_ADMIN_EMAIL = 'case_admin@example.com' +TEST_CASE_ADMIN_PASSWORD = 'test case admin password' +TEST_CASE_ADMIN_USERNAME = 'test_case_admin' -TEST_CASE_GROUP_NAME = 'test group' -TEST_CASE_USER_EMAIL = 'user@example.com' -TEST_CASE_USER_PASSWORD = 'test user password' -TEST_CASE_USER_USERNAME = 'test_user' +TEST_CASE_GROUP_NAME = 'test case group' + +TEST_CASE_USER_EMAIL = 'test_case_user@example.com' +TEST_CASE_USER_PASSWORD = 'test case user password' +TEST_CASE_USER_USERNAME = 'test_case_user' TEST_GROUP_NAME = 'test group' TEST_GROUP_NAME_EDITED = 'test group edited' TEST_GROUP_2_NAME = 'test group 2' TEST_GROUP_2_NAME_EDITED = 'test group 2 edited' -TEST_USER_EMAIL = 'user@example.com' + +TEST_USER_EMAIL = 'testuser@example.com' TEST_USER_PASSWORD = 'test user password' TEST_USER_PASSWORD_EDITED = 'test user password edited' TEST_USER_USERNAME = 'test_user' TEST_USER_USERNAME_EDITED = 'test_user_edited' -TEST_USER_2_EMAIL = 'user2@example.com' +TEST_USER_2_EMAIL = 'testuser2@example.com' TEST_USER_2_PASSWORD = 'test user 2 password' TEST_USER_2_PASSWORD_EDITED = 'test user 2 password edited' TEST_USER_2_USERNAME = 'test_user_2' diff --git a/mayan/apps/user_management/tests/mixins.py b/mayan/apps/user_management/tests/mixins.py index 81643866ef..cd2c26ec43 100644 --- a/mayan/apps/user_management/tests/mixins.py +++ b/mayan/apps/user_management/tests/mixins.py @@ -5,97 +5,117 @@ from django.contrib.auth.models import Group from .literals import ( TEST_CASE_ADMIN_EMAIL, TEST_CASE_ADMIN_PASSWORD, TEST_CASE_ADMIN_USERNAME, - TEST_CASE_GROUP_NAME, TEST_CASE_USER_EMAIL, TEST_CASE_USER_PASSWORD, - TEST_CASE_USER_USERNAME, TEST_GROUP_NAME, TEST_GROUP_2_NAME, - TEST_GROUP_2_NAME_EDITED, TEST_USER_2_EMAIL, TEST_USER_2_PASSWORD, - TEST_USER_EMAIL, TEST_USER_USERNAME, TEST_USER_PASSWORD, - TEST_USER_2_USERNAME, TEST_USER_2_USERNAME_EDITED + TEST_CASE_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_CASE_USER_EMAIL, + TEST_CASE_USER_PASSWORD, TEST_CASE_USER_USERNAME, TEST_GROUP_NAME, + TEST_USER_EMAIL, TEST_USER_USERNAME, TEST_USER_USERNAME_EDITED, + TEST_USER_PASSWORD ) +__all__ = ('GroupTestMixin', 'UserTestCaseMixin', 'UserTestMixin') + class UserTestCaseMixin(object): - auto_login_admin = False + """ + This TestCaseMixin is used to create an user and group to execute the + test case, these are used to just create an identity which is required by + most of the code in the project, these are not meant to be acted upon + (edited, deleted, etc). To create a test users or groups to modify, use + the UserTestMixin instead and the respective test_user and test_group. + The user and group created by this mixin will be prepended with + _test_case_{...}. The _test_case_user and _test_case_group are meant + to be used by other test case mixins like the ACLs test case mixin which + adds shorthand methods to create ACL entries to test access control. + """ + auto_login_superuser = False auto_login_user = True + create_test_case_superuser = False + create_test_case_user = True def setUp(self): super(UserTestCaseMixin, self).setUp() - if self.auto_login_user: - self._test_case_user = get_user_model().objects.create_user( - username=TEST_CASE_USER_USERNAME, email=TEST_CASE_USER_EMAIL, - password=TEST_CASE_USER_PASSWORD - ) - self.login_user() - self._test_case_group = Group.objects.create(name=TEST_GROUP_NAME) + if self.create_test_case_user: + self._create_test_case_user() + self._create_test_case_group() self._test_case_group.user_set.add(self._test_case_user) - elif self.auto_login_admin: - self._test_case_admin_user = get_user_model().objects.create_superuser( - username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL, - password=TEST_CASE_ADMIN_PASSWORD - ) - self.login_admin_user() + + if self.auto_login_user: + self.login_user() + + elif self.create_test_case_superuser: + self._create_test_case_superuser() + + if self.auto_login_superuser: + self.login_superuser() def tearDown(self): self.client.logout() super(UserTestCaseMixin, self).tearDown() + def _create_test_case_group(self): + self._test_case_group = Group.objects.create(name=TEST_CASE_GROUP_NAME) + + def _create_test_case_superuser(self): + self._test_case_superuser = get_user_model().objects.create_superuser( + username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL, + password=TEST_CASE_ADMIN_PASSWORD + ) + + def _create_test_case_user(self): + self._test_case_user = get_user_model().objects.create_user( + username=TEST_CASE_USER_USERNAME, email=TEST_CASE_USER_EMAIL, + password=TEST_CASE_USER_PASSWORD + ) + def login(self, *args, **kwargs): logged_in = self.client.login(*args, **kwargs) return logged_in - def login_user(self): - self.login( - username=TEST_CASE_USER_USERNAME, password=TEST_CASE_USER_PASSWORD - ) - - def login_admin_user(self): + def login_superuser(self): self.login( username=TEST_CASE_ADMIN_USERNAME, password=TEST_CASE_ADMIN_PASSWORD ) + def login_user(self): + self.login( + username=TEST_CASE_USER_USERNAME, password=TEST_CASE_USER_PASSWORD + ) + def logout(self): self.client.logout() -class UserTestMixin(object): +class GroupTestMixin(object): def _create_test_group(self): - self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME) + self.test_group = Group.objects.create(name=TEST_GROUP_NAME) def _edit_test_group(self): - self.test_group.name = TEST_GROUP_2_NAME_EDITED + self.test_group.name = TEST_GROUP_NAME_EDITED self.test_group.save() - def _create_test_user(self): - self.test_user = get_user_model().objects.create( - username=TEST_USER_2_USERNAME, email=TEST_USER_2_EMAIL, - password=TEST_USER_2_PASSWORD - ) - - # Group views - def _request_test_group_create_view(self): reponse = self.post( viewname='user_management:group_create', data={ - 'name': TEST_GROUP_2_NAME + 'name': TEST_GROUP_NAME } ) - self.test_group = Group.objects.filter(name=TEST_GROUP_2_NAME).first() + self.test_group = Group.objects.filter(name=TEST_GROUP_NAME).first() return reponse def _request_test_group_delete_view(self): return self.post( viewname='user_management:group_delete', kwargs={ - 'group_pk': self.test_group.pk + 'group_id': self.test_group.pk } ) def _request_test_group_edit_view(self): return self.post( viewname='user_management:group_edit', kwargs={ - 'group_pk': self.test_group.pk + 'group_id': self.test_group.pk }, data={ - 'name': TEST_GROUP_2_NAME_EDITED + 'name': TEST_GROUP_NAME_EDITED } ) @@ -105,41 +125,65 @@ class UserTestMixin(object): def _request_test_group_members_view(self): return self.get( viewname='user_management:group_members', - kwargs={'group_pk': self.test_group.pk} + kwargs={'group_id': self.test_group.pk} ) - # User views + +class UserTestMixin(object): + def _create_test_superuser(self): + self.test_superuser = get_user_model().objects.create_superuser( + username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL, + password=TEST_CASE_ADMIN_PASSWORD + ) + + def _create_test_user(self): + self.test_user = get_user_model().objects.create( + username=TEST_USER_USERNAME, email=TEST_USER_EMAIL, + password=TEST_USER_PASSWORD + ) + + def _request_test_superuser_delete_view(self): + return self.post( + viewname='user_management:user_delete', + kwargs={'user_id': self.test_superuser.pk} + ) + + def _request_test_superuser_detail_view(self): + return self.get( + viewname='user_management:user_details', + kwargs={'user_id': self.test_superuser.pk} + ) def _request_test_user_create_view(self): reponse = self.post( viewname='user_management:user_create', data={ - 'username': TEST_USER_2_USERNAME, - 'password': TEST_USER_2_PASSWORD + 'username': TEST_USER_USERNAME, + 'password': TEST_USER_PASSWORD } ) self.test_user = get_user_model().objects.filter( - username=TEST_USER_2_USERNAME + username=TEST_USER_USERNAME ).first() return reponse def _request_test_user_delete_view(self): return self.post( viewname='user_management:user_delete', - kwargs={'user_pk': self.test_user.pk} + kwargs={'user_id': self.test_user.pk} ) def _request_test_user_edit_view(self): return self.post( viewname='user_management:user_edit', kwargs={ - 'user_pk': self.test_user.pk + 'user_id': self.test_user.pk }, data={ - 'username': TEST_USER_2_USERNAME_EDITED + 'username': TEST_USER_USERNAME_EDITED } ) def _request_test_user_groups_view(self): return self.get( viewname='user_management:user_groups', - kwargs={'user_pk': self.test_user.pk} + kwargs={'user_id': self.test_user.pk} ) diff --git a/mayan/apps/user_management/tests/test_api.py b/mayan/apps/user_management/tests/test_api.py index e1cb7cc181..d30e474828 100644 --- a/mayan/apps/user_management/tests/test_api.py +++ b/mayan/apps/user_management/tests/test_api.py @@ -22,9 +22,9 @@ from .literals import ( from .mixins import UserTestMixin -class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): +class UserAPITestCase(UserTestMixin, BaseAPITestCase): def setUp(self): - super(UserManagementUserAPITestCase, self).setUp() + super(UserAPITestCase, self).setUp() self.login_user() def _request_api_test_user_create(self): @@ -54,22 +54,25 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): viewname='rest_api:user-list', data={ 'email': TEST_USER_2_EMAIL, 'password': TEST_USER_2_PASSWORD, 'username': TEST_USER_2_USERNAME, - 'groups_pk_list': self.test_groups_pk_list + 'groups_id_list': self.test_groups_id_list } ) + """ def test_user_create_with_group_no_permission(self): self._create_test_group() - self.test_groups_pk_list = '{}'.format(self.test_group.pk) + self.test_groups_id_list = '{}'.format(self.test_group.pk) response = self._request_api_create_test_user_with_extra_data() self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_user_create_with_group_with_permission(self): + def test_user_create_with_group_with_user_access(self): self._create_test_group() - self.test_groups_pk_list = '{}'.format(self.test_group.pk) + self.test_groups_id_list = '{}'.format(self.test_group.pk) - self.grant_permission(permission=permission_user_create) + self.grant_access( + obj=self.test_user, permission=permission_user_create + ) response = self._request_api_create_test_user_with_extra_data() self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -78,22 +81,57 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self.assertEqual(user.username, TEST_USER_2_USERNAME) self.assertQuerysetEqual(user.groups.all(), (repr(self.test_group),)) + + def test_user_create_with_group_with_user_access(self): + self._create_test_group() + self.test_groups_id_list = '{}'.format(self.test_group.pk) + + self.grant_access( + obj=self.test_user, permission=permission_user_create + ) + response = self._request_api_create_test_user_with_extra_data() + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + user = get_user_model().objects.get(pk=response.data['id']) + self.assertEqual(user.username, TEST_USER_2_USERNAME) + self.assertQuerysetEqual(user.groups.all(), (repr(self.test_group),)) + """ + def test_user_create_with_groups_no_permission(self): group_1 = Group.objects.create(name='test group 1') group_2 = Group.objects.create(name='test group 2') - self.test_groups_pk_list = '{},{}'.format(group_1.pk, group_2.pk) + self.test_groups_id_list = '{},{}'.format(group_1.pk, group_2.pk) response = self._request_api_create_test_user_with_extra_data() self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_user_create_with_groups_with_permission(self): + def test_user_create_with_groups_with_user_permission(self): group_1 = Group.objects.create(name='test group 1') group_2 = Group.objects.create(name='test group 2') - self.test_groups_pk_list = '{},{}'.format(group_1.pk, group_2.pk) + self.test_groups_id_list = '{},{}'.format(group_1.pk, group_2.pk) self.grant_permission(permission=permission_user_create) response = self._request_api_create_test_user_with_extra_data() self.assertEqual(response.status_code, status.HTTP_201_CREATED) + user = get_user_model().objects.get(pk=response.data['id']) + self.assertEqual(user.username, TEST_USER_2_USERNAME) + #self.assertQuerysetEqual( + # user.groups.all().order_by('name'), (repr(group_1), repr(group_2)) + #) + self.assertEqual(user.groups.count(), 0) + + def test_user_create_with_groups_with_full_access(self): + group_1 = Group.objects.create(name='test group 1') + group_2 = Group.objects.create(name='test group 2') + self.test_groups_id_list = '{},{}'.format(group_1.pk, group_2.pk) + self.grant_permission(permission=permission_user_create) + self.grant_access(obj=group_1, permission=permission_group_edit) + self.grant_access(obj=group_2, permission=permission_group_edit) + response = self._request_api_create_test_user_with_extra_data() + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + user = get_user_model().objects.get(pk=response.data['id']) self.assertEqual(user.username, TEST_USER_2_USERNAME) self.assertQuerysetEqual( @@ -105,10 +143,6 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def test_user_create_login(self): self._create_test_user() - self.login( - username=TEST_USER_2_USERNAME, password=TEST_USER_2_PASSWORD - ) - self.assertTrue( self.login( username=TEST_USER_2_USERNAME, password=TEST_USER_2_PASSWORD @@ -117,15 +151,18 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): # User password change - def test_user_create_login_password_change_no_access(self): - self._create_test_user() - - self.patch( - viewname='rest_api:user-detail', args=(self.test_user.pk,), data={ + def _request_api_user_password_change(self): + return self.patch( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, data={ 'password': TEST_USER_2_PASSWORD_EDITED, } ) + def test_user_create_login_password_change_no_access(self): + self._create_test_user() + self._request_api_user_password_change() + self.assertFalse( self.client.login( username=TEST_USER_2_USERNAME, @@ -136,12 +173,8 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def test_user_create_login_password_change_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) - self.patch( - viewname='rest_api:user-detail', args=(self.test_user.pk,), data={ - 'password': TEST_USER_2_PASSWORD_EDITED, - } - ) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + self._request_api_user_password_change() self.assertTrue( self.client.login( @@ -154,7 +187,8 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def _request_api_test_user_edit_via_put(self): return self.put( - viewname='rest_api:user-detail', args=(self.test_user.pk,), + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, data={'username': TEST_USER_2_USERNAME_EDITED} ) @@ -162,14 +196,14 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self._create_test_user() response = self._request_api_test_user_edit_via_put() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() self.assertEqual(self.test_user.username, TEST_USER_2_USERNAME) def test_user_edit_via_put_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_api_test_user_edit_via_put() self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -179,7 +213,8 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def _request_api_test_user_edit_via_patch(self): return self.patch( - viewname='rest_api:user-detail', args=(self.test_user.pk,), + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, data={'username': TEST_USER_2_USERNAME_EDITED} ) @@ -187,14 +222,14 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self._create_test_user() response = self._request_api_test_user_edit_via_patch() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() self.assertEqual(self.test_user.username, TEST_USER_2_USERNAME) def test_user_edit_via_patch_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_api_test_user_edit_via_patch() self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -204,8 +239,9 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def _request_api_test_user_edit_via_patch_with_extra_data(self): return self.patch( - viewname='rest_api:user-detail', args=(self.test_user.pk,), - data={'groups_pk_list': '{}'.format(self.test_group.pk)} + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, + data={'groups_id_list': '{}'.format(self.test_group.pk)} ) def test_user_edit_add_groups_via_patch_no_access(self): @@ -214,7 +250,7 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): response = self._request_api_test_user_edit_via_patch_with_extra_data() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() self.assertEqual(self.test_user.username, TEST_USER_2_USERNAME) @@ -226,7 +262,8 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def test_user_edit_add_groups_via_patch_with_access(self): self._create_test_group() self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + self.grant_access(obj=self.test_group, permission=permission_group_edit) response = self._request_api_test_user_edit_via_patch_with_extra_data() self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -242,13 +279,14 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def _request_api_test_user_delete(self): return self.delete( - viewname='rest_api:user-detail', args=(self.test_user.pk,) + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk} ) def test_user_delete_no_access(self): self._create_test_user() response = self._request_api_test_user_delete() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertTrue( get_user_model().objects.filter(pk=self.test_user.pk).exists() @@ -257,7 +295,7 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): def test_user_delete_with_access(self): self._create_test_user() self.grant_access( - permission=permission_user_delete, obj=self.test_user + obj=self.test_user, permission=permission_user_delete ) response = self._request_api_test_user_delete() self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -266,11 +304,12 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): get_user_model().objects.filter(pk=self.test_user.pk).exists() ) - # User view + # User group listview def _request_api_test_user_group_view(self): return self.get( - viewname='rest_api:users-group-list', args=(self.test_user.pk,) + viewname='rest_api:users-group-list', + kwargs={'user_id': self.test_user.pk} ) def test_user_group_list_no_access(self): @@ -278,13 +317,13 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self._create_test_user() self.test_user.groups.add(group) response = self._request_api_test_user_group_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_user_group_list_with_user_access(self): group = Group.objects.create(name=TEST_GROUP_2_NAME) self._create_test_user() self.test_user.groups.add(group) - self.grant_access(permission=permission_user_view, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_view) response = self._request_api_test_user_group_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['count'], 0) @@ -294,46 +333,44 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self._create_test_user() self.test_user.groups.add(self.test_group) self.grant_access( - permission=permission_group_view, obj=self.test_group + obj=self.test_group, permission=permission_group_view ) response = self._request_api_test_user_group_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_user_group_list_with_access(self): self._create_test_group() self._create_test_user() self.test_user.groups.add(self.test_group) - self.grant_access(permission=permission_user_view, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_view) self.grant_access( - permission=permission_group_view, obj=self.test_group + obj=self.test_group, permission=permission_group_view ) response = self._request_api_test_user_group_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['count'], 1) def _request_api_test_user_group_add(self): - return self.post( - viewname='rest_api:users-group-list', args=(self.test_user.pk,), - data={'group_pk_list': '{}'.format(self.test_group.pk)} + return self.patch( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, + data={'group_id_list': '{}'.format(self.test_group.pk)} ) def test_user_group_add_no_access(self): self._create_test_group() self._create_test_user() response = self._request_api_test_user_group_add() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() self.assertEqual(self.test_group.user_set.first(), None) def test_user_group_add_with_user_access(self): self._create_test_group() self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_api_test_user_group_add() - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - # FIXME: Should this endpoint return a 201 or a 200 since - # the user is being edited and there is not resource creation - # happening. + self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_user.refresh_from_db() self.assertEqual(self.test_group.user_set.first(), None) @@ -341,38 +378,32 @@ class UserManagementUserAPITestCase(UserTestMixin, BaseAPITestCase): self._create_test_group() self._create_test_user() self.grant_access( - permission=permission_group_view, obj=self.test_group + obj=self.test_group, permission=permission_group_edit ) response = self._request_api_test_user_group_add() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # FIXME: Should this endpoint return a 201 or a 200 since - # the user is being edited and there is not resource creation - # happening. + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() self.assertEqual(self.test_group.user_set.first(), None) - def test_user_group_add_with_access(self): + def test_user_group_add_with_full_access(self): self._create_test_group() self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) self.grant_access( - permission=permission_group_view, obj=self.test_group + obj=self.test_group, permission=permission_group_edit ) response = self._request_api_test_user_group_add() - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - # FIXME: Should this endpoint return a 201 or a 200 since - # the user is being edited and there is not resource creation - # happening. + self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_user.refresh_from_db() self.assertEqual(self.test_group.user_set.first(), self.test_user) -class UserManagementGroupAPITestCase(BaseAPITestCase): +class GroupAPITestCase(UserTestMixin, BaseAPITestCase): def setUp(self): - super(UserManagementGroupAPITestCase, self).setUp() + super(GroupAPITestCase, self).setUp() self.login_user() - def _request_api_test_group_create(self): + def _request_api_test_group_create_view(self): return self.post( viewname='rest_api:group-list', data={ 'name': TEST_GROUP_2_NAME @@ -380,7 +411,7 @@ class UserManagementGroupAPITestCase(BaseAPITestCase): ) def test_group_create_no_permission(self): - response = self._request_api_test_group_create() + response = self._request_api_test_group_create_view() self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertFalse( TEST_GROUP_2_NAME in list( @@ -390,7 +421,7 @@ class UserManagementGroupAPITestCase(BaseAPITestCase): def test_group_create_with_permission(self): self.grant_permission(permission=permission_group_create) - response = self._request_api_test_group_create() + response = self._request_api_test_group_create_view() self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertTrue( TEST_GROUP_2_NAME in list( @@ -398,43 +429,17 @@ class UserManagementGroupAPITestCase(BaseAPITestCase): ) ) - def _request_api_test_group_edit_via_patch(self): - return self.patch( - viewname='rest_api:group-detail', args=(self.test_group.pk,), - data={ - 'name': TEST_GROUP_2_NAME_EDITED - } - ) - - def test_group_edit_via_patch_no_access(self): - self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME) - response = self._request_api_test_group_edit_via_patch() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - self.test_group.refresh_from_db() - self.assertEqual(self.test_group.name, TEST_GROUP_2_NAME) - - def test_group_edit_via_patch_with_access(self): - self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME) - self.grant_access( - permission=permission_group_edit, obj=self.test_group - ) - response = self._request_api_test_group_edit_via_patch() - self.assertEqual(response.status_code, status.HTTP_200_OK) - - self.test_group.refresh_from_db() - self.assertEqual(self.test_group.name, TEST_GROUP_2_NAME_EDITED) - - def _request_api_test_group_delete(self): + def _request_api_test_group_delete_view(self): return self.delete( - viewname='rest_api:group-detail', args=(self.test_group.pk,) + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk} ) def test_group_delete_no_access(self): - self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME) - response = self._request_api_test_group_delete() + self._create_test_group() + response = self._request_api_test_group_delete_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertTrue( TEST_GROUP_2_NAME in list( Group.objects.values_list('name', flat=True) @@ -442,9 +447,11 @@ class UserManagementGroupAPITestCase(BaseAPITestCase): ) def test_group_delete_with_access(self): - self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME) - self.grant_access(permission=permission_group_delete, obj=self.test_group) - response = self._request_api_test_group_delete() + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_delete + ) + response = self._request_api_test_group_delete_view() self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse( @@ -452,3 +459,79 @@ class UserManagementGroupAPITestCase(BaseAPITestCase): Group.objects.values_list('name', flat=True) ) ) + + def _request_api_test_group_detail_view(self): + return self.get( + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk} + ) + + def test_group_detail_no_access(self): + self._create_test_group() + response = self._request_api_test_group_detail_view() + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertNotEqual( + self.test_group.name, response.data.get('name', None) + ) + + def test_group_detail_with_access(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_view + ) + response = self._request_api_test_group_detail_view() + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(self.test_group.name, response.data.get('name', None)) + + + def _request_api_test_group_edit_via_patch_view(self): + return self.patch( + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk}, + data={ + 'name': TEST_GROUP_2_NAME_EDITED + } + ) + + def test_group_edit_via_patch_no_access(self): + self._create_test_group() + response = self._request_api_test_group_edit_via_patch_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.test_group.refresh_from_db() + self.assertEqual(self.test_group.name, TEST_GROUP_2_NAME) + + def test_group_edit_via_patch_with_access(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_api_test_group_edit_via_patch_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.test_group.refresh_from_db() + self.assertEqual(self.test_group.name, TEST_GROUP_2_NAME_EDITED) + + def _request_api_test_group_list_view(self): + return self.get(viewname='rest_api:group-list') + + def test_group_list_no_access(self): + self._create_test_group() + response = self._request_api_test_group_list_view() + self.assertNotContains( + response=response, text=self.test_group.name, + status_code=status.HTTP_200_OK + ) + + def test_group_list_with_access(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_view + ) + response = self._request_api_test_group_list_view() + self.assertContains( + response=response, text=self.test_group.name, + status_code=status.HTTP_200_OK + ) diff --git a/mayan/apps/user_management/tests/test_events.py b/mayan/apps/user_management/tests/test_events.py index 9e4e2c4c73..e6d630c89f 100644 --- a/mayan/apps/user_management/tests/test_events.py +++ b/mayan/apps/user_management/tests/test_events.py @@ -4,47 +4,60 @@ from actstream.models import Action from mayan.apps.common.tests import GenericViewTestCase +from ..permissions import ( + permission_group_create, permission_group_edit, permission_user_create, + permission_user_edit +) + from ..events import ( event_group_created, event_group_edited, event_user_created, event_user_edited ) -from .mixins import UserTestMixin +from .mixins import GroupTestMixin, UserTestMixin -class GroupEventsTestCase(UserTestMixin, GenericViewTestCase): - auto_create_group = False - +class GroupEventsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): def test_group_create_event(self): - self.login_admin_user() Action.objects.all().delete() + + self.grant_permission( + permission=permission_group_create + ) self._request_test_group_create_view() self.assertEqual(Action.objects.last().target, self.test_group) self.assertEqual(Action.objects.last().verb, event_group_created.id) def test_group_edit_event(self): - self.login_admin_user() self._create_test_group() Action.objects.all().delete() + + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) self._request_test_group_edit_view() self.assertEqual(Action.objects.last().target, self.test_group) self.assertEqual(Action.objects.last().verb, event_group_edited.id) class UserEventsTestCase(UserTestMixin, GenericViewTestCase): - auto_create_group = False - def test_user_create_event(self): - self.login_admin_user() Action.objects.all().delete() + + self.grant_permission( + permission=permission_user_create + ) self._request_test_user_create_view() self.assertEqual(Action.objects.last().target, self.test_user) self.assertEqual(Action.objects.last().verb, event_user_created.id) def test_user_edit_event(self): - self.login_admin_user() self._create_test_user() Action.objects.all().delete() + + self.grant_access( + obj=self.test_user, permission=permission_user_edit + ) self._request_test_user_edit_view() self.assertEqual(Action.objects.last().target, self.test_user) self.assertEqual(Action.objects.last().verb, event_user_edited.id) diff --git a/mayan/apps/user_management/tests/test_models.py b/mayan/apps/user_management/tests/test_models.py index 7e96927453..e913a1262c 100644 --- a/mayan/apps/user_management/tests/test_models.py +++ b/mayan/apps/user_management/tests/test_models.py @@ -7,5 +7,5 @@ from .mixins import UserTestMixin class UserTestCase(UserTestMixin, BaseTestCase): def test_natural_keys(self): - self._create_user() + self._create_test_user() self._test_database_conversion('auth', 'user_management') diff --git a/mayan/apps/user_management/tests/test_views.py b/mayan/apps/user_management/tests/test_views.py index 12a8659fe0..a3bb41ddaa 100644 --- a/mayan/apps/user_management/tests/test_views.py +++ b/mayan/apps/user_management/tests/test_views.py @@ -6,70 +6,253 @@ from django.contrib.auth.models import Group from mayan.apps.common.tests import GenericViewTestCase from mayan.apps.documents.tests import GenericDocumentViewTestCase from mayan.apps.metadata.models import MetadataType -from mayan.apps.metadata.permissions import permission_metadata_document_edit +from mayan.apps.metadata.permissions import permission_document_metadata_edit from mayan.apps.metadata.tests.literals import ( TEST_METADATA_TYPE_LABEL, TEST_METADATA_TYPE_NAME, ) from ..permissions import ( - permission_user_create, permission_user_delete, permission_user_edit + permission_group_create, permission_group_delete, permission_group_edit, + permission_group_view, permission_user_create, permission_user_delete, + permission_user_edit, permission_user_view ) from .literals import ( - TEST_USER_PASSWORD_EDITED, TEST_USER_USERNAME, TEST_USER_2_USERNAME + TEST_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_USER_PASSWORD_EDITED, + TEST_USER_USERNAME ) -from .mixins import UserTestMixin +from .mixins import GroupTestMixin, UserTestMixin TEST_USER_TO_DELETE_USERNAME = 'user_to_delete' -class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): - def setUp(self): - super(UserManagementViewTestCase, self).setUp() - self.login_user() +class GroupViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): + def test_group_create_view_no_permission(self): + response = self._request_test_group_create_view() + self.assertEqual(response.status_code, 403) + self.assertEqual(Group.objects.count(), 1) - #def _request_test_user_create_view(self): - # return self.post( - # viewname='user_management:user_create', data={ - # 'username': TEST_USER_2_USERNAME - # } - # ) + def test_group_create_view_with_permission(self): + self.grant_permission(permission=permission_group_create) + response = self._request_test_group_create_view() + self.assertEqual(response.status_code, 302) + self.assertEqual(Group.objects.count(), 2) + def test_group_delete_view_no_permission(self): + self._create_test_group() + response = self._request_test_group_delete_view() + self.assertEqual(response.status_code, 404) + self.assertEqual(Group.objects.count(), 2) + + def test_group_delete_view_with_access(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_delete + ) + response = self._request_test_group_delete_view() + self.assertEqual(response.status_code, 302) + self.assertEqual(Group.objects.count(), 1) + + def test_group_edit_view_no_permission(self): + self._create_test_group() + response = self._request_test_group_edit_view() + self.assertEqual(response.status_code, 404) + self.test_group.refresh_from_db() + self.assertEqual(self.test_group.name, TEST_GROUP_NAME) + + def test_group_edit_view_with_access(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_test_group_edit_view() + self.assertEqual(response.status_code, 302) + self.test_group.refresh_from_db() + self.assertEqual(self.test_group.name, TEST_GROUP_NAME_EDITED) + + def test_group_list_view_no_permission(self): + self._create_test_group() + response = self._request_test_group_list_view() + self.assertNotContains( + response=response, text=self.test_group.name, status_code=200 + ) + + def test_group_list_view_with_permission(self): + self._create_test_group() + self.grant_access( + obj=self.test_group, permission=permission_group_view + ) + response = self._request_test_group_list_view() + self.assertContains( + response=response, text=self.test_group.name, status_code=200 + ) + + def test_group_members_view_no_permission(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + response = self._request_test_group_members_view() + self.assertEqual(response.status_code, 404) + + def test_group_members_view_with_group_access(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_test_group_members_view() + self.assertContains( + response=response, text=self.test_group.name, status_code=200 + ) + self.assertNotContains( + response=response, text=self.test_user.username, status_code=200 + ) + + def test_group_members_view_with_user_access(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + response = self._request_test_group_members_view() + self.assertNotContains( + response=response, text=self.test_group.name, status_code=404 + ) + + def test_group_members_view_with_full_access(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + response = self._request_test_group_members_view() + self.assertContains( + response=response, text=self.test_user.username, status_code=200 + ) + self.assertContains( + response=response, text=self.test_group.name, status_code=200 + ) + + +class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): def test_user_create_view_no_permission(self): + user_count = get_user_model().objects.count() + response = self._request_test_user_create_view() self.assertEqual(response.status_code, 403) - self.assertEqual(get_user_model().objects.count(), 2) - self.assertFalse(TEST_USER_2_USERNAME in get_user_model().objects.values_list('username', flat=True)) + + self.assertEqual(get_user_model().objects.count(), user_count) + self.assertFalse(TEST_USER_USERNAME in get_user_model().objects.values_list('username', flat=True)) def test_user_create_view_with_permission(self): + user_count = get_user_model().objects.count() + self.grant_permission(permission=permission_user_create) response = self._request_test_user_create_view() self.assertEqual(response.status_code, 302) - self.assertEqual(get_user_model().objects.count(), 3) - self.assertTrue(TEST_USER_2_USERNAME in get_user_model().objects.values_list('username', flat=True)) - def _request_user_groups_view(self): - return self.post( - viewname='user_management:user_groups', args=(self.test_user.pk,) + self.assertEqual(get_user_model().objects.count(), user_count + 1) + self.assertTrue(TEST_USER_USERNAME in get_user_model().objects.values_list('username', flat=True)) + + def test_user_delete_view_no_access(self): + self._create_test_user() + user_count = get_user_model().objects.count() + + response = self._request_test_user_delete_view() + self.assertEqual(response.status_code, 404) + + self.assertEqual(get_user_model().objects.count(), user_count) + + def test_user_delete_view_with_access(self): + self._create_test_user() + user_count = get_user_model().objects.count() + + self.grant_access( + obj=self.test_user, permission=permission_user_delete ) + response = self._request_test_user_delete_view() + self.assertEqual(response.status_code, 302) + + self.assertEqual(get_user_model().objects.count(), user_count - 1) + + def test_superuser_delete_view_with_access(self): + self._create_test_superuser() + + superuser_count = get_user_model().objects.filter(is_superuser=True).count() + self.grant_access( + obj=self.test_superuser, permission=permission_user_delete + ) + response = self._request_test_superuser_delete_view() + self.assertEqual(response.status_code, 404) + self.assertEqual( + get_user_model().objects.filter(is_superuser=True).count(), + superuser_count + ) + + def test_superuser_detail_view_with_access(self): + self._create_test_superuser() + + self.grant_access( + obj=self.test_superuser, permission=permission_user_view + ) + response = self._request_test_superuser_detail_view() + self.assertEqual(response.status_code, 404) def test_user_groups_view_no_permission(self): self._create_test_user() - response = self._request_user_groups_view() - self.assertEqual(response.status_code, 403) + self._create_test_group() + self.test_user.groups.add(self.test_group) + response = self._request_test_user_groups_view() + self.assertEqual(response.status_code, 404) - def test_user_groups_view_with_access(self): + def test_user_groups_view_with_group_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_test_user_groups_view() + self.assertNotContains( + response=response, text=self.test_user.username, status_code=404 + ) - response = self._request_user_groups_view() + def test_user_groups_view_with_user_access(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + response = self._request_test_user_groups_view() self.assertContains( response=response, text=self.test_user.username, status_code=200 ) + self.assertNotContains( + response=response, text=self.test_group.name, status_code=200 + ) + + def test_user_groups_view_with_full_access(self): + self._create_test_user() + self._create_test_group() + self.test_user.groups.add(self.test_group) + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + self.grant_access(obj=self.test_user, permission=permission_user_edit) + response = self._request_test_user_groups_view() + + self.assertContains( + response=response, text=self.test_user.username, status_code=200 + ) + self.assertContains( + response=response, text=self.test_group.name, status_code=200 + ) def _request_set_password_view(self, password): return self.post( - viewname='user_management:user_set_password', args=(self.test_user.pk,), + viewname='user_management:user_set_password', + kwargs={'user_id': self.test_user.pk}, data={ 'new_password1': password, 'new_password2': password } @@ -81,13 +264,14 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): password=TEST_USER_PASSWORD_EDITED ) - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) self.logout() with self.assertRaises(AssertionError): self.login( - username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED + username=self.test_user.username, + password=TEST_USER_PASSWORD_EDITED ) response = self.get(viewname='user_management:current_user_details') @@ -96,7 +280,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): def test_user_set_password_view_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_set_password_view( password=TEST_USER_PASSWORD_EDITED @@ -106,7 +290,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): self.logout() self.login( - username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED + username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD_EDITED ) response = self.get(viewname='user_management:current_user_details') @@ -128,13 +312,14 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): password=TEST_USER_PASSWORD_EDITED ) - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) self.logout() with self.assertRaises(AssertionError): self.login( - username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED + username=self.test_user.username, + password=TEST_USER_PASSWORD_EDITED ) response = self.get(viewname='user_management:current_user_details') @@ -142,7 +327,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): def test_user_multiple_set_password_view_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_edit, obj=self.test_user) + self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_multiple_user_set_password_view( password=TEST_USER_PASSWORD_EDITED @@ -152,30 +337,12 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): self.logout() self.login( - username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED + username=self.test_user.username, password=TEST_USER_PASSWORD_EDITED ) response = self.get(viewname='user_management:current_user_details') self.assertEqual(response.status_code, 200) - def _request_user_delete_view(self): - return self.post( - viewname='user_management:user_delete', args=(self.test_user.pk,) - ) - - def test_user_delete_view_no_access(self): - self._create_test_user() - response = self._request_user_delete_view() - self.assertEqual(response.status_code, 302) - self.assertEqual(get_user_model().objects.count(), 3) - - def test_user_delete_view_with_access(self): - self._create_test_user() - self.grant_access(permission=permission_user_delete, obj=self.test_user) - response = self._request_user_delete_view() - self.assertEqual(response.status_code, 302) - self.assertEqual(get_user_model().objects.count(), 2) - def _request_user_multiple_delete_view(self): return self.post( viewname='user_management:user_multiple_delete', data={ @@ -185,16 +352,24 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase): def test_user_multiple_delete_view_no_access(self): self._create_test_user() + user_count = get_user_model().objects.count() + response = self._request_user_multiple_delete_view() - self.assertEqual(response.status_code, 302) - self.assertEqual(get_user_model().objects.count(), 3) + self.assertEqual(response.status_code, 404) + + self.assertEqual(get_user_model().objects.count(), user_count) def test_user_multiple_delete_view_with_access(self): self._create_test_user() - self.grant_access(permission=permission_user_delete, obj=self.test_user) + user_count = get_user_model().objects.count() + + self.grant_access( + obj=self.test_user, permission=permission_user_delete + ) response = self._request_user_multiple_delete_view() self.assertEqual(response.status_code, 302) - self.assertEqual(get_user_model().objects.count(), 2) + + self.assertEqual(get_user_model().objects.count(), user_count - 1) class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase): @@ -212,13 +387,13 @@ class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase): self.metadata_type.lookup = '{{ users }}' self.metadata_type.save() self.document.metadata.create(metadata_type=self.metadata_type) - self.role.permissions.add( - permission_metadata_document_edit.stored_permission + self.grant_access( + obj=self.document, permission=permission_document_metadata_edit ) response = self.get( viewname='metadata:document_metadata_edit', - kwargs={'pk': self.document.pk} + kwargs={'document_id': self.document.pk} ) self.assertContains( response=response, text=''.format( @@ -230,13 +405,13 @@ class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase): self.metadata_type.lookup = '{{ groups }}' self.metadata_type.save() self.document.metadata.create(metadata_type=self.metadata_type) - self.role.permissions.add( - permission_metadata_document_edit.stored_permission + self.grant_access( + obj=self.document, permission=permission_document_metadata_edit ) response = self.get( viewname='metadata:document_metadata_edit', - kwargs={'pk': self.document.pk} + kwargs={'document_id': self.document.pk} ) self.assertContains( diff --git a/mayan/apps/user_management/urls.py b/mayan/apps/user_management/urls.py index 20a94f1d7b..a0c103277e 100644 --- a/mayan/apps/user_management/urls.py +++ b/mayan/apps/user_management/urls.py @@ -14,76 +14,93 @@ from .views import ( ) urlpatterns = [ - url(r'^groups/list/$', GroupListView.as_view(), name='group_list'), - url(r'^groups/create/$', GroupCreateView.as_view(), name='group_create'), url( - r'^groups/(?P\d+)/edit/$', GroupEditView.as_view(), - name='group_edit' + regex=r'^groups/$', name='group_list', view=GroupListView.as_view() ), url( - r'^groups/(?P\d+)/delete/$', GroupDeleteView.as_view(), - name='group_delete' + regex=r'^groups/create/$', name='group_create', + view=GroupCreateView.as_view() ), url( - r'^groups/(?P\d+)/members/$', GroupMembersView.as_view(), - name='group_members' - ), - - url( - r'^users/current/$', CurrentUserDetailsView.as_view(), - name='current_user_details' + regex=r'^groups/(?P\d+)/delete/$', name='group_delete', + view=GroupDeleteView.as_view() ), url( - r'^users/current/edit/$', CurrentUserEditView.as_view(), - name='current_user_edit' - ), - url(r'^users/list/$', UserListView.as_view(), name='user_list'), - url(r'^users/create/$', UserCreateView.as_view(), name='user_create'), - url( - r'^users/(?P\d+)/delete/$', UserDeleteView.as_view(), - name='user_delete' - ), - url(r'^users/(?P\d+)/edit/$', UserEditView.as_view(), name='user_edit'), - url( - r'^users/(?P\d+)/$', UserDetailsView.as_view(), - name='user_details' + regex=r'^groups/(?P\d+)/edit/$', name='group_edit', + view=GroupEditView.as_view() ), url( - r'^users/multiple/delete/$', UserDeleteView.as_view(), - name='user_multiple_delete' + regex=r'^groups/(?P\d+)/members/$', name='group_members', + view=GroupMembersView.as_view() ), url( - r'^users/(?P\d+)/set_password/$', UserSetPasswordView.as_view(), - name='user_set_password' + regex=r'^user/$', name='current_user_details', + view=CurrentUserDetailsView.as_view() ), url( - r'^users/multiple/set_password/$', UserSetPasswordView.as_view(), - name='user_multiple_set_password' + regex=r'^user/edit/$', name='current_user_edit', + view=CurrentUserEditView.as_view() ), url( - r'^users/(?P\d+)/groups/$', UserGroupsView.as_view(), - name='user_groups' + regex=r'^users/$', name='user_list', view=UserListView.as_view() ), url( - r'^users/(?P\d+)/options/$', - UserOptionsEditView.as_view(), - name='user_options' + regex=r'^users/create/$', name='user_create', + view=UserCreateView.as_view() ), + url( + regex=r'^users/(?P\d+)/$', name='user_details', + view=UserDetailsView.as_view() + ), + url( + regex=r'^users/(?P\d+)/delete/$', name='user_delete', + view=UserDeleteView.as_view(), + ), + url( + regex=r'^users/(?P\d+)/edit/$', name='user_edit', + view=UserEditView.as_view() + ), + url( + regex=r'^users/(?P\d+)/groups/$', name='user_groups', + view=UserGroupsView.as_view() + ), + url( + regex=r'^users/(?P\d+)/options/$', name='user_options', + view=UserOptionsEditView.as_view() + ), + url( + regex=r'^users/(?P\d+)/set_password/$', + name='user_set_password', view=UserSetPasswordView.as_view() + ), + url( + regex=r'^users/multiple/delete/$', name='user_multiple_delete', + view=UserDeleteView.as_view() + ), + url( + regex=r'^users/multiple/set_password/$', + name='user_multiple_set_password', view=UserSetPasswordView.as_view() + ) ] api_urls = [ - url(r'^groups/$', APIGroupListView.as_view(), name='group-list'), url( - r'^groups/(?P[0-9]+)/$', APIGroupView.as_view(), - name='group-detail' - ), - url(r'^users/$', APIUserListView.as_view(), name='user-list'), - url(r'^users/(?P[0-9]+)/$', APIUserView.as_view(), name='user-detail'), - url( - r'^users/current/$', APICurrentUserView.as_view(), name='user-current' + regex=r'^groups/$', name='group-list', view=APIGroupListView.as_view() ), url( - r'^users/(?P[0-9]+)/groups/$', APIUserGroupList.as_view(), - name='users-group-list' + regex=r'^groups/(?P\d+)/$', name='group-detail', + view=APIGroupView.as_view(), + ), + url( + regex=r'^user/$', name='user-current', + view=APICurrentUserView.as_view() + ), + url(regex=r'^users/$', name='user-list', view=APIUserListView.as_view()), + url( + regex=r'^users/(?P\d+)/$', name='user-detail', + view=APIUserView.as_view() + ), + url( + regex=r'^users/(?P\d+)/groups/$', name='users-group-list', + view=APIUserGroupList.as_view() ), ] diff --git a/mayan/apps/user_management/utils.py b/mayan/apps/user_management/utils.py index 5d5f689609..4c3b74fafe 100644 --- a/mayan/apps/user_management/utils.py +++ b/mayan/apps/user_management/utils.py @@ -5,22 +5,22 @@ from django.contrib.auth import get_user_model from django.utils.translation import ugettext_lazy as _ -def get_groups(): +def get_user_label_text(context): + if not context['request'].user.is_authenticated: + return _('Anonymous') + else: + return context['request'].user.get_full_name() or context['request'].user + + +def lookup_get_groups(): Group = apps.get_model(app_label='auth', model_name='Group') return ','.join([group.name for group in Group.objects.all()]) -def get_users(): +def lookup_get_users(): return ','.join( [ user.get_full_name() or user.username for user in get_user_model().objects.all() ] ) - - -def get_user_label_text(context): - if not context['request'].user.is_authenticated: - return _('Anonymous') - else: - return context['request'].user.get_full_name() or context['request'].user diff --git a/mayan/apps/user_management/views.py b/mayan/apps/user_management/views.py index 14cc8d6536..9cdd9fefd0 100644 --- a/mayan/apps/user_management/views.py +++ b/mayan/apps/user_management/views.py @@ -5,19 +5,21 @@ from django.contrib.auth import get_user_model from django.contrib.auth.forms import SetPasswordForm from django.contrib.auth.models import Group from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import PermissionDenied +from django.db import transaction from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.template import RequestContext from django.urls import reverse, reverse_lazy from django.utils.translation import ungettext, ugettext_lazy as _ -from mayan.apps.common.views import ( +from mayan.apps.acls.models import AccessControlList +from mayan.apps.common.generics import ( AssignRemoveView, MultipleObjectConfirmActionView, MultipleObjectFormActionView, SingleObjectCreateView, SingleObjectDeleteView, SingleObjectDetailView, SingleObjectEditView, SingleObjectListView ) +from mayan.apps.common.mixins import ExternalObjectMixin from .events import ( event_group_created, event_group_edited, event_user_created, @@ -52,7 +54,9 @@ class CurrentUserDetailsView(SingleObjectDetailView): class CurrentUserEditView(SingleObjectEditView): extra_context = {'object': None, 'title': _('Edit current user details')} form_class = UserForm - post_action_redirect = reverse_lazy('user_management:current_user_details') + post_action_redirect = reverse_lazy( + viewname='user_management:current_user_details' + ) def get_object(self): return self.request.user @@ -62,38 +66,50 @@ class GroupCreateView(SingleObjectCreateView): extra_context = {'title': _('Create new group')} fields = ('name',) model = Group - post_action_redirect = reverse_lazy('user_management:group_list') + post_action_redirect = reverse_lazy(viewname='user_management:group_list') view_permission = permission_group_create def form_valid(self, form): - group = form.save() + with transaction.atomic(): + result = super(GroupCreateView, self).form_valid(form=form) + event_group_created.commit( + actor=self.request.user, target=self.object + ) + return result - event_group_created.commit( - actor=self.request.user, target=group - ) - messages.success( - self.request, _('Group "%s" created successfully.') % group - ) - return super(GroupCreateView, self).form_valid(form=form) +class GroupDeleteView(SingleObjectDeleteView): + model = Group + object_permission = permission_group_delete + pk_url_kwarg = 'group_id' + post_action_redirect = reverse_lazy(viewname='user_management:group_list') + + def get_extra_context(self): + return { + 'object': self.object, + 'title': _('Delete the group: %s?') % self.object, + } class GroupEditView(SingleObjectEditView): fields = ('name',) model = Group object_permission = permission_group_edit - post_action_redirect = reverse_lazy('user_management:group_list') + pk_url_kwarg = 'group_id' + post_action_redirect = reverse_lazy(viewname='user_management:group_list') def form_valid(self, form): - event_group_edited.commit( - actor=self.request.user, target=self.get_object() - ) - return super(GroupEditView, self).form_valid(form=form) + with transaction.atomic(): + result = super(GroupEditView, self).form_valid(form=form) + event_group_edited.commit( + actor=self.request.user, target=self.object + ) + return result def get_extra_context(self): return { - 'object': self.get_object(), - 'title': _('Edit group: %s') % self.get_object(), + 'object': self.object, + 'title': _('Edit group: %s') % self.object, } @@ -120,29 +136,20 @@ class GroupListView(SingleObjectListView): } -class GroupDeleteView(SingleObjectDeleteView): - model = Group - object_permission = permission_group_delete - post_action_redirect = reverse_lazy('user_management:group_list') - - def get_extra_context(self): - return { - 'object': self.get_object(), - 'title': _('Delete the group: %s?') % self.get_object(), - } - - -class GroupMembersView(AssignRemoveView): +class GroupMembersView(ExternalObjectMixin, AssignRemoveView): decode_content_type = True + external_object_class = Group + external_object_permission = permission_group_edit + external_object_pk_url_kwarg = 'group_id' left_list_title = _('Available users') - right_list_title = _('Users in group') object_permission = permission_group_edit + right_list_title = _('Users in group') @staticmethod def generate_choices(choices): results = [] for choice in choices: - ct = ContentType.objects.get_for_model(choice) + ct = ContentType.objects.get_for_model(model=choice) label = choice.get_full_name() if choice.get_full_name() else choice results.append(('%s,%s' % (ct.model, choice.pk), '%s' % (label))) @@ -151,31 +158,43 @@ class GroupMembersView(AssignRemoveView): return sorted(results, key=lambda x: x[1]) def add(self, item): - self.get_object().user_set.add(item) + self.object.user_set.add(item) + + def dispatch(self, *args, **kwargs): + self.object = self.get_object() + return super(GroupMembersView, self).dispatch(*args, **kwargs) def get_extra_context(self): return { - 'object': self.get_object(), - 'title': _('Users of group: %s') % self.get_object() + 'object': self.object, + 'title': _('Users of group: %s') % self.object } def get_object(self): - return get_object_or_404(klass=Group, pk=self.kwargs['pk']) + return self.get_external_object() def left_list(self): - return GroupMembersView.generate_choices( - get_user_model().objects.exclude( - groups=self.get_object() - ).exclude(is_staff=True).exclude(is_superuser=True) + queryset = AccessControlList.objects.restrict_queryset( + permission=permission_user_edit, + queryset=get_user_model().objects.exclude( + groups=self.object + ).exclude(is_staff=True).exclude(is_superuser=True), + user=self.request.user ) + return GroupMembersView.generate_choices(choices=queryset) + def right_list(self): - return GroupMembersView.generate_choices( - self.get_object().user_set.all() + queryset = AccessControlList.objects.restrict_queryset( + permission=permission_user_edit, + queryset=self.object.user_set.all(), + user=self.request.user ) + return GroupMembersView.generate_choices(choices=queryset) + def remove(self, item): - self.get_object().user_set.remove(item) + self.object.user_set.remove(item) class UserCreateView(SingleObjectCreateView): @@ -186,25 +205,24 @@ class UserCreateView(SingleObjectCreateView): view_permission = permission_user_create def form_valid(self, form): - user = form.save(commit=False) - user.set_unusable_password() - user.save() + with transaction.atomic(): + super(UserCreateView, self).form_valid(form=form) + event_user_created.commit( + actor=self.request.user, target=self.object + ) - event_user_created.commit( - actor=self.request.user, target=user - ) - - messages.success( - self.request, _('User "%s" created successfully.') % user - ) return HttpResponseRedirect( - reverse('user_management:user_set_password', args=(user.pk,)) + reverse( + viewname='user_management:user_set_password', + kwargs={'user_id': self.object.pk} + ) ) class UserDeleteView(MultipleObjectConfirmActionView): object_permission = permission_user_delete - queryset = get_user_model().objects.filter( + pk_url_kwarg = 'user_id' + source_queryset = get_user_model().objects.filter( is_superuser=False, is_staff=False ) success_message = _('User delete request performed on %(count)d user') @@ -213,13 +231,13 @@ class UserDeleteView(MultipleObjectConfirmActionView): ) def get_extra_context(self): - queryset = self.get_queryset() + queryset = self.get_object_list() result = { 'title': ungettext( - 'Delete user', - 'Delete users', - queryset.count() + singular='Delete user', + plural='Delete users', + number=queryset.count() ) } @@ -235,26 +253,18 @@ class UserDeleteView(MultipleObjectConfirmActionView): def object_action(self, form, instance): try: - if instance.is_superuser or instance.is_staff: - messages.error( - self.request, - _( - 'Super user and staff user deleting is not ' - 'allowed, use the admin interface for these cases.' - ) - ) - else: - instance.delete() - messages.success( - self.request, _( - 'User "%s" deleted successfully.' - ) % instance - ) + instance.delete() + messages.success( + message=_( + 'User "%s" deleted successfully.' + ) % instance, request=self.request + ) except Exception as exception: messages.error( - self.request, _( + message=_( 'Error deleting user "%(user)s": %(error)s' - ) % {'user': instance, 'error': exception} + ) % {'user': instance, 'error': exception}, + request=self.request ) @@ -264,7 +274,8 @@ class UserDetailsView(SingleObjectDetailView): 'date_joined', 'groups', ) object_permission = permission_user_view - queryset = get_user_model().objects.filter( + pk_url_kwarg = 'user_id' + source_queryset = get_user_model().objects.filter( is_superuser=False, is_staff=False ) @@ -278,58 +289,72 @@ class UserDetailsView(SingleObjectDetailView): class UserEditView(SingleObjectEditView): fields = ('username', 'first_name', 'last_name', 'email', 'is_active',) object_permission = permission_user_edit - post_action_redirect = reverse_lazy('user_management:user_list') - queryset = get_user_model().objects.filter( + pk_url_kwarg = 'user_id' + post_action_redirect = reverse_lazy(viewname='user_management:user_list') + source_queryset = get_user_model().objects.filter( is_superuser=False, is_staff=False ) def form_valid(self, form): - event_user_edited.commit( - actor=self.request.user, target=self.get_object() - ) - return super(UserEditView, self).form_valid(form=form) + with transaction.atomic(): + result = super(UserEditView, self).form_valid(form=form) + event_user_edited.commit( + actor=self.request.user, target=self.object + ) + + return result def get_extra_context(self): return { - 'object': self.get_object(), - 'title': _('Edit user: %s') % self.get_object(), + 'object': self.object, + 'title': _('Edit user: %s') % self.object, } -class UserGroupsView(AssignRemoveView): +class UserGroupsView(ExternalObjectMixin, AssignRemoveView): decode_content_type = True + external_object_queryset = get_user_model().objects.filter( + is_staff=False, is_superuser=False + ) + external_object_permission = permission_user_edit + external_object_pk_url_kwarg = 'user_id' left_list_title = _('Available groups') right_list_title = _('Groups joined') - object_permission = permission_user_edit def add(self, item): - item.user_set.add(self.get_object()) + item.user_set.add(self.object) + + def dispatch(self, *args, **kwargs): + self.object = self.get_object() + return super(UserGroupsView, self).dispatch(*args, **kwargs) def get_extra_context(self): return { - 'object': self.get_object(), - 'title': _('Groups of user: %s') % self.get_object() + 'object': self.object, + 'title': _('Groups of user: %s') % self.object } def get_object(self): - return get_object_or_404( - klass=get_user_model().objects.filter( - is_superuser=False, is_staff=False - ), pk=self.kwargs['pk'] - ) + return self.get_external_object() def left_list(self): - return AssignRemoveView.generate_choices( - Group.objects.exclude(user=self.get_object()) + queryset = AccessControlList.objects.restrict_queryset( + permission=permission_group_edit, + queryset=Group.objects.exclude(user=self.object), + user=self.request.user ) + return AssignRemoveView.generate_choices(choices=queryset) def right_list(self): - return AssignRemoveView.generate_choices( - Group.objects.filter(user=self.get_object()) + queryset = AccessControlList.objects.restrict_queryset( + permission=permission_group_edit, + queryset=Group.objects.filter(user=self.object), + user=self.request.user ) + return AssignRemoveView.generate_choices(choices=queryset) def remove(self, item): - item.user_set.remove(self.get_object()) + item.user_set.remove(self.object) class UserListView(SingleObjectListView): @@ -350,7 +375,7 @@ class UserListView(SingleObjectListView): 'title': _('Users'), } - def get_object_list(self): + def get_source_queryset(self): return get_user_model().objects.exclude( is_superuser=True ).exclude(is_staff=True).order_by('last_name', 'first_name') @@ -372,13 +397,13 @@ class UserOptionsEditView(SingleObjectEditView): return self.get_user().user_options def get_post_action_redirect(self): - return reverse('user_management:user_list') + return reverse(viewname='user_management:user_list') def get_user(self): return get_object_or_404( klass=get_user_model().objects.filter( is_superuser=False, is_staff=False - ), pk=self.kwargs['pk'] + ), pk=self.kwargs['user_id'] ) @@ -386,66 +411,57 @@ class UserSetPasswordView(MultipleObjectFormActionView): form_class = SetPasswordForm model = get_user_model() object_permission = permission_user_edit + pk_url_kwarg = 'user_id' + source_queryset = get_user_model().objects.filter( + is_superuser=False, is_staff=False + ) success_message = _('Password change request performed on %(count)d user') success_message_plural = _( 'Password change request performed on %(count)d users' ) def get_extra_context(self): - queryset = self.get_queryset() + queryset = self.get_object_list() result = { 'submit_label': _('Submit'), 'title': ungettext( - 'Change user password', - 'Change users passwords', - queryset.count() - ) + singular='Change the password of the %(count)d selected user', + plural='Change the password of the %(count)d selected users', + number=queryset.count() + ) % {'count': queryset.count()} } if queryset.count() == 1: result.update( { 'object': queryset.first(), - 'title': _('Change password for user: %s') % queryset.first() + 'title': _( + 'Change the password of user: %s' + ) % queryset.first() } ) return result def get_form_extra_kwargs(self): - queryset = self.get_queryset() - result = {} - if queryset: - result['user'] = queryset.first() - return result - else: - raise PermissionDenied + queryset = self.get_object_list() + return {'user': queryset.first()} def object_action(self, form, instance): try: - if instance.is_superuser or instance.is_staff: - messages.error( - self.request, - _( - 'Super user and staff user password ' - 'reseting is not allowed, use the admin ' - 'interface for these cases.' - ) - ) - else: - instance.set_password(form.cleaned_data['new_password1']) - instance.save() - messages.success( - self.request, _( - 'Successful password reset for user: %s.' - ) % instance - ) + instance.set_password(form.cleaned_data['new_password1']) + instance.save() + messages.success( + message=_( + 'Successful password reset for user: %s.' + ) % instance, request=self.request + ) except Exception as exception: messages.error( - self.request, _( + message=_( 'Error reseting password for user "%(user)s": %(error)s' ) % { 'user': instance, 'error': exception - } + }, request=self.request )