Merge pull request #1519 from ostcar/rest-anonymous-user

Fix anonymous user for rest requests

openslides/agenda/models.py

@@ -163,7 +163,7 @@ class Item(RESTModelMixin, SlideMixin, AbsoluteUrlMixin, MPTTModel):
         Return the title of this item.
         """
         if not self.content_object:
-            agenda_title = self.title
+            agenda_title = self.title or ""
         else:
             try:
                 agenda_title = self.content_object.get_agenda_title()

openslides/global_settings.py

@@ -183,5 +183,8 @@ TEST_RUNNER = 'openslides.utils.test.OpenSlidesDiscoverRunner'
 
 # Config for the REST Framework
 REST_FRAMEWORK = {
-    'UNAUTHENTICATED_USER': 'openslides.users.auth.AnonymousUser',
+    'DEFAULT_AUTHENTICATION_CLASSES': (
+        'rest_framework.authentication.SessionAuthentication',
+        'openslides.users.auth.AnonymousAuthentication',
+    )
 }

openslides/users/auth.py

@@ -1,15 +1,15 @@
+from django.contrib.auth import get_user as _get_user
 from django.contrib.auth import get_user_model
 from django.contrib.auth.backends import ModelBackend
-from django.contrib.auth.models import AnonymousUser as DjangoAnonymousUser
 from django.contrib.auth.context_processors import auth as _auth
-from django.contrib.auth import get_user as _get_user
+from django.contrib.auth.models import AnonymousUser as DjangoAnonymousUser
+from django.contrib.auth.models import Permission
 from django.db.models import Q
 from django.utils.functional import SimpleLazyObject
+from rest_framework.authentication import BaseAuthentication
 
 from openslides.config.api import config
 
-from .models import Permission
-
 
 class AnonymousUser(DjangoAnonymousUser):
     """
@@ -94,6 +94,20 @@ class AuthenticationMiddleware(object):
         request.user = SimpleLazyObject(lambda: get_user(request))
 
 
+class AnonymousAuthentication(BaseAuthentication):
+    """
+    Authentication class for the Django REST framework.
+
+    Sets the user to the our AnonymousUser but only if system_enable_anonymous
+    is set to True in the config.
+    """
+
+    def authenticate(self, request):
+        if config['system_enable_anonymous']:
+            return (AnonymousUser(), None)
+        return None
+
+
 def get_user(request):
     """
     Gets the user from the request.

openslides/users/models.py

@@ -99,7 +99,7 @@ class User(RESTModelMixin, SlideMixin, AbsoluteUrlMixin, PermissionsMixin, Abstr
     slide_callback_name = 'user'
 
     username = models.CharField(
-        ugettext_lazy('Username'), max_length=255, unique=True)
+        ugettext_lazy('Username'), max_length=255, unique=True, blank=True)
 
     first_name = models.CharField(
         ugettext_lazy('First name'), max_length=255, blank=True)

openslides/users/serializers.py

@@ -1,4 +1,3 @@
-from django.core.exceptions import ImproperlyConfigured
 from django.contrib.auth.hashers import make_password
 from django.utils.translation import ugettext as _, ugettext_lazy
 
@@ -31,32 +30,6 @@ class UserFullSerializer(ModelSerializer):
 
     Serializes all relevant fields.
     """
-    class Meta:
-        model = User
-        fields = (
-            'id',
-            'is_present',
-            'username',
-            'title',
-            'first_name',
-            'last_name',
-            'structure_level',
-            'about_me',
-            'comment',
-            'groups',
-            'default_password',
-            'last_login',
-            'is_active',)
-
-
-class UserCreateUpdateSerializer(ModelSerializer):
-    """
-    Serializer for users.models.User objects.
-
-    Serializes data to create new users or update users.
-
-    Do not use this for list or retrieve requests.
-    """
     groups = PrimaryKeyRelatedField(
         many=True,
         queryset=Group.objects.exclude(pk__in=(1, 2)),
@@ -80,37 +53,33 @@ class UserCreateUpdateSerializer(ModelSerializer):
             'default_password',
             'is_active',)
 
-    def __init__(self, *args, **kwargs):
-        """
-        Overridden to add read_only flag to username field in create requests.
-        """
-        super().__init__(*args, **kwargs)
-        if self.context['view'].action == 'create':
-            self.fields['username'].read_only = True
-        elif self.context['view'].action == 'update':
-            # Everything is fine. Do nothing.
-            pass
-        else:  # Other action than 'create' or 'update'.
-            raise ImproperlyConfigured('This serializer can only be used in create and update requests.')
-
     def validate(self, data):
         """
         Checks that first_name or last_name is given.
+
+        Generates the username if it is empty.
         """
         if not (data.get('username') or data.get('first_name') or data.get('last_name')):
             raise ValidationError(_('Username, first name and last name can not all be empty.'))
+
+        # Generate username. But only if it is not set and the serializer is not
+        # called in a patch-context.
+        try:
+            action = self.context['view'].action
+        except (KeyError, AttributeError):
+            action = None
+
+        if not data.get('username') and action != 'partial_update':
+            data['username'] = User.objects.generate_username(
+                data.get('first_name', ''),
+                data.get('last_name', ''))
         return data
 
     def create(self, validated_data):
         """
-        Creates user with generated username and sets the default_password.
+        Creates the user. Sets the default_password. Adds the new user to the
-        Adds the new user to the registered group.
+        registered group.
         """
-        # Generate username if neccessary.
-        if not validated_data.get('username'):
-            validated_data['username'] = User.objects.generate_username(
-                validated_data.get('first_name', ''),
-                validated_data.get('last_name', ''))
         # Prepare setup password.
         if not validated_data.get('default_password'):
             validated_data['default_password'] = User.objects.generate_password()

openslides/users/views.py

@@ -22,7 +22,6 @@ from .models import Group, User
 from .pdf import users_passwords_to_pdf, users_to_pdf
 from .serializers import (
     GroupSerializer,
-    UserCreateUpdateSerializer,
     UserFullSerializer,
     UserShortSerializer,
 )
@@ -88,9 +87,8 @@ class UserViewSet(ModelViewSet):
         Returns different serializer classes with respect to action and user's
         permissions.
         """
-        if self.action in ('create', 'update'):
+        if (self.action in ('create', 'update') or
-            serializer_class = UserCreateUpdateSerializer
+                self.request.user.has_perm('users.can_see_extra_data')):
-        elif self.request.user.has_perm('users.can_see_extra_data'):
             serializer_class = UserFullSerializer
         else:
             serializer_class = UserShortSerializer

requirements_production.txt

@@ -5,7 +5,7 @@ bleach>=1.4,<1.5
 django-ckeditor-updated>=4.2.3,<4.4
 django-haystack>=2.1,<2.4
 django-mptt>=0.6,<0.7
-djangorestframework>=3.0.5,<3.1.0
+djangorestframework>=3.0.5,<3.2.0
 jsonfield>=0.9.19,<1.1
 natsort>=3.2,<3.6
 reportlab>=3.0,<3.2

setup.cfg

@@ -10,4 +10,4 @@ max_line_length = 150
 [isort]
 include_trailing_comma = true
 multi_line_output = 3
-skip = main.py,signals.py,csv_import.py,global_settings.py,test_personal_info.py,tests.py,auth.py,search_indexes.py,test_list_of_speakers.py,chatbox.py,test_main_menu.py,test_config.py,runserver.py,rest_api.py,settings.py,test_pdf.py,__main__.py,test_serializers.py,autoupdate.py,pdf.py,widgets.py,models.py,test_csv_import.py,plugins.py,serializers.py,test_csv.py,test_overlays.py,backupdb.py,views.py,test_views.py,urls.py,forms.py,utils.py,test_auth.py
+skip = main.py,signals.py,csv_import.py,global_settings.py,test_personal_info.py,tests.py,search_indexes.py,test_list_of_speakers.py,chatbox.py,test_main_menu.py,test_config.py,runserver.py,rest_api.py,settings.py,test_pdf.py,__main__.py,autoupdate.py,pdf.py,widgets.py,models.py,test_csv_import.py,plugins.py,serializers.py,test_csv.py,test_overlays.py,backupdb.py,views.py,test_views.py,urls.py,forms.py,utils.py

tests/integration/users/test_rest_anonymous_user.py

@@ -0,0 +1,25 @@
+from unittest.mock import patch
+
+from openslides.utils.test import TestCase
+
+
+class TestAnonymousRequests(TestCase):
+    """
+    Test that a request with an user that is not logged in gets only the
+    requested data, if the anonymous user is activated in the config.
+
+    Expects that the page '/rest/users/user/' needs a permission and the
+    anonymous user has this permission.
+    """
+
+    @patch('openslides.users.auth.config', {'system_enable_anonymous': True})
+    def test_with_anonymous_user(self):
+        response = self.client.get('/rest/users/user/')
+
+        self.assertEqual(response.status_code, 200)
+
+    @patch('openslides.users.auth.config', {'system_enable_anonymous': False})
+    def test_without_anonymous_user(self):
+        response = self.client.get('/rest/users/user/')
+
+        self.assertEqual(response.status_code, 403)

tests/integration/users/test_viewset.py

@@ -6,6 +6,25 @@ from openslides.users.models import Group, User
 from openslides.utils.test import TestCase
 
 
+class UserGetTest(TestCase):
+    """
+    Tests to receive a users via REST API.
+    """
+    def test_get_with_user_who_is_in_group_with_pk_1(self):
+        """
+        It is invalid, that a user is in the group with the pk 1. But if the
+        database is invalid, the user should nevertheless be received.
+        """
+        admin = User.objects.get(pk=1)
+        group1 = Group.objects.get(pk=1)
+        admin.groups.add(group1)
+        self.client.login(username='admin', password='admin')
+
+        response = self.client.get('/rest/users/user/1/')
+
+        self.assertEqual(response.status_code, 200)
+
+
 class UserCreate(TestCase):
     """
     Tests creation of users via REST API.
@@ -47,7 +66,7 @@ class UserCreate(TestCase):
              'groups': group_pks})
 
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(response.data, {'groups': ["Invalid pk '%d' - object does not exist." % group_pks[0]]})
+        self.assertEqual(response.data, {'groups': ["Invalid pk \"%d\" - object does not exist." % group_pks[0]]})
 
 
 class UserUpdate(TestCase):
@@ -55,6 +74,11 @@ class UserUpdate(TestCase):
     Tests update of users via REST API.
     """
     def test_simple_update_via_patch(self):
+        """
+        Test to only update the last_name with a patch request.
+
+        The field username *should not* be changed by the request.
+        """
         admin_client = APIClient()
         admin_client.login(username='admin', password='admin')
         # This is the builtin user 'Administrator' with username 'admin'. The pk is valid.
@@ -70,6 +94,11 @@ class UserUpdate(TestCase):
         self.assertEqual(user.username, 'admin')
 
     def test_simple_update_via_put(self):
+        """
+        Test to only update the last_name with a put request.
+
+        The field username *should* be changed by the request.
+        """
         admin_client = APIClient()
         admin_client.login(username='admin', password='admin')
         # This is the builtin user 'Administrator'. The pk is valid.
@@ -77,10 +106,10 @@ class UserUpdate(TestCase):
 
         response = admin_client.put(
             reverse('user-detail', args=[user_pk]),
-            {'last_name': 'New name Ohy4eeyei5Sahzah0Os2'})
+            {'last_name': 'New name Ohy4eeyei5'})
 
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, {'username': ['This field is required.']})
+        self.assertEqual(User.objects.get(pk=1).username, 'New name Ohy4eeyei5')
 
 
 class UserDelete(TestCase):

tests/unit/users/test_auth.py

@@ -1,7 +1,7 @@
 from unittest import TestCase
-from unittest.mock import patch, MagicMock
+from unittest.mock import MagicMock, patch
 
-from openslides.users.auth import AnonymousUser, get_user, auth
+from openslides.users.auth import AnonymousUser, auth, get_user
 
 
 class TestAnonymousUser(TestCase):

tests/unit/users/test_serializers.py

@@ -1,83 +1,42 @@
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
-from django.core.exceptions import ImproperlyConfigured
+from openslides.users.serializers import UserFullSerializer
-from rest_framework import status
+from openslides.utils.rest_api import ValidationError
-from rest_framework.viewsets import ModelViewSet
-from rest_framework.test import APIRequestFactory
 
 
-class UserCreateUpdateSerializer(TestCase):
+class UserCreateUpdateSerializerTest(TestCase):
-    def test_improperly_configured_exception_list_request(self):
+    def test_validate_no_data(self):
         """
-        Tests that ImproperlyConfigured is raised if one tries to use the
+        Tests, that the validator raises a ValidationError, if not data is given.
-        UserCreateUpdateSerializer with a list request.
         """
-        # Global import is not possible for some unknown magic.
+        serializer = UserFullSerializer()
-        from openslides.users.serializers import UserCreateUpdateSerializer
+        data = {}
-        factory = APIRequestFactory()
-        request = factory.get('/')
-        view_class = ModelViewSet
-        view_class.queryset = MagicMock()
-        view_class.serializer_class = UserCreateUpdateSerializer
-        view = view_class.as_view({'get': 'list'})
 
-        with self.assertRaises(ImproperlyConfigured):
+        with self.assertRaises(ValidationError):
-            view(request)
+            serializer.validate(data)
 
-    @patch('rest_framework.generics.get_object_or_404')
+    @patch('openslides.users.serializers.User.objects.generate_username')
-    def test_improperly_configured_exception_retrieve_request(self, mock_get_object_or_404):
+    def test_validate_no_username(self, generate_username):
         """
-        Tests that ImproperlyConfigured is raised if one tries to use the
+        Tests, that an empty username is generated.
-        UserCreateUpdateSerializer with a retrieve request.
         """
-        # Global import is not possible for some unknown magic.
+        generate_username.return_value = 'test_value'
-        from openslides.users.serializers import UserCreateUpdateSerializer
+        serializer = UserFullSerializer()
-        factory = APIRequestFactory()
+        data = {'first_name': 'TestName'}
-        request = factory.get('/')
-        view_class = ModelViewSet
-        view_class.queryset = MagicMock()
-        view_class.serializer_class = UserCreateUpdateSerializer
-        view = view_class.as_view({'get': 'retrieve'})
-        mock_get_object_or_404.return_value = MagicMock()
 
-        with self.assertRaises(ImproperlyConfigured):
+        new_data = serializer.validate(data)
-            view(request, pk='1')
 
-    def test_no_improperly_configured_exception_create_request(self):
+        self.assertEqual(new_data['username'], 'test_value')
+
+    def test_validate_no_username_in_patch_request(self):
         """
-        Tests that ImproperlyConfigured is not raised if one tries to use the
+        Tests, that an empty username is not set in a patch request context.
-        UserCreateUpdateSerializer with a create request.
         """
-        # Global import is not possible for some unknown magic.
+        view = MagicMock(action='partial_update')
-        from openslides.users.serializers import UserCreateUpdateSerializer
+        serializer = UserFullSerializer(context={'view': view})
-        factory = APIRequestFactory()
+        data = {'first_name': 'TestName'}
-        request = factory.get('/')
-        view_class = ModelViewSet
-        view_class.queryset = MagicMock()
-        view_class.serializer_class = UserCreateUpdateSerializer
-        view = view_class.as_view({'get': 'create'})
 
-        response = view(request)
+        new_data = serializer.validate(data)
 
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertIsNone(new_data.get('username'))
-
-    @patch('rest_framework.generics.get_object_or_404')
-    def test_no_improperly_configured_exception_update_request(self, mock_get_object_or_404):
-        """
-        Tests that ImproperlyConfigured is not raised if one tries to use the
-        UserCreateUpdateSerializer with a update request.
-        """
-        # Global import is not possible for some unknown magic.
-        from openslides.users.serializers import UserCreateUpdateSerializer
-        factory = APIRequestFactory()
-        request = factory.get('/')
-        view_class = ModelViewSet
-        view_class.queryset = MagicMock()
-        view_class.serializer_class = UserCreateUpdateSerializer
-        view = view_class.as_view({'get': 'update'})
-        mock_get_object_or_404.return_value = MagicMock()
-
-        response = view(request, pk='1')
-
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)