From cd34d30866521d320d829729ba4308fd38d4734f Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Thu, 1 Nov 2018 17:30:18 +0100 Subject: [PATCH] Remove utils.collections.Collection class and other cleanups * Activate restricted_data_cache on inmemory cache * Use ElementCache in rest-api get requests * Get requests on the restapi return 404 when the user has no permission * Added async function for has_perm and in_some_groups * changed Cachable.get_restricted_data to be an ansync function * rewrote required_user_system * changed default implementation of access_permission.check_permission to check a given permission or check if anonymous is enabled --- openslides/agenda/access_permissions.py | 18 ++- openslides/agenda/apps.py | 21 +++- openslides/agenda/models.py | 1 + openslides/agenda/signals.py | 18 --- openslides/agenda/views.py | 2 +- openslides/assignments/access_permissions.py | 14 +-- openslides/assignments/apps.py | 24 +++- openslides/assignments/models.py | 1 + openslides/assignments/signals.py | 22 ---- openslides/core/access_permissions.py | 43 +------ openslides/core/apps.py | 18 ++- openslides/core/models.py | 1 + openslides/core/signals.py | 23 ---- openslides/core/websocket.py | 2 +- openslides/mediafiles/access_permissions.py | 14 +-- openslides/mediafiles/apps.py | 22 +++- openslides/mediafiles/models.py | 1 + openslides/mediafiles/signals.py | 18 --- openslides/motions/access_permissions.py | 66 ++++------- openslides/motions/apps.py | 23 +++- openslides/motions/models.py | 1 + openslides/motions/signals.py | 22 +--- openslides/topics/access_permissions.py | 7 +- openslides/users/access_permissions.py | 58 ++++------ openslides/users/views.py | 18 ++- openslides/utils/access_permissions.py | 81 ++++++++++++- openslides/utils/auth.py | 46 ++++++-- openslides/utils/cache.py | 49 +++++--- openslides/utils/cache_providers.py | 102 +++++++---------- openslides/utils/collection.py | 114 +------------------ openslides/utils/consumers.py | 8 +- openslides/utils/models.py | 4 +- openslides/utils/redis.py | 37 ++++++ openslides/utils/rest_api.py | 27 +---- tests/conftest.py | 2 + tests/integration/agenda/test_viewset.py | 2 +- tests/integration/helpers.py | 4 +- tests/integration/motions/test_viewset.py | 8 +- tests/integration/users/test_viewset.py | 4 +- tests/integration/utils/test_collection.py | 34 ------ tests/settings.py | 3 + tests/unit/users/test_access_permissions.py | 20 ---- tests/unit/utils/cache_provider.py | 4 +- tests/unit/utils/test_cache.py | 1 - 44 files changed, 418 insertions(+), 590 deletions(-) create mode 100644 openslides/utils/redis.py delete mode 100644 tests/unit/users/test_access_permissions.py diff --git a/openslides/agenda/access_permissions.py b/openslides/agenda/access_permissions.py index 9d1738f44..940e5a3b0 100644 --- a/openslides/agenda/access_permissions.py +++ b/openslides/agenda/access_permissions.py @@ -1,7 +1,7 @@ from typing import Any, Dict, Iterable, List, Optional from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import has_perm +from ..utils.auth import async_has_perm from ..utils.collection import CollectionElement @@ -9,11 +9,7 @@ class ItemAccessPermissions(BaseAccessPermissions): """ Access permissions container for Item and ItemViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'agenda.can_see') + base_permission = 'agenda.can_see' def get_serializer_class(self, user=None): """ @@ -26,7 +22,7 @@ class ItemAccessPermissions(BaseAccessPermissions): # TODO: In the following method we use full_data['is_hidden'] and # full_data['is_internal'] but this can be out of date. - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -47,11 +43,11 @@ class ItemAccessPermissions(BaseAccessPermissions): return {key: full_data[key] for key in whitelist} # Parse data. - if full_data and has_perm(user, 'agenda.can_see'): - if has_perm(user, 'agenda.can_manage') and has_perm(user, 'agenda.can_see_internal_items'): + if full_data and await async_has_perm(user, 'agenda.can_see'): + if await async_has_perm(user, 'agenda.can_manage') and await async_has_perm(user, 'agenda.can_see_internal_items'): # Managers with special permission can see everything. data = full_data - elif has_perm(user, 'agenda.can_see_internal_items'): + elif await async_has_perm(user, 'agenda.can_see_internal_items'): # Non managers with special permission can see everything but # comments and hidden items. data = [full for full in full_data if not full['is_hidden']] # filter hidden items @@ -72,7 +68,7 @@ class ItemAccessPermissions(BaseAccessPermissions): # In non internal case managers see everything and non managers see # everything but comments. - if has_perm(user, 'agenda.can_manage'): + if await async_has_perm(user, 'agenda.can_manage'): blocked_keys_non_internal_hidden_case: Iterable[str] = [] can_see_hidden = True else: diff --git a/openslides/agenda/apps.py b/openslides/agenda/apps.py index ed5434341..3629d1407 100644 --- a/openslides/agenda/apps.py +++ b/openslides/agenda/apps.py @@ -1,3 +1,5 @@ +from typing import Any, Dict, Set + from django.apps import AppConfig from ..utils.projector import register_projector_elements @@ -12,15 +14,15 @@ class AgendaAppConfig(AppConfig): def ready(self): # Import all required stuff. from django.db.models.signals import pre_delete, post_save - from ..core.signals import permission_change, user_data_required + from ..core.signals import permission_change from ..utils.rest_api import router from .projector import get_projector_elements from .signals import ( get_permission_change_data, listen_to_related_object_post_delete, - listen_to_related_object_post_save, - required_users) + listen_to_related_object_post_save) from .views import ItemViewSet + from ..utils.access_permissions import required_user # Define projector elements. register_projector_elements(get_projector_elements()) @@ -35,13 +37,13 @@ class AgendaAppConfig(AppConfig): permission_change.connect( get_permission_change_data, dispatch_uid='agenda_get_permission_change_data') - user_data_required.connect( - required_users, - dispatch_uid='agenda_required_users') # Register viewsets. router.register(self.get_model('Item').get_collection_string(), ItemViewSet) + # register required_users + required_user.add_collection_string(self.get_model('Item').get_collection_string(), required_users) + def get_config_variables(self): from .config_variables import get_config_variables return get_config_variables() @@ -52,3 +54,10 @@ class AgendaAppConfig(AppConfig): connection. """ yield self.get_model('Item') + + +def required_users(element: Dict[str, Any]) -> Set[int]: + """ + Returns all user ids that are displayed as speaker in the given element. + """ + return set(speaker['user_id'] for speaker in element['speakers']) diff --git a/openslides/agenda/models.py b/openslides/agenda/models.py index 45ecae422..afca04c4a 100644 --- a/openslides/agenda/models.py +++ b/openslides/agenda/models.py @@ -182,6 +182,7 @@ class Item(RESTModelMixin, models.Model): """ access_permissions = ItemAccessPermissions() objects = ItemManager() + can_see_permission = 'agenda.can_see' AGENDA_ITEM = 1 INTERNAL_ITEM = 2 diff --git a/openslides/agenda/signals.py b/openslides/agenda/signals.py index 36208d139..9411da5d0 100644 --- a/openslides/agenda/signals.py +++ b/openslides/agenda/signals.py @@ -1,11 +1,7 @@ -from typing import Set - from django.apps import apps from django.contrib.contenttypes.models import ContentType -from ..utils.auth import has_perm from ..utils.autoupdate import inform_changed_data -from ..utils.collection import Collection from .models import Item @@ -64,17 +60,3 @@ def get_permission_change_data(sender, permissions, **kwargs): and permission.codename in ('can_see', 'can_see_internal_items')): yield from agenda_app.get_startup_elements() break - - -def required_users(sender, request_user, **kwargs): - """ - Returns all user ids that are displayed as speakers in any agenda item - if request_user can see the agenda. This function may return an empty - set. - """ - speakers: Set[int] = set() - if has_perm(request_user, 'agenda.can_see'): - for item_collection_element in Collection(Item.get_collection_string()).element_generator(): - full_data = item_collection_element.get_full_data() - speakers.update(speaker['user_id'] for speaker in full_data['speakers']) - return speakers diff --git a/openslides/agenda/views.py b/openslides/agenda/views.py index 48898237d..e0e295fd5 100644 --- a/openslides/agenda/views.py +++ b/openslides/agenda/views.py @@ -50,7 +50,7 @@ class ItemViewSet(ListModelMixin, RetrieveModelMixin, UpdateModelMixin, GenericV elif self.action in ('speak', 'sort_speakers'): result = (has_perm(self.request.user, 'agenda.can_see') and has_perm(self.request.user, 'agenda.can_manage_list_of_speakers')) - elif self.action in ('numbering'): + elif self.action in ('numbering', ): result = (has_perm(self.request.user, 'agenda.can_see') and has_perm(self.request.user, 'agenda.can_manage')) else: diff --git a/openslides/assignments/access_permissions.py b/openslides/assignments/access_permissions.py index f0dd3fe4d..e8d70e778 100644 --- a/openslides/assignments/access_permissions.py +++ b/openslides/assignments/access_permissions.py @@ -1,7 +1,7 @@ from typing import Any, Dict, List, Optional from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import has_perm +from ..utils.auth import async_has_perm, has_perm from ..utils.collection import CollectionElement @@ -9,11 +9,7 @@ class AssignmentAccessPermissions(BaseAccessPermissions): """ Access permissions container for Assignment and AssignmentViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'assignments.can_see') + base_permission = 'assignments.can_see' def get_serializer_class(self, user=None): """ @@ -27,7 +23,7 @@ class AssignmentAccessPermissions(BaseAccessPermissions): serializer_class = AssignmentShortSerializer return serializer_class - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -37,9 +33,9 @@ class AssignmentAccessPermissions(BaseAccessPermissions): only get a result like the AssignmentShortSerializer would give them. """ # Parse data. - if has_perm(user, 'assignments.can_see') and has_perm(user, 'assignments.can_manage'): + if await async_has_perm(user, 'assignments.can_see') and await async_has_perm(user, 'assignments.can_manage'): data = full_data - elif has_perm(user, 'assignments.can_see'): + elif await async_has_perm(user, 'assignments.can_see'): # Exclude unpublished poll votes. data = [] for full in full_data: diff --git a/openslides/assignments/apps.py b/openslides/assignments/apps.py index 301aaca79..555faf16c 100644 --- a/openslides/assignments/apps.py +++ b/openslides/assignments/apps.py @@ -1,4 +1,4 @@ -from typing import List +from typing import Any, Dict, List, Set from django.apps import AppConfig from mypy_extensions import TypedDict @@ -14,11 +14,12 @@ class AssignmentsAppConfig(AppConfig): def ready(self): # Import all required stuff. - from ..core.signals import permission_change, user_data_required + from ..core.signals import permission_change from ..utils.rest_api import router from .projector import get_projector_elements - from .signals import get_permission_change_data, required_users + from .signals import get_permission_change_data from .views import AssignmentViewSet, AssignmentPollViewSet + from ..utils.access_permissions import required_user # Define projector elements. register_projector_elements(get_projector_elements()) @@ -27,14 +28,14 @@ class AssignmentsAppConfig(AppConfig): permission_change.connect( get_permission_change_data, dispatch_uid='assignments_get_permission_change_data') - user_data_required.connect( - required_users, - dispatch_uid='assignments_required_users') # Register viewsets. router.register(self.get_model('Assignment').get_collection_string(), AssignmentViewSet) router.register('assignments/poll', AssignmentPollViewSet) + # Register required_users + required_user.add_collection_string(self.get_model('Assignment').get_collection_string(), required_users) + def get_config_variables(self): from .config_variables import get_config_variables return get_config_variables() @@ -56,3 +57,14 @@ class AssignmentsAppConfig(AppConfig): 'display_name': phase[1], }) return {'AssignmentPhases': phases} + + +def required_users(element: Dict[str, Any]) -> Set[int]: + """ + Returns all user ids that are displayed as candidates (including poll + options) in the assignment element. + """ + candidates = set(related_user['user_id'] for related_user in element['assignment_related_users']) + for poll in element['polls']: + candidates.update(option['candidate_id'] for option in poll['options']) + return candidates diff --git a/openslides/assignments/models.py b/openslides/assignments/models.py index b970f8e56..53b894221 100644 --- a/openslides/assignments/models.py +++ b/openslides/assignments/models.py @@ -91,6 +91,7 @@ class Assignment(RESTModelMixin, models.Model): Model for assignments. """ access_permissions = AssignmentAccessPermissions() + can_see_permission = 'assignments.can_see' objects = AssignmentManager() diff --git a/openslides/assignments/signals.py b/openslides/assignments/signals.py index 48bc9cdb8..22182ba51 100644 --- a/openslides/assignments/signals.py +++ b/openslides/assignments/signals.py @@ -1,11 +1,5 @@ -from typing import Any, Set - from django.apps import apps -from ..utils.auth import has_perm -from ..utils.collection import Collection -from .models import Assignment - def get_permission_change_data(sender, permissions=None, **kwargs): """ @@ -16,19 +10,3 @@ def get_permission_change_data(sender, permissions=None, **kwargs): # There could be only one 'assignment.can_see' and then we want to return data. if permission.content_type.app_label == assignments_app.label and permission.codename == 'can_see': yield from assignments_app.get_startup_elements() - - -def required_users(sender, request_user, **kwargs): - """ - Returns all user ids that are displayed as candidates (including poll - options) in any assignment if request_user can see assignments. This - function may return an empty set. - """ - candidates: Set[Any] = set() - if has_perm(request_user, 'assignments.can_see'): - for assignment_collection_element in Collection(Assignment.get_collection_string()).element_generator(): - full_data = assignment_collection_element.get_full_data() - candidates.update(related_user['user_id'] for related_user in full_data['assignment_related_users']) - for poll in full_data['polls']: - candidates.update(option['candidate_id'] for option in poll['options']) - return candidates diff --git a/openslides/core/access_permissions.py b/openslides/core/access_permissions.py index d07368110..2e2276465 100644 --- a/openslides/core/access_permissions.py +++ b/openslides/core/access_permissions.py @@ -1,18 +1,11 @@ -from django.contrib.auth.models import AnonymousUser - from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import anonymous_is_enabled, has_perm class ProjectorAccessPermissions(BaseAccessPermissions): """ Access permissions container for Projector and ProjectorViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'core.can_see_projector') + base_permission = 'core.can_see_projector' def get_serializer_class(self, user=None): """ @@ -27,13 +20,6 @@ class TagAccessPermissions(BaseAccessPermissions): """ Access permissions container for Tag and TagViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - # Every authenticated user can retrieve tags. Anonymous users can do - # so if they are enabled. - return not isinstance(user, AnonymousUser) or anonymous_is_enabled() def get_serializer_class(self, user=None): """ @@ -48,13 +34,7 @@ class ChatMessageAccessPermissions(BaseAccessPermissions): """ Access permissions container for ChatMessage and ChatMessageViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - # Anonymous users can see the chat if the anonymous group has the - # permission core.can_use_chat. But they can not use it. See views.py. - return has_perm(user, 'core.can_use_chat') + base_permission = 'core.can_use_chat' def get_serializer_class(self, user=None): """ @@ -69,11 +49,7 @@ class ProjectorMessageAccessPermissions(BaseAccessPermissions): """ Access permissions for ProjectorMessage. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'core.can_see_projector') + base_permission = 'core.can_see_projector' def get_serializer_class(self, user=None): """ @@ -88,11 +64,7 @@ class CountdownAccessPermissions(BaseAccessPermissions): """ Access permissions for Countdown. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'core.can_see_projector') + base_permission = 'core.can_see_projector' def get_serializer_class(self, user=None): """ @@ -108,13 +80,6 @@ class ConfigAccessPermissions(BaseAccessPermissions): Access permissions container for the config (ConfigStore and ConfigViewSet). """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - # Every authenticated user can see the metadata and list or retrieve - # the config. Anonymous users can do so if they are enabled. - return not isinstance(user, AnonymousUser) or anonymous_is_enabled() def get_serializer_class(self, user=None): """ diff --git a/openslides/core/apps.py b/openslides/core/apps.py index 1d85f4dd4..3b94070c6 100644 --- a/openslides/core/apps.py +++ b/openslides/core/apps.py @@ -1,6 +1,6 @@ from collections import OrderedDict from operator import attrgetter -from typing import Any, Dict, List +from typing import Any, Dict, List, Set from django.apps import AppConfig from django.conf import settings @@ -28,8 +28,6 @@ class CoreAppConfig(AppConfig): get_permission_change_data, permission_change, post_permission_creation, - required_users, - user_data_required, ) from .views import ( ChatMessageViewSet, @@ -47,6 +45,7 @@ class CoreAppConfig(AppConfig): AutoupdateWebsocketClientMessage, ) from ..utils.websocket import register_client_message + from ..utils.access_permissions import required_user # Collect all config variables before getting the constants. config.collect_config_variables_from_apps() @@ -68,9 +67,6 @@ class CoreAppConfig(AppConfig): permission_change.connect( get_permission_change_data, dispatch_uid='core_get_permission_change_data') - user_data_required.connect( - required_users, - dispatch_uid='core_required_users') post_migrate.connect(call_save_default_values, sender=self, dispatch_uid='core_save_config_default_values') @@ -95,6 +91,9 @@ class CoreAppConfig(AppConfig): register_client_message(GetElementsWebsocketClientMessage()) register_client_message(AutoupdateWebsocketClientMessage()) + # register required_users + required_user.add_collection_string(self.get_model('ChatMessage').get_collection_string(), required_users) + def get_config_variables(self): from .config_variables import get_config_variables return get_config_variables() @@ -154,3 +153,10 @@ class CoreAppConfig(AppConfig): def call_save_default_values(**kwargs): from .config import config config.save_default_values() + + +def required_users(element: Dict[str, Any]) -> Set[int]: + """ + Returns all user ids that are displayed as chatters. + """ + return set(element['user_id']) diff --git a/openslides/core/models.py b/openslides/core/models.py index e80ef2f69..0c5af53f1 100644 --- a/openslides/core/models.py +++ b/openslides/core/models.py @@ -239,6 +239,7 @@ class ChatMessage(RESTModelMixin, models.Model): At the moment we only have one global chat room for managers. """ access_permissions = ChatMessageAccessPermissions() + can_see_permission = 'core.can_use_chat' message = models.TextField() diff --git a/openslides/core/signals.py b/openslides/core/signals.py index 663b41852..f2a77e2f7 100644 --- a/openslides/core/signals.py +++ b/openslides/core/signals.py @@ -4,10 +4,6 @@ from django.contrib.contenttypes.models import ContentType from django.db.models import Q from django.dispatch import Signal -from ..utils.auth import has_perm -from ..utils.collection import Collection -from .models import ChatMessage - # This signal is send when the migrate command is done. That means it is sent # after post_migrate sending and creating all Permission objects. Don't use it @@ -18,12 +14,6 @@ post_permission_creation = Signal() # permission). Connected receivers may yield Collections. permission_change = Signal() -# This signal is sent if someone wants to see basic user data. Connected -# receivers may answer a set of user ids that are required for the request -# user (this can be anything that is allowd as argument for -# utils.auth.has_perm()) e. g. as motion submitter or assignment candidate. -user_data_required = Signal(providing_args=['request_user']) - def delete_django_app_permissions(sender, **kwargs): """ @@ -51,16 +41,3 @@ def get_permission_change_data(sender, permissions, **kwargs): yield core_app.get_model('Countdown') elif permission.codename == 'can_use_chat': yield core_app.get_model('ChatMessage') - - -def required_users(sender, request_user, **kwargs): - """ - Returns all user ids that are displayed as chatters if request_user can - use the chat. This function may return an empty set. - """ - chatters = set() - if has_perm(request_user, 'core.can_use_chat'): - for chat_message_collection_element in Collection(ChatMessage.get_collection_string()).element_generator(): - full_data = chat_message_collection_element.get_full_data() - chatters.add(full_data['user_id']) - return chatters diff --git a/openslides/core/websocket.py b/openslides/core/websocket.py index 8c04d348c..d747e1611 100644 --- a/openslides/core/websocket.py +++ b/openslides/core/websocket.py @@ -45,7 +45,7 @@ class NotifyWebsocketClientMessage(BaseWebsocketClientMessage): "type": "send_notify", "incomming": content, "senderReplyChannelName": consumer.channel_name, - "senderUserId": consumer.scope['user'].id or 0, + "senderUserId": consumer.scope['user'].id if consumer.scope['user'] else 0, }, ) diff --git a/openslides/mediafiles/access_permissions.py b/openslides/mediafiles/access_permissions.py index 93c84c8b4..461e75774 100644 --- a/openslides/mediafiles/access_permissions.py +++ b/openslides/mediafiles/access_permissions.py @@ -1,7 +1,7 @@ from typing import Any, Dict, List, Optional from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import has_perm +from ..utils.auth import async_has_perm from ..utils.collection import CollectionElement @@ -9,11 +9,7 @@ class MediafileAccessPermissions(BaseAccessPermissions): """ Access permissions container for Mediafile and MediafileViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'mediafiles.can_see') + base_permission = 'mediafiles.can_see' def get_serializer_class(self, user=None): """ @@ -23,7 +19,7 @@ class MediafileAccessPermissions(BaseAccessPermissions): return MediafileSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -32,9 +28,9 @@ class MediafileAccessPermissions(BaseAccessPermissions): for the user. Removes hidden mediafiles for some users. """ # Parse data. - if has_perm(user, 'mediafiles.can_see') and has_perm(user, 'mediafiles.can_see_hidden'): + if await async_has_perm(user, 'mediafiles.can_see') and await async_has_perm(user, 'mediafiles.can_see_hidden'): data = full_data - elif has_perm(user, 'mediafiles.can_see'): + elif await async_has_perm(user, 'mediafiles.can_see'): # Exclude hidden mediafiles. data = [full for full in full_data if not full['hidden']] else: diff --git a/openslides/mediafiles/apps.py b/openslides/mediafiles/apps.py index 90d16b3d3..a7c340310 100644 --- a/openslides/mediafiles/apps.py +++ b/openslides/mediafiles/apps.py @@ -1,3 +1,5 @@ +from typing import Any, Dict, Set + from django.apps import AppConfig from ..utils.projector import register_projector_elements @@ -11,11 +13,12 @@ class MediafilesAppConfig(AppConfig): def ready(self): # Import all required stuff. - from openslides.core.signals import permission_change, user_data_required + from openslides.core.signals import permission_change from openslides.utils.rest_api import router from .projector import get_projector_elements - from .signals import get_permission_change_data, required_users + from .signals import get_permission_change_data from .views import MediafileViewSet + from ..utils.access_permissions import required_user # Define projector elements. register_projector_elements(get_projector_elements()) @@ -24,16 +27,25 @@ class MediafilesAppConfig(AppConfig): permission_change.connect( get_permission_change_data, dispatch_uid='mediafiles_get_permission_change_data') - user_data_required.connect( - required_users, - dispatch_uid='mediafiles_required_users') # Register viewsets. router.register(self.get_model('Mediafile').get_collection_string(), MediafileViewSet) + # register required_users + required_user.add_collection_string(self.get_model('Mediafile').get_collection_string(), required_users) + def get_startup_elements(self): """ Yields all Cachables required on startup i. e. opening the websocket connection. """ yield self.get_model('Mediafile') + + +def required_users(element: Dict[str, Any]) -> Set[int]: + """ + Returns all user ids that are displayed as uploaders in any mediafile + if request_user can see mediafiles. This function may return an empty + set. + """ + return set(element['uploader_id']) diff --git a/openslides/mediafiles/models.py b/openslides/mediafiles/models.py index 043f1d5b5..c418ce6cf 100644 --- a/openslides/mediafiles/models.py +++ b/openslides/mediafiles/models.py @@ -14,6 +14,7 @@ class Mediafile(RESTModelMixin, models.Model): Class for uploaded files which can be delivered under a certain url. """ access_permissions = MediafileAccessPermissions() + can_see_permission = 'mediafiles.can_see' mediafile = models.FileField(upload_to='file') """ diff --git a/openslides/mediafiles/signals.py b/openslides/mediafiles/signals.py index 72d1c2f4c..ad297ac19 100644 --- a/openslides/mediafiles/signals.py +++ b/openslides/mediafiles/signals.py @@ -1,9 +1,5 @@ from django.apps import apps -from ..utils.auth import has_perm -from ..utils.collection import Collection -from .models import Mediafile - def get_permission_change_data(sender, permissions=None, **kwargs): """ @@ -14,17 +10,3 @@ def get_permission_change_data(sender, permissions=None, **kwargs): # There could be only one 'mediafiles.can_see' and then we want to return data. if permission.content_type.app_label == mediafiles_app.label and permission.codename == 'can_see': yield from mediafiles_app.get_startup_elements() - - -def required_users(sender, request_user, **kwargs): - """ - Returns all user ids that are displayed as uploaders in any mediafile - if request_user can see mediafiles. This function may return an empty - set. - """ - uploaders = set() - if has_perm(request_user, 'mediafiles.can_see'): - for mediafile_collection_element in Collection(Mediafile.get_collection_string()).element_generator(): - full_data = mediafile_collection_element.get_full_data() - uploaders.add(full_data['uploader_id']) - return uploaders diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 3af2d2f58..069cadadc 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -2,7 +2,7 @@ from copy import deepcopy from typing import Any, Dict, List, Optional from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import has_perm, in_some_groups +from ..utils.auth import async_has_perm, async_in_some_groups from ..utils.collection import CollectionElement @@ -10,11 +10,7 @@ class MotionAccessPermissions(BaseAccessPermissions): """ Access permissions container for Motion and MotionViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -24,7 +20,7 @@ class MotionAccessPermissions(BaseAccessPermissions): return MotionSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -36,7 +32,7 @@ class MotionAccessPermissions(BaseAccessPermissions): personal notes. """ # Parse data. - if has_perm(user, 'motions.can_see'): + if await async_has_perm(user, 'motions.can_see'): # TODO: Refactor this after personal_notes system is refactored. data = [] for full in full_data: @@ -52,8 +48,8 @@ class MotionAccessPermissions(BaseAccessPermissions): required_permission_to_see = full['state_required_permission_to_see'] permission = ( not required_permission_to_see or - has_perm(user, required_permission_to_see) or - has_perm(user, 'motions.can_manage') or + await async_has_perm(user, required_permission_to_see) or + await async_has_perm(user, 'motions.can_manage') or is_submitter) # Parse single motion. @@ -61,7 +57,7 @@ class MotionAccessPermissions(BaseAccessPermissions): full_copy = deepcopy(full) full_copy['comments'] = [] for comment in full['comments']: - if in_some_groups(user, comment['read_groups_id']): + if await async_in_some_groups(user, comment['read_groups_id']): full_copy['comments'].append(comment) data.append(full_copy) else: @@ -74,11 +70,7 @@ class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions): """ Access permissions container for MotionChangeRecommendation and MotionChangeRecommendationViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -88,7 +80,7 @@ class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions): return MotionChangeRecommendationSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -98,8 +90,8 @@ class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions): the can_see permission. """ # Parse data. - if has_perm(user, 'motions.can_see'): - has_manage_perms = has_perm(user, 'motion.can_manage') + if await async_has_perm(user, 'motions.can_see'): + has_manage_perms = await async_has_perm(user, 'motion.can_manage') data = [] for full in full_data: if not full['internal'] or has_manage_perms: @@ -114,11 +106,7 @@ class MotionCommentSectionAccessPermissions(BaseAccessPermissions): """ Access permissions container for MotionCommentSection and MotionCommentSectionViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -128,7 +116,7 @@ class MotionCommentSectionAccessPermissions(BaseAccessPermissions): return MotionCommentSectionSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -137,12 +125,12 @@ class MotionCommentSectionAccessPermissions(BaseAccessPermissions): will be removed, when the user is not in at least one of the read_groups. """ data: List[Dict[str, Any]] = [] - if has_perm(user, 'motions.can_manage'): + if await async_has_perm(user, 'motions.can_manage'): data = full_data else: for full in full_data: read_groups = full.get('read_groups_id', []) - if in_some_groups(user, read_groups): + if await async_in_some_groups(user, read_groups): data.append(full) return data @@ -151,11 +139,7 @@ class StatuteParagraphAccessPermissions(BaseAccessPermissions): """ Access permissions container for StatuteParagraph and StatuteParagraphViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -170,11 +154,7 @@ class CategoryAccessPermissions(BaseAccessPermissions): """ Access permissions container for Category and CategoryViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -189,11 +169,7 @@ class MotionBlockAccessPermissions(BaseAccessPermissions): """ Access permissions container for Category and CategoryViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ @@ -208,11 +184,7 @@ class WorkflowAccessPermissions(BaseAccessPermissions): """ Access permissions container for Workflow and WorkflowViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'motions.can_see') + base_permission = 'motions.can_see' def get_serializer_class(self, user=None): """ diff --git a/openslides/motions/apps.py b/openslides/motions/apps.py index c5ee14299..7ce32bdeb 100644 --- a/openslides/motions/apps.py +++ b/openslides/motions/apps.py @@ -1,3 +1,5 @@ +from typing import Any, Dict, Set + from django.apps import AppConfig from django.db.models.signals import post_migrate @@ -12,13 +14,12 @@ class MotionsAppConfig(AppConfig): def ready(self): # Import all required stuff. - from openslides.core.signals import permission_change, user_data_required + from openslides.core.signals import permission_change from openslides.utils.rest_api import router from .projector import get_projector_elements from .signals import ( create_builtin_workflows, get_permission_change_data, - required_users, ) from .views import ( CategoryViewSet, @@ -31,6 +32,7 @@ class MotionsAppConfig(AppConfig): StateViewSet, WorkflowViewSet, ) + from ..utils.access_permissions import required_user # Define projector elements. register_projector_elements(get_projector_elements()) @@ -42,9 +44,6 @@ class MotionsAppConfig(AppConfig): permission_change.connect( get_permission_change_data, dispatch_uid='motions_get_permission_change_data') - user_data_required.connect( - required_users, - dispatch_uid='motions_required_users') # Register viewsets. router.register(self.get_model('Category').get_collection_string(), CategoryViewSet) @@ -58,6 +57,9 @@ class MotionsAppConfig(AppConfig): router.register(self.get_model('MotionPoll').get_collection_string(), MotionPollViewSet) router.register(self.get_model('State').get_collection_string(), StateViewSet) + # Register required_users + required_user.add_collection_string(self.get_model('Motion').get_collection_string(), required_users) + def get_config_variables(self): from .config_variables import get_config_variables return get_config_variables() @@ -70,3 +72,14 @@ class MotionsAppConfig(AppConfig): for model_name in ('Category', 'StatuteParagraph', 'Motion', 'MotionBlock', 'Workflow', 'MotionChangeRecommendation', 'MotionCommentSection'): yield self.get_model(model_name) + + +def required_users(element: Dict[str, Any]) -> Set[int]: + """ + Returns all user ids that are displayed as as submitter or supporter in + any motion if request_user can see motions. This function may return an + empty set. + """ + submitters_supporters = set([submitter['user_id'] for submitter in element['submitters']]) + submitters_supporters.update(element['supporters_id']) + return submitters_supporters diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 1ff79ee67..c6098dd16 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -93,6 +93,7 @@ class Motion(RESTModelMixin, models.Model): This class is the main entry point to all other classes related to a motion. """ access_permissions = MotionAccessPermissions() + can_see_permission = 'motions.can_see' objects = MotionManager() diff --git a/openslides/motions/signals.py b/openslides/motions/signals.py index e7abc8032..3e7383612 100644 --- a/openslides/motions/signals.py +++ b/openslides/motions/signals.py @@ -1,11 +1,7 @@ -from typing import Set - from django.apps import apps from django.utils.translation import ugettext_noop -from ..utils.auth import has_perm -from ..utils.collection import Collection -from .models import Motion, State, Workflow +from .models import State, Workflow def create_builtin_workflows(sender, **kwargs): @@ -108,19 +104,3 @@ def get_permission_change_data(sender, permissions, **kwargs): # There could be only one 'motions.can_see' and then we want to return data. if permission.content_type.app_label == motions_app.label and permission.codename == 'can_see': yield from motions_app.get_startup_elements() - - -def required_users(sender, request_user, **kwargs): - """ - Returns all user ids that are displayed as as submitter or supporter in - any motion if request_user can see motions. This function may return an - empty set. - """ - submitters_supporters: Set[int] = set() - if has_perm(request_user, 'motions.can_see'): - for motion_collection_element in Collection(Motion.get_collection_string()).element_generator(): - full_data = motion_collection_element.get_full_data() - submitters_supporters.update( - [submitter['user_id'] for submitter in full_data['submitters']]) - submitters_supporters.update(full_data['supporters_id']) - return submitters_supporters diff --git a/openslides/topics/access_permissions.py b/openslides/topics/access_permissions.py index ec9696b7a..49a254a73 100644 --- a/openslides/topics/access_permissions.py +++ b/openslides/topics/access_permissions.py @@ -1,16 +1,11 @@ from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import has_perm class TopicAccessPermissions(BaseAccessPermissions): """ Access permissions container for Topic and TopicViewSet. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - return has_perm(user, 'agenda.can_see') + base_permission = 'agenda.can_see' def get_serializer_class(self, user=None): """ diff --git a/openslides/users/access_permissions.py b/openslides/users/access_permissions.py index a7ce3cac4..b6fc27848 100644 --- a/openslides/users/access_permissions.py +++ b/openslides/users/access_permissions.py @@ -1,22 +1,17 @@ -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Set -from django.contrib.auth.models import AnonymousUser - -from ..core.signals import user_data_required -from ..utils.access_permissions import BaseAccessPermissions -from ..utils.auth import anonymous_is_enabled, has_perm -from ..utils.collection import CollectionElement +from ..utils.access_permissions import BaseAccessPermissions, required_user +from ..utils.auth import async_has_perm +from ..utils.collection import ( + CollectionElement, + get_model_from_collection_string, +) class UserAccessPermissions(BaseAccessPermissions): """ Access permissions container for User and UserViewSet. """ - def check_permissions(self, user): - """ - Every user has read access for their model instnces. - """ - return True def get_serializer_class(self, user=None): """ @@ -26,7 +21,7 @@ class UserAccessPermissions(BaseAccessPermissions): return UserFullSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: @@ -62,9 +57,9 @@ class UserAccessPermissions(BaseAccessPermissions): litte_data_fields.discard('groups') # Check user permissions. - if has_perm(user, 'users.can_see_name'): - if has_perm(user, 'users.can_see_extra_data'): - if has_perm(user, 'users.can_manage'): + if await async_has_perm(user, 'users.can_see_name'): + if await async_has_perm(user, 'users.can_see_extra_data'): + if await async_has_perm(user, 'users.can_manage'): data = [filtered_data(full, all_data_fields) for full in full_data] else: data = [filtered_data(full, many_data_fields) for full in full_data] @@ -73,23 +68,21 @@ class UserAccessPermissions(BaseAccessPermissions): else: # Build a list of users, that can be seen without any permissions (with little fields). - user_ids = set() - # Everybody can see himself. Also everybody can see every user # that is required e. g. as speaker, motion submitter or # assignment candidate. + can_see_collection_strings: Set[str] = set() + for collection_string in required_user.get_collection_strings(): + if await async_has_perm(user, get_model_from_collection_string(collection_string).can_see_permission): + can_see_collection_strings.add(collection_string) + + user_ids = await required_user.get_required_users(can_see_collection_strings) + # Add oneself. if user is not None: user_ids.add(user.id) - # Get a list of all users, that are required by another app. - receiver_responses = user_data_required.send( - sender=self.__class__, - request_user=user) - for receiver, response in receiver_responses: - user_ids.update(response) - # Parse data. data = [ filtered_data(full, litte_data_fields) @@ -104,13 +97,6 @@ class GroupAccessPermissions(BaseAccessPermissions): """ Access permissions container for Groups. Everyone can see them """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - # Every authenticated user can retrieve groups. Anonymous users can do - # so if they are enabled. - return not isinstance(user, AnonymousUser) or anonymous_is_enabled() def get_serializer_class(self, user=None): """ @@ -126,12 +112,6 @@ class PersonalNoteAccessPermissions(BaseAccessPermissions): Access permissions container for personal notes. Every authenticated user can handle personal notes. """ - def check_permissions(self, user): - """ - Returns True if the user has read access model instances. - """ - # Every authenticated user can retrieve personal notes. - return not isinstance(user, AnonymousUser) def get_serializer_class(self, user=None): """ @@ -141,7 +121,7 @@ class PersonalNoteAccessPermissions(BaseAccessPermissions): return PersonalNoteSerializer - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: diff --git a/openslides/users/views.py b/openslides/users/views.py index 84fa779bf..bdf4fe6bc 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -30,7 +30,7 @@ from ..utils.autoupdate import ( inform_data_collection_element_list, ) from ..utils.cache import element_cache -from ..utils.collection import Collection, CollectionElement +from ..utils.collection import CollectionElement from ..utils.rest_api import ( ModelViewSet, Response, @@ -329,9 +329,11 @@ class GroupViewSet(ModelViewSet): if len(new_permissions) > 0: collection_elements: List[CollectionElement] = [] signal_results = permission_change.send(None, permissions=new_permissions, action='added') + all_full_data = async_to_sync(element_cache.get_all_full_data)() for receiver, signal_collections in signal_results: for cachable in signal_collections: - collection_elements.extend(Collection(cachable.get_collection_string()).element_generator()) + for element in all_full_data.get(cachable.get_collection_string(), {}): + collection_elements.append(CollectionElement.from_values(cachable.get_collection_string(), element['id'])) inform_data_collection_element_list(collection_elements) # TODO: Some permissions are deleted. @@ -462,8 +464,10 @@ class UserLoginView(APIView): else: # self.request.method == 'POST' context['user_id'] = self.user.pk - user_collection = CollectionElement.from_instance(self.user) - context['user'] = user_collection.as_dict_for_user(self.user) + context['user'] = async_to_sync(element_cache.get_element_restricted_data)( + CollectionElement.from_instance(self.user), + self.user.get_collection_string(), + self.user.pk) return super().get_context_data(**context) @@ -494,8 +498,10 @@ class WhoAmIView(APIView): """ user_id = self.request.user.pk if user_id is not None: - user_collection = CollectionElement.from_instance(self.request.user) - user_data = user_collection.as_dict_for_user(self.request.user) + user_data = async_to_sync(element_cache.get_element_restricted_data)( + user_to_collection_user(self.request.user), + self.request.user.get_collection_string(), + user_id) else: user_data = None return super().get_context_data( diff --git a/openslides/utils/access_permissions.py b/openslides/utils/access_permissions.py index 309aec9ff..14e49dae5 100644 --- a/openslides/utils/access_permissions.py +++ b/openslides/utils/access_permissions.py @@ -1,8 +1,15 @@ -from typing import Any, Dict, List, Optional +from typing import Any, Callable, Dict, List, Optional, Set +from asgiref.sync import async_to_sync from django.db.models import Model from rest_framework.serializers import Serializer +from .auth import ( + async_anonymous_is_enabled, + async_has_perm, + user_to_collection_user, +) +from .cache import element_cache from .collection import CollectionElement @@ -14,11 +21,30 @@ class BaseAccessPermissions: from this base class for every autoupdate root model. """ + base_permission = '' + """ + Set to a permission the user needs to see the element. + + If this string is empty, all users can see it. + """ + def check_permissions(self, user: Optional[CollectionElement]) -> bool: """ Returns True if the user has read access to model instances. """ - return False + # Convert user to right type + # TODO: Remove this and make sure, that user has always the right type + user = user_to_collection_user(user) + return async_to_sync(self.async_check_permissions)(user) + + async def async_check_permissions(self, user: Optional[CollectionElement]) -> bool: + """ + Returns True if the user has read access to model instances. + """ + if self.base_permission: + return await async_has_perm(user, self.base_permission) + else: + return user is not None or await async_anonymous_is_enabled() def get_serializer_class(self, user: CollectionElement = None) -> Serializer: """ @@ -37,7 +63,7 @@ class BaseAccessPermissions: """ return self.get_serializer_class(user=None)(instance).data - def get_restricted_data( + async def get_restricted_data( self, full_data: List[Dict[str, Any]], user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ @@ -55,4 +81,51 @@ class BaseAccessPermissions: have access restrictions in your view or viewset in methods like retrieve() or list(). """ - return full_data if self.check_permissions(user) else [] + return full_data if await self.async_check_permissions(user) else [] + + +class RequiredUsers: + """ + Helper class to find all users that are required by another element. + """ + + callables: Dict[str, Callable[[Dict[str, Any]], Set[int]]] = {} + + def get_collection_strings(self) -> Set[str]: + """ + Returns all collection strings for elements that could have required users. + """ + return set(self.callables.keys()) + + def add_collection_string(self, collection_string: str, callable: Callable[[Dict[str, Any]], Set[int]]) -> None: + """ + Add a callable for a collection_string to get the required users of the + elements. + """ + self.callables[collection_string] = callable + + async def get_required_users(self, collection_strings: Set[str]) -> Set[int]: + """ + Returns the user ids that are required by other elements. + + Returns only user ids required by elements with a collection_string + in the argument collection_strings. + """ + user_ids: Set[int] = set() + + all_full_data = await element_cache.get_all_full_data() + for collection_string in collection_strings: + # Get the callable for the collection_string + get_user_ids = self.callables.get(collection_string) + elements = all_full_data.get(collection_string, {}) + if not (get_user_ids and elements): + # if the collection_string is unknown or it has no data, do nothing + continue + + for element in elements: + user_ids.update(get_user_ids(element)) + + return user_ids + + +required_user = RequiredUsers() diff --git a/openslides/utils/auth.py b/openslides/utils/auth.py index a957907e4..5cb5e73b9 100644 --- a/openslides/utils/auth.py +++ b/openslides/utils/auth.py @@ -1,5 +1,6 @@ from typing import Dict, List, Optional, Union, cast +from asgiref.sync import async_to_sync from django.apps import apps from django.conf import settings from django.contrib.auth import get_user_model @@ -35,17 +36,28 @@ def has_perm(user: Optional[CollectionElement], perm: str) -> bool: User can be a CollectionElement of a user or None. """ - group_collection_string = 'users/group' # This is the hard coded collection string for openslides.users.models.Group - # Convert user to right type # TODO: Remove this and make use, that user has always the right type user = user_to_collection_user(user) - if user is None and not anonymous_is_enabled(): + return async_to_sync(async_has_perm)(user, perm) + + +async def async_has_perm(user: Optional[CollectionElement], perm: str) -> bool: + """ + Checks that user has a specific permission. + + User can be a CollectionElement of a user or None. + """ + group_collection_string = 'users/group' # This is the hard coded collection string for openslides.users.models.Group + + if user is None and not await async_anonymous_is_enabled(): has_perm = False elif user is None: # Use the permissions from the default group. - default_group = CollectionElement.from_values(group_collection_string, GROUP_DEFAULT_PK) - has_perm = perm in default_group.get_full_data()['permissions'] + default_group = await element_cache.get_element_full_data(group_collection_string, GROUP_DEFAULT_PK) + if default_group is None: + raise RuntimeError('Default Group does not exist.') + has_perm = perm in default_group['permissions'] elif GROUP_ADMIN_PK in user.get_full_data()['groups_id']: # User in admin group (pk 2) grants all permissions. has_perm = True @@ -54,8 +66,11 @@ def has_perm(user: Optional[CollectionElement], perm: str) -> bool: # permission. If the user has no groups, then use the default group. group_ids = user.get_full_data()['groups_id'] or [GROUP_DEFAULT_PK] for group_id in group_ids: - group = CollectionElement.from_values(group_collection_string, group_id) - if perm in group.get_full_data()['permissions']: + group = await element_cache.get_element_full_data(group_collection_string, group_id) + if group is None: + raise RuntimeError('User is in non existing group with id {}.'.format(group_id)) + + if perm in group['permissions']: has_perm = True break else: @@ -78,7 +93,22 @@ def in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool # Convert user to right type # TODO: Remove this and make use, that user has always the right type user = user_to_collection_user(user) - if user is None and not anonymous_is_enabled(): + return async_to_sync(async_in_some_groups)(user, groups) + + +async def async_in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool: + """ + Checks that user is in at least one given group. Groups can be given as a list + of ids. If the user is in the admin group (pk = 2) the result + is always true. + + User can be a CollectionElement of a user or None. + """ + + if not len(groups): + return False # early end here, if no groups are given. + + if user is None and not await async_anonymous_is_enabled(): in_some_groups = False elif user is None: # Use the permissions from the default group. diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 46f4df3cf..0e216334b 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -14,7 +14,7 @@ from typing import ( Type, ) -from asgiref.sync import async_to_sync, sync_to_async +from asgiref.sync import async_to_sync from django.conf import settings from .cache_providers import ( @@ -23,8 +23,8 @@ from .cache_providers import ( MemmoryCacheProvider, RedisCacheProvider, get_all_cachables, - no_redis_dependency, ) +from .redis import use_redis from .utils import get_element_id, get_user_id, split_element_id @@ -60,7 +60,6 @@ class ElementCache: def __init__( self, - redis: str, use_restricted_data_cache: bool = False, cache_provider_class: Type[ElementCacheProvider] = RedisCacheProvider, cachable_provider: Callable[[], List[Cachable]] = get_all_cachables, @@ -71,7 +70,7 @@ class ElementCache: When restricted_data_cache is false, no restricted data is saved. """ self.use_restricted_data_cache = use_restricted_data_cache - self.cache_provider = cache_provider_class(redis) + self.cache_provider = cache_provider_class() self.cachable_provider = cachable_provider self._cachables: Optional[Dict[str, Cachable]] = None @@ -210,8 +209,6 @@ class ElementCache: """ Returns one element as full data. - If the cache is empty, it is created. - Returns None if the element does not exist. """ element = await self.cache_provider.get_element(get_element_id(collection_string, id)) @@ -276,7 +273,7 @@ class ElementCache: mapping = {} for collection_string, full_data in full_data_elements.items(): restricter = self.cachables[collection_string].restrict_elements - elements = await sync_to_async(restricter)(user, full_data) + elements = await restricter(user, full_data) for element in elements: mapping.update( {get_element_id(collection_string, element['id']): @@ -303,7 +300,7 @@ class ElementCache: all_restricted_data = {} for collection_string, full_data in (await self.get_all_full_data()).items(): restricter = self.cachables[collection_string].restrict_elements - elements = await sync_to_async(restricter)(user, full_data) + elements = await restricter(user, full_data) all_restricted_data[collection_string] = elements return all_restricted_data @@ -335,7 +332,7 @@ class ElementCache: restricted_data = {} for collection_string, full_data in changed_elements.items(): restricter = self.cachables[collection_string].restrict_elements - elements = await sync_to_async(restricter)(user, full_data) + elements = await restricter(user, full_data) restricted_data[collection_string] = elements return restricted_data, deleted_elements @@ -358,6 +355,25 @@ class ElementCache: for collection_string, value_list in raw_changed_elements.items()}, deleted_elements) + async def get_element_restricted_data(self, user: Optional['CollectionElement'], collection_string: str, id: int) -> Optional[Dict[str, Any]]: + """ + Returns the restricted_data of one element. + + Returns None, if the element does not exists or the user has no permission to see it. + """ + if not self.use_restricted_data_cache: + full_data = await self.get_element_full_data(collection_string, id) + if full_data is None: + return None + restricter = self.cachables[collection_string].restrict_elements + restricted_data = await restricter(user, [full_data]) + return restricted_data[0] if restricted_data else None + + await self.update_restricted_data(user) + + out = await self.cache_provider.get_element(get_element_id(collection_string, id), get_user_id(user)) + return json.loads(out.decode()) if out else None + async def get_current_change_id(self) -> int: """ Returns the current change id. @@ -383,19 +399,18 @@ class ElementCache: return value -def load_element_cache(redis_addr: str = '', restricted_data: bool = True) -> ElementCache: +def load_element_cache(restricted_data: bool = True) -> ElementCache: """ Generates an element cache instance. """ - if not redis_addr: - return ElementCache(redis='', cache_provider_class=MemmoryCacheProvider) + if use_redis: + cache_provider_class: Type[ElementCacheProvider] = RedisCacheProvider + else: + cache_provider_class = MemmoryCacheProvider - if no_redis_dependency: - raise ImportError("OpenSlides is configured to use redis as cache backend, but aioredis is not installed.") - return ElementCache(redis=redis_addr, use_restricted_data_cache=restricted_data) + return ElementCache(cache_provider_class=cache_provider_class, use_restricted_data_cache=restricted_data) # Set the element_cache -redis_address = getattr(settings, 'REDIS_ADDRESS', '') use_restricted_data = getattr(settings, 'RESTRICTED_DATA_CACHE', True) -element_cache = load_element_cache(redis_addr=redis_address, restricted_data=use_restricted_data) +element_cache = load_element_cache(restricted_data=use_restricted_data) diff --git a/openslides/utils/cache_providers.py b/openslides/utils/cache_providers.py index f5962bd55..869c1202d 100644 --- a/openslides/utils/cache_providers.py +++ b/openslides/utils/cache_providers.py @@ -13,20 +13,18 @@ from typing import ( from django.apps import apps from typing_extensions import Protocol +from .redis import use_redis from .utils import split_element_id, str_dict_to_bytes +if use_redis: + from .redis import get_connection, aioredis + + if TYPE_CHECKING: # Dummy import Collection for mypy, can be fixed with python 3.7 from .collection import CollectionElement # noqa -try: - import aioredis -except ImportError: - no_redis_dependency = True -else: - no_redis_dependency = False - class ElementCacheProvider(Protocol): """ @@ -35,8 +33,6 @@ class ElementCacheProvider(Protocol): See RedisCacheProvider as reverence implementation. """ - def __init__(self, *args: Any) -> None: ... - async def clear_cache(self) -> None: ... async def reset_full_cache(self, data: Dict[str, str]) -> None: ... @@ -57,7 +53,7 @@ class ElementCacheProvider(Protocol): user_id: Optional[int] = None, max_change_id: int = -1) -> Tuple[Dict[str, List[bytes]], List[str]]: ... - async def get_element(self, element_id: str) -> Optional[bytes]: ... + async def get_element(self, element_id: str, user_id: Optional[int] = None) -> Optional[bytes]: ... async def del_restricted_data(self, user_id: int) -> None: ... @@ -76,42 +72,15 @@ class ElementCacheProvider(Protocol): async def get_lowest_change_id(self) -> Optional[int]: ... -class RedisConnectionContextManager: - """ - Async context manager for connections - """ - # TODO: contextlib.asynccontextmanager can be used in python 3.7 - - def __init__(self, redis_address: str) -> None: - self.redis_address = redis_address - - async def __aenter__(self) -> 'aioredis.RedisConnection': - self.conn = await aioredis.create_redis(self.redis_address) - return self.conn - - async def __aexit__(self, exc_type: Any, exc: Any, tb: Any) -> None: - self.conn.close() - - class RedisCacheProvider: """ Cache provider that loads and saves the data to redis. """ - redis_pool: Optional[aioredis.RedisConnection] = None full_data_cache_key: str = 'full_data' restricted_user_cache_key: str = 'restricted_data:{user_id}' change_id_cache_key: str = 'change_id' prefix: str = 'element_cache_' - def __init__(self, redis: str) -> None: - self.redis_address = redis - - def get_connection(self) -> RedisConnectionContextManager: - """ - Returns contextmanager for a redis connection. - """ - return RedisConnectionContextManager(self.redis_address) - def get_full_data_cache_key(self) -> str: return "".join((self.prefix, self.full_data_cache_key)) @@ -125,14 +94,14 @@ class RedisCacheProvider: """ Deleted all cache entries created with this element cache. """ - async with self.get_connection() as redis: + async with get_connection() as redis: await redis.eval("return redis.call('del', 'fake_key', unpack(redis.call('keys', ARGV[1])))", keys=[], args=["{}*".format(self.prefix)]) async def reset_full_cache(self, data: Dict[str, str]) -> None: """ Deletes the full_data_cache and write new data in it. """ - async with self.get_connection() as redis: + async with get_connection() as redis: tr = redis.multi_exec() tr.delete(self.get_full_data_cache_key()) tr.hmset_dict(self.get_full_data_cache_key(), data) @@ -145,7 +114,7 @@ class RedisCacheProvider: If user_id is None, the method tests for full_data. If user_id is an int, it tests for the restricted_data_cache for the user with the user_id. 0 is for anonymous. """ - async with self.get_connection() as redis: + async with get_connection() as redis: if user_id is None: cache_key = self.get_full_data_cache_key() else: @@ -159,7 +128,7 @@ class RedisCacheProvider: elements is a list with an even len. the odd values are the element_ids and the even values are the elements. The elements have to be encoded, for example with json. """ - async with self.get_connection() as redis: + async with get_connection() as redis: await redis.hmset( self.get_full_data_cache_key(), *elements) @@ -173,7 +142,7 @@ class RedisCacheProvider: If user_id is None, the elements are deleted from the full_data cache. If user_id is an int, the elements are deleted one restricted_data_cache. 0 is for anonymous. """ - async with self.get_connection() as redis: + async with get_connection() as redis: if user_id is None: cache_key = self.get_full_data_cache_key() else: @@ -188,7 +157,7 @@ class RedisCacheProvider: Generates and returns the change_id. """ - async with self.get_connection() as redis: + async with get_connection() as redis: return int(await redis.eval( lua_script_change_data, keys=[self.get_change_id_cache_key()], @@ -206,18 +175,24 @@ class RedisCacheProvider: cache_key = self.get_full_data_cache_key() else: cache_key = self.get_restricted_data_cache_key(user_id) - async with self.get_connection() as redis: + + async with get_connection() as redis: return await redis.hgetall(cache_key) - async def get_element(self, element_id: str) -> Optional[bytes]: + async def get_element(self, element_id: str, user_id: Optional[int] = None) -> Optional[bytes]: """ - Returns one element from the full_data_cache. + Returns one element from the cache. Returns None, when the element does not exist. """ - async with self.get_connection() as redis: + if user_id is None: + cache_key = self.get_full_data_cache_key() + else: + cache_key = self.get_restricted_data_cache_key(user_id) + + async with get_connection() as redis: return await redis.hget( - self.get_full_data_cache_key(), + cache_key, element_id) async def get_data_since( @@ -244,7 +219,7 @@ class RedisCacheProvider: # Convert max_change_id to a string. If its negative, use the string '+inf' redis_max_change_id = "+inf" if max_change_id < 0 else str(max_change_id) - async with self.get_connection() as redis: + async with get_connection() as redis: # lua script that returns gets all element_ids from change_id_cache_key # and then uses each element_id on full_data or restricted_data. # It returns a list where the odd values are the change_id and the @@ -282,7 +257,7 @@ class RedisCacheProvider: """ Deletes all restricted_data for an user. 0 is for the anonymous user. """ - async with self.get_connection() as redis: + async with get_connection() as redis: await redis.delete(self.get_restricted_data_cache_key(user_id)) async def set_lock(self, lock_name: str) -> bool: @@ -294,14 +269,14 @@ class RedisCacheProvider: Returns False when the lock was already set. """ # TODO: Improve lock. See: https://redis.io/topics/distlock - async with self.get_connection() as redis: + async with get_connection() as redis: return await redis.setnx("{}lock_{}".format(self.prefix, lock_name), 1) async def get_lock(self, lock_name: str) -> bool: """ Returns True, when the lock for the restricted_data of an user is set. Else False. """ - async with self.get_connection() as redis: + async with get_connection() as redis: return await redis.get("{}lock_{}".format(self.prefix, lock_name)) async def del_lock(self, lock_name: str) -> None: @@ -309,7 +284,7 @@ class RedisCacheProvider: Deletes the lock for the restricted_data of an user. Does nothing when the lock is not set. """ - async with self.get_connection() as redis: + async with get_connection() as redis: await redis.delete("{}lock_{}".format(self.prefix, lock_name)) async def get_change_id_user(self, user_id: int) -> Optional[int]: @@ -318,7 +293,7 @@ class RedisCacheProvider: This is the change_id where the restricted_data was last calculated. """ - async with self.get_connection() as redis: + async with get_connection() as redis: return await redis.hget(self.get_restricted_data_cache_key(user_id), '_config:change_id') async def update_restricted_data(self, user_id: int, data: Dict[str, str]) -> None: @@ -328,14 +303,14 @@ class RedisCacheProvider: data has to be a dict where the key is an element_id and the value the (json-) encoded element. """ - async with self.get_connection() as redis: + async with get_connection() as redis: await redis.hmset_dict(self.get_restricted_data_cache_key(user_id), data) async def get_current_change_id(self) -> List[Tuple[str, int]]: """ Get the highest change_id from redis. """ - async with self.get_connection() as redis: + async with get_connection() as redis: return await redis.zrevrangebyscore( self.get_change_id_cache_key(), withscores=True, @@ -348,7 +323,7 @@ class RedisCacheProvider: Returns None if lowest score does not exist. """ - async with self.get_connection() as redis: + async with get_connection() as redis: return await redis.zscore( self.get_change_id_cache_key(), '_config:lowest_change_id') @@ -364,7 +339,7 @@ class MemmoryCacheProvider: When you use different processes they will use diffrent data. """ - def __init__(self, *args: Any, **kwargs: Any) -> None: + def __init__(self) -> None: self.set_data_dicts() def set_data_dicts(self) -> None: @@ -428,8 +403,13 @@ class MemmoryCacheProvider: return str_dict_to_bytes(cache_dict) - async def get_element(self, element_id: str) -> Optional[bytes]: - value = self.full_data.get(element_id, None) + async def get_element(self, element_id: str, user_id: Optional[int] = None) -> Optional[bytes]: + if user_id is None: + cache_dict = self.full_data + else: + cache_dict = self.restricted_data.get(user_id, {}) + + value = cache_dict.get(element_id, None) return value.encode() if value is not None else None async def get_data_since( @@ -516,7 +496,7 @@ class Cachable(Protocol): Returns all elements of the cachable. """ - def restrict_elements( + async def restrict_elements( self, user: Optional['CollectionElement'], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index b7b3f08d9..917572dc0 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -85,15 +85,6 @@ class CollectionElement: return (self.collection_string == collection_element.collection_string and self.id == collection_element.id) - def as_dict_for_user(self, user: Optional['CollectionElement']) -> Optional[Dict[str, Any]]: - """ - Returns a dict with the data for a user. Can be used for the rest api. - - Returns None if the user does not has the permission to see the element. - """ - restricted_data = self.get_access_permissions().get_restricted_data([self.get_full_data()], user) - return restricted_data[0] if restricted_data else None - def get_model(self) -> Type[Model]: """ Returns the django model that is used for this collection. @@ -106,20 +97,6 @@ class CollectionElement: """ return self.get_model().get_access_permissions() - def get_element_from_db(self) -> Optional[Dict[str, Any]]: - # Hack for django 2.0 and channels 2.1 to stay in the same thread. - # This is needed for the tests. - try: - query = self.get_model().objects.get_full_queryset() - except AttributeError: - # If the model des not have to method get_full_queryset(), then use - # the default queryset from django. - query = self.get_model().objects - try: - return self.get_access_permissions().get_full_data(query.get(pk=self.id)) - except self.get_model().DoesNotExist: - return None - def get_full_data(self) -> Dict[str, Any]: """ Returns the full_data of this collection_element from with all other @@ -151,95 +128,6 @@ class CollectionElement: return self.deleted -class Collection: - """ - Represents all elements of one collection. - """ - - def __init__(self, collection_string: str, full_data: List[Dict[str, Any]] = None) -> None: - """ - Initiates a Collection. A collection_string has to be given. If - full_data (a list of dictionaries) is not given the method - get_full_data() exposes all data by iterating over all - CollectionElements. - """ - self.collection_string = collection_string - self.full_data = full_data - - def get_model(self) -> Type[Model]: - """ - Returns the django model that is used for this collection. - """ - return get_model_from_collection_string(self.collection_string) - - def get_access_permissions(self) -> 'BaseAccessPermissions': - """ - Returns the get_access_permissions object for the this collection. - """ - return self.get_model().get_access_permissions() - - def element_generator(self) -> Generator[CollectionElement, None, None]: - """ - Generator that yields all collection_elements of this collection. - """ - for full_data in self.get_full_data(): - yield CollectionElement.from_values( - self.collection_string, - full_data['id'], - full_data=full_data) - - def get_elements_from_db(self) ->Dict[str, List[Dict[str, Any]]]: - # Hack for django 2.0 and channels 2.1 to stay in the same thread. - # This is needed for the tests. - try: - query = self.get_model().objects.get_full_queryset() - except AttributeError: - # If the model des not have to method get_full_queryset(), then use - # the default queryset from django. - query = self.get_model().objects - return {self.collection_string: [self.get_model().get_access_permissions().get_full_data(instance) for instance in query.all()]} - - def get_full_data(self) -> List[Dict[str, Any]]: - """ - Returns a list of dictionaries with full_data of all collection - elements. - """ - if self.full_data is None: - # The type of all_full_data has to be set for mypy - all_full_data: Dict[str, List[Dict[str, Any]]] = {} - all_full_data = async_to_sync(element_cache.get_all_full_data)() - self.full_data = all_full_data.get(self.collection_string, []) - return self.full_data # type: ignore - - def as_list_for_user(self, user: Optional[CollectionElement]) -> List[Dict[str, Any]]: - """ - Returns a list of dictonaries to send them to a user, for example over - the rest api. - """ - return self.get_access_permissions().get_restricted_data(self.get_full_data(), user) - - def get_collection_string(self) -> str: - """ - Returns the collection_string. - """ - return self.collection_string - - def get_elements(self) -> List[Dict[str, Any]]: - """ - Returns all elements of the Collection as full_data. - """ - return self.get_full_data() - - def restrict_elements( - self, - user: Optional['CollectionElement'], - elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Converts the full_data to restricted data. - """ - return self.get_model().get_access_permissions().get_restricted_data(user, elements) - - _models_to_collection_string: Dict[str, Type[Model]] = {} @@ -268,5 +156,5 @@ def get_model_from_collection_string(collection_string: str) -> Type[Model]: try: model = _models_to_collection_string[collection_string] except KeyError: - raise ValueError('Invalid message. A valid collection_string is missing.') + raise ValueError('Invalid message. A valid collection_string is missing. Got {}'.format(collection_string)) return model diff --git a/openslides/utils/consumers.py b/openslides/utils/consumers.py index 3a7d06aa2..f392e82f3 100644 --- a/openslides/utils/consumers.py +++ b/openslides/utils/consumers.py @@ -23,8 +23,12 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): Sends the startup data to the user. """ + # If the user is the anonymous user, change the value to None + if self.scope['user'].id is None: + self.scope['user'] = None + change_id = None - if not await async_anonymous_is_enabled() and self.scope['user'].id is None: + if not await async_anonymous_is_enabled() and self.scope['user'] is None: await self.close() return @@ -61,7 +65,7 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): """ Send a notify message to the user. """ - user_id = self.scope['user'].id or 0 + user_id = self.scope['user'].id if self.scope['user'] else 0 out = [] for item in event['incomming']: diff --git a/openslides/utils/models.py b/openslides/utils/models.py index f37b95b74..74389a5b3 100644 --- a/openslides/utils/models.py +++ b/openslides/utils/models.py @@ -134,11 +134,11 @@ class RESTModelMixin: return [cls.get_access_permissions().get_full_data(instance) for instance in query.all()] @classmethod - def restrict_elements( + async def restrict_elements( cls, user: Optional['CollectionElement'], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Converts a list of elements from full_data to restricted_data. """ - return cls.get_access_permissions().get_restricted_data(elements, user) + return await cls.get_access_permissions().get_restricted_data(elements, user) diff --git a/openslides/utils/redis.py b/openslides/utils/redis.py new file mode 100644 index 000000000..86baaa86f --- /dev/null +++ b/openslides/utils/redis.py @@ -0,0 +1,37 @@ +from typing import Any + +from django.conf import settings + + +try: + import aioredis +except ImportError: + use_redis = False +else: + # set use_redis to true, if there is a value for REDIS_ADDRESS in the settings + redis_address = getattr(settings, 'REDIS_ADDRESS', '') + use_redis = bool(redis_address) + + +class RedisConnectionContextManager: + """ + Async context manager for connections + """ + # TODO: contextlib.asynccontextmanager can be used in python 3.7 + + def __init__(self, redis_address: str) -> None: + self.redis_address = redis_address + + async def __aenter__(self) -> 'aioredis.RedisConnection': + self.conn = await aioredis.create_redis(self.redis_address) + return self.conn + + async def __aexit__(self, exc_type: Any, exc: Any, tb: Any) -> None: + self.conn.close() + + +def get_connection() -> RedisConnectionContextManager: + """ + Returns contextmanager for a redis connection. + """ + return RedisConnectionContextManager(redis_address) diff --git a/openslides/utils/rest_api.py b/openslides/utils/rest_api.py index 36a9f3b72..cbd1578e8 100644 --- a/openslides/utils/rest_api.py +++ b/openslides/utils/rest_api.py @@ -1,6 +1,7 @@ from collections import OrderedDict from typing import Any, Dict, Iterable, Optional, Type +from asgiref.sync import async_to_sync from django.http import Http404 from rest_framework import status from rest_framework.decorators import detail_route, list_route @@ -42,7 +43,7 @@ from rest_framework.viewsets import ( from .access_permissions import BaseAccessPermissions from .auth import user_to_collection_user -from .collection import Collection, CollectionElement +from .cache import element_cache __all__ = ['detail_route', 'DecimalField', 'list_route', 'SimpleMetadata', @@ -187,11 +188,6 @@ class ModelSerializer(_ModelSerializer): class ListModelMixin(_ListModelMixin): """ Mixin to add the caching system to list requests. - - It is not allowed to use the method get_queryset() in derivated classes. - The attribute queryset has to be used in the following form: - - queryset = Model.objects.all() """ def list(self, request: Any, *args: Any, **kwargs: Any) -> Response: model = self.get_queryset().model @@ -201,20 +197,14 @@ class ListModelMixin(_ListModelMixin): # The corresponding queryset does not support caching. response = super().list(request, *args, **kwargs) else: - collection = Collection(collection_string) - user = user_to_collection_user(request.user) - response = Response(collection.as_list_for_user(user)) + all_restricted_data = async_to_sync(element_cache.get_all_restricted_data)(user_to_collection_user(request.user)) + response = Response(all_restricted_data.get(collection_string, [])) return response class RetrieveModelMixin(_RetrieveModelMixin): """ Mixin to add the caching system to retrieve requests. - - It is not allowed to use the method get_queryset() in derivated classes. - The attribute queryset has to be used in the following form: - - queryset = Model.objects.all() """ def retrieve(self, request: Any, *args: Any, **kwargs: Any) -> Response: model = self.get_queryset().model @@ -225,15 +215,10 @@ class RetrieveModelMixin(_RetrieveModelMixin): response = super().retrieve(request, *args, **kwargs) else: lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field - collection_element = CollectionElement.from_values( - collection_string, self.kwargs[lookup_url_kwarg]) user = user_to_collection_user(request.user) - try: - content = collection_element.as_dict_for_user(user) - except collection_element.get_model().DoesNotExist: - raise Http404 + content = async_to_sync(element_cache.get_element_restricted_data)(user, collection_string, self.kwargs[lookup_url_kwarg]) if content is None: - self.permission_denied(request) + raise Http404 response = Response(content) return response diff --git a/tests/conftest.py b/tests/conftest.py index cc7364fc2..3bb7d4d21 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import pytest +from asgiref.sync import async_to_sync from django.test import TestCase, TransactionTestCase from pytest_django.django_compat import is_django_unittest from pytest_django.plugin import validate_django_db @@ -68,4 +69,5 @@ def reset_cache(request): """ if 'django_db' in request.node.keywords or is_django_unittest(request): # When the db is created, use the original cachables + async_to_sync(element_cache.cache_provider.clear_cache)() element_cache.ensure_cache(reset=True) diff --git a/tests/integration/agenda/test_viewset.py b/tests/integration/agenda/test_viewset.py index ec7232d60..c1aa10067 100644 --- a/tests/integration/agenda/test_viewset.py +++ b/tests/integration/agenda/test_viewset.py @@ -42,7 +42,7 @@ class RetrieveItem(TestCase): def test_hidden_by_anonymous_without_manage_perms(self): response = self.client.get(reverse('item-detail', args=[self.item.pk])) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, 404) def test_hidden_by_anonymous_with_manage_perms(self): group = Group.objects.get(pk=1) # Group with pk 1 is for anonymous users. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 439decd29..38b6c9f46 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -27,7 +27,7 @@ class TConfig: config.key_to_id[item.name] = id+1 return elements - def restrict_elements( + async def restrict_elements( self, user: Optional['CollectionElement'], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: @@ -50,7 +50,7 @@ class TUser: 'last_email_send': None, 'comment': '', 'is_active': True, 'default_password': 'admin', 'session_auth_hash': '362d4f2de1463293cb3aaba7727c967c35de43ee'}] - def restrict_elements( + async def restrict_elements( self, user: Optional['CollectionElement'], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 143bc67b7..9865e8f3d 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -428,7 +428,7 @@ class RetrieveMotion(TestCase): inform_changed_data(self.motion) response = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, 404) def test_admin_state_with_required_permission_to_see(self): state = self.motion.state @@ -461,6 +461,7 @@ class RetrieveMotion(TestCase): config['general_system_enable_anonymous'] = True guest_client = APIClient() inform_changed_data(group) + inform_changed_data(self.motion) response_1 = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) self.assertEqual(response_1.status_code, status.HTTP_200_OK) @@ -473,7 +474,7 @@ class RetrieveMotion(TestCase): password='password_ooth7taechai5Oocieya') response_3 = guest_client.get(reverse('user-detail', args=[extra_user.pk])) - self.assertEqual(response_3.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response_3.status_code, 404) class UpdateMotion(TestCase): @@ -983,6 +984,7 @@ class TestMotionCommentSection(TestCase): section.save() response = self.client.get(reverse('motioncommentsection-list')) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertTrue(isinstance(response.data, list)) self.assertEqual(len(response.data), 1) @@ -995,10 +997,12 @@ class TestMotionCommentSection(TestCase): """ self.admin.groups.remove(self.group_in) # group_in has motions.can_manage permission self.admin.groups.add(self.group_out) # group_out does not. + inform_changed_data(self.admin) section = MotionCommentSection(name='test_name_f3mMD28LMcm29Coelwcm') section.save() section.read_groups.add(self.group_out, self.group_in) + inform_changed_data(section) response = self.client.get(reverse('motioncommentsection-list')) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 6eb270465..994c0b34a 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -69,7 +69,7 @@ class UserGetTest(TestCase): response = guest_client.get('/rest/users/user/1/') - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) class UserCreate(TestCase): @@ -535,7 +535,7 @@ class PersonalNoteTest(TestCase): config['general_system_enable_anonymous'] = True guest_client = APIClient() response = guest_client.get(reverse('personalnote-detail', args=[personal_note.pk])) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, 404) def test_admin_send_JSON(self): admin_client = APIClient() diff --git a/tests/integration/utils/test_collection.py b/tests/integration/utils/test_collection.py index 873d43fc8..d2c7f8d2f 100644 --- a/tests/integration/utils/test_collection.py +++ b/tests/integration/utils/test_collection.py @@ -26,37 +26,3 @@ class TestCollectionElementCache(TestCase): """ with self.assertRaises(Topic.DoesNotExist): collection.CollectionElement.from_values('topics/topic', 999) - - -class TestCollectionCache(TestCase): - def test_with_cache(self): - """ - Tests that no db query is used when the list is received twice. - """ - Topic.objects.create(title='test topic1') - Topic.objects.create(title='test topic2') - Topic.objects.create(title='test topic3') - topic_collection = collection.Collection('topics/topic') - list(topic_collection.get_full_data()) - - with self.assertNumQueries(0): - instance_list = list(topic_collection.get_full_data()) - self.assertEqual(len(instance_list), 3) - - def test_deletion(self): - """ - When an element is deleted, the cache should be updated automaticly via - the autoupdate system. So there should be no db queries. - """ - Topic.objects.create(title='test topic1') - Topic.objects.create(title='test topic2') - topic3 = Topic.objects.create(title='test topic3') - topic_collection = collection.Collection('topics/topic') - list(topic_collection.get_full_data()) - - collection.CollectionElement.from_instance(topic3, deleted=True) - topic3.delete() - - with self.assertNumQueries(0): - instance_list = list(collection.Collection('topics/topic').get_full_data()) - self.assertEqual(len(instance_list), 2) diff --git a/tests/settings.py b/tests/settings.py index 765edc90e..4338177a0 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -75,3 +75,6 @@ MOTION_IDENTIFIER_MIN_DIGITS = 1 PASSWORD_HASHERS = [ 'django.contrib.auth.hashers.MD5PasswordHasher', ] + +# Deactivate restricted_data_cache +RESTRICTED_DATA_CACHE = False diff --git a/tests/unit/users/test_access_permissions.py b/tests/unit/users/test_access_permissions.py deleted file mode 100644 index 1561c7a84..000000000 --- a/tests/unit/users/test_access_permissions.py +++ /dev/null @@ -1,20 +0,0 @@ -from unittest import TestCase - -from openslides.users.access_permissions import PersonalNoteAccessPermissions -from openslides.utils.collection import CollectionElement - - -class TestPersonalNoteAccessPermissions(TestCase): - def test_get_restricted_data(self): - ap = PersonalNoteAccessPermissions() - rd = ap.get_restricted_data( - [{'user_id': 1}], - CollectionElement.from_values('users/user', 5, full_data={})) - self.assertEqual(rd, []) - - def test_get_restricted_data_for_anonymous(self): - ap = PersonalNoteAccessPermissions() - rd = ap.get_restricted_data( - [{'user_id': 1}], - None) - self.assertEqual(rd, []) diff --git a/tests/unit/utils/cache_provider.py b/tests/unit/utils/cache_provider.py index 4c0ead6fd..7e57e5767 100644 --- a/tests/unit/utils/cache_provider.py +++ b/tests/unit/utils/cache_provider.py @@ -32,7 +32,7 @@ class Collection1: {'id': 1, 'value': 'value1'}, {'id': 2, 'value': 'value2'}] - def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + async def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: return restrict_elements(user, elements) @@ -45,7 +45,7 @@ class Collection2: {'id': 1, 'key': 'value1'}, {'id': 2, 'key': 'value2'}] - def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + async def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: return restrict_elements(user, elements) diff --git a/tests/unit/utils/test_cache.py b/tests/unit/utils/test_cache.py index f90ac7623..43b505390 100644 --- a/tests/unit/utils/test_cache.py +++ b/tests/unit/utils/test_cache.py @@ -30,7 +30,6 @@ def sort_dict(encoded_dict: Dict[str, List[Dict[str, Any]]]) -> Dict[str, List[D @pytest.fixture def element_cache(): element_cache = ElementCache( - 'test_redis', cache_provider_class=TTestCacheProvider, cachable_provider=get_cachable_provider(), start_time=0)