From c9fb64e04320e77e42565cc756a22242ddb2ccd1 Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Fri, 17 Feb 2023 17:58:47 +0100 Subject: [PATCH 1/7] Change permissions logic --- CHANGELOG.md | 4 +- concrete_datastore/admin/admin.py | 32 ++ concrete_datastore/api/v1/permissions.py | 302 ++++++++++++++++- concrete_datastore/api/v1/views.py | 10 +- .../concrete/automation/signal_processor.py | 184 +++++++++- .../concrete/automation/tasks.py | 151 ++++++++- concrete_datastore/concrete/models.py | 47 +++ development/settings.py | 38 +++ .../0013_instancepermission_systemversion.py | 38 +++ tests/test_instance_permission.py | 317 ++++++++++++++++++ tests/unittest_settings.py | 6 +- 11 files changed, 1097 insertions(+), 32 deletions(-) create mode 100644 tests/migrations/0013_instancepermission_systemversion.py create mode 100644 tests/test_instance_permission.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 69b3f4a8..291e7528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,11 @@ ### Added -- nothing added +- Add model InstancePermission to determine read and write permissions for a user ### Changed -- nothing changed +- Changed the logic of read/write permissions by user: A user that has read/write access to an instance of the model Model, will have an instance of InstancePermission (with the model_name="Model") and the uid of the instance will be n the read/write_instance_uids ### Removed diff --git a/concrete_datastore/admin/admin.py b/concrete_datastore/admin/admin.py index ffa43600..2cddfee3 100644 --- a/concrete_datastore/admin/admin.py +++ b/concrete_datastore/admin/admin.py @@ -12,6 +12,7 @@ AuthToken, ConcretePermission, EmailDevice, + InstancePermission, ) from concrete_datastore.concrete.models import ( divider, @@ -230,3 +231,34 @@ class EmailDeviceAdmin(SaveModelMixin, admin.ModelAdmin): 'modification_date', ] list_filter = ['mfa_mode', 'confirmed'] + + +@admin.register(InstancePermission, site=admin_site) +class InstancePermissionAdmin(SaveModelMixin, admin.ModelAdmin): + list_display = [ + 'uid', + 'user', + 'model_name', + 'creation_date', + 'modification_date', + ] + search_fields = ['user__email'] + readonly_fields = [ + 'uid', + 'user', + 'model_name', + 'creation_date', + 'modification_date', + ] + date_hierarchy = 'creation_date' + + fields = [ + 'uid', + 'user', + 'model_name', + 'read_instance_uids', + 'write_instance_uids', + 'creation_date', + 'modification_date', + ] + list_filter = ['model_name'] diff --git a/concrete_datastore/api/v1/permissions.py b/concrete_datastore/api/v1/permissions.py index 7af6ceea..1a0516fd 100644 --- a/concrete_datastore/api/v1/permissions.py +++ b/concrete_datastore/api/v1/permissions.py @@ -1,19 +1,22 @@ # coding: utf-8 from importlib import import_module - +import logging +import warnings +from copy import deepcopy from django.db.models import Q from django.conf import settings from django.contrib.auth import get_user_model from rest_framework.exceptions import APIException from rest_framework import permissions, status - from concrete_datastore.concrete.models import ( ConcretePermission, + InstancePermission, DIVIDER_MODEL, UNDIVIDED_MODEL, ) +logger = logging.getLogger(__name__) DIVIDER_MODELs = "{}s".format(DIVIDER_MODEL) DIVIDER_MODELs_LOWER = DIVIDER_MODELs.lower() @@ -253,20 +256,19 @@ def has_object_permission(self, request, view, obj): else: return False else: - return authenticated and ( - (obj.created_by is not None and obj.created_by.pk == user.pk) - or (user.is_at_least_admin) - or ( - obj.can_admin_users.filter(pk=user.pk).exists() - or does_intersect( - obj.can_admin_groups, user.concrete_groups - ) - ) - or ( - at_least_staff - and self.check_divider_permission(request, obj) is True + if authenticated is False: + return False + if user.is_at_least_admin: + return True + + try: + model_name = obj._meta.model.__name__ + instance_permissions = InstancePermission.objects.get( + user_id=user.pk, model_name=model_name ) - ) + except InstancePermission.DoesNotExist: + return False + return str(obj.pk) in instance_permissions.write_instance_uids def get_available_scope_pks_for(user): @@ -288,6 +290,10 @@ def apply_scope_filters(user, queryset): def filter_queryset_by_divider(queryset, user, divider): + warnings.warn( + 'The method "filter_queryset_by_permissions" is deprecated', + DeprecationWarning, + ) """ queryset is the one of the current view, user is the connected one or anonymous @@ -312,6 +318,10 @@ def filter_queryset_by_divider(queryset, user, divider): def filter_queryset_by_permissions(queryset, user, divider): + warnings.warn( + 'The method "filter_queryset_by_permissions" is deprecated', + DeprecationWarning, + ) if queryset.model == get_user_model(): raise ValueError( "Queryset of model User cannot be filtered by permissions" @@ -374,3 +384,265 @@ def filter_queryset_by_permissions(queryset, user, divider): queryset = queryset.distinct() return queryset + + +def filter_queryset_by_permissions_and_scope(queryset, user, divider=None): + if user.is_anonymous: + return queryset.filter(public=True) + model_name = queryset.model.__name__ + kwargs = {} + if divider is not None: + kwargs[DIVIDER_MODEL_LOWER] = divider + if user.is_authenticated and user.is_at_least_admin: + qs = queryset + else: + try: + instance_permission = user.instance_permissions.get( + model_name=model_name + ) + filters = Q(pk__in=instance_permission.read_instance_uids) | Q( + public=True + ) + except InstancePermission.DoesNotExist: + filters = Q(public=True) + qs = queryset.filter(filters) + if user.is_at_least_staff: + model_is_divided = model_name not in UNDIVIDED_MODEL + if model_is_divided: + divider_query = "{}_id__in".format(DIVIDER_MODEL_LOWER) + all_divider_pk = get_available_scope_pks_for(user) + qs |= queryset.filter(**{divider_query: all_divider_pk}) + + return qs.filter(**kwargs) + + +def get_read_write_permission_users( + user, + instances_qs, + user_groups_pks, + include_divider=True, + include_view_groups=True, + include_admin_groups=True, + include_view_users=True, + include_admin_users=True, +): + user_id = user.pk + created_by_qs = instances_qs.filter(created_by_id=user_id).values_list( + 'pk', flat=True + ) + can_view_users_qs = instances_qs.filter( + can_view_users__pk=user_id + ).values_list('pk', flat=True) + can_view_groups_qs = instances_qs.filter( + can_view_groups__pk__in=user_groups_pks + ).values_list('pk', flat=True) + + can_admin_users_qs = instances_qs.filter( + can_admin_users__pk=user_id + ).values_list('pk', flat=True) + can_admin_groups_qs = instances_qs.filter( + can_admin_groups__pk__in=user_groups_pks + ).values_list('pk', flat=True) + write_instances_uids = created_by_qs + + if include_admin_groups is True: + write_instances_uids = write_instances_uids.union(can_admin_groups_qs) + + if include_admin_users is True: + write_instances_uids = write_instances_uids.union(can_admin_users_qs) + + #: A manager has write permissions on his scope + model_name = instances_qs.model.__name__ + if ( + (user.is_at_least_staff is True) + and (model_name not in UNDIVIDED_MODEL) + and (include_divider is True) + ): + all_divider_pk = getattr(user, DIVIDER_MODELs_LOWER).values_list( + 'pk', flat=True + ) + divider_field_pk = f"{DIVIDER_MODEL_LOWER}_id__in" + divided_qs = instances_qs.filter( + **{divider_field_pk: all_divider_pk} + ).values_list('pk', flat=True) + write_instances_uids = write_instances_uids.union(divided_qs) + + read_instances_uids = write_instances_uids + if include_view_groups is True: + read_instances_uids = read_instances_uids.union(can_view_groups_qs) + if include_view_users is True: + read_instances_uids = read_instances_uids.union(can_view_users_qs) + + write_instances_uids_set = set( + map(str, set(filter(lambda x: x is not None, write_instances_uids))) + ) + read_instances_uids_set = set( + map(str, set(filter(lambda x: x is not None, read_instances_uids))) + ) + return read_instances_uids_set, write_instances_uids_set + + +def add_or_remove_element_into_list( + elt, current_list, check_list, has_changes=False +): + """ + Adds or removes an element from a list: + - if the element exists in the check list, and is not in the current_list + it is added to the curent_list + - if the element does not exist in the check_list and exists in the + current_list, it should be removed + + example 1: + - elt : 5 + - current_list : [1, 2, 3, 4, 5] + - check_list : [1, 2, 3] + => Result: the element 5 should be removed from the current_list + + example 2: + - elt : 5 + - current_list : [1, 2, 3, 4] + - check_list : [1, 2, 3, 5] + => Result: the element 5 should be added to the current_list + + Arguments + --------- + :elt: the element to add/remove + :current_list: the list that contains the initial elements, and will + contain the final result + :check_list:: the list used to check whether the element should be added + or removed from the list + :has_changes: a boolean that is called recursively by the main method + to describe whether the data changed + + Returns + ------- + :has_changes: a boolean that describes whether the data has changed + :current_list: the list of the updated elements + """ + if elt in current_list and elt not in check_list: + has_changes = True + current_list.remove(elt) + elif elt not in current_list and elt in check_list: + has_changes = True + current_list.append(elt) + return has_changes, current_list + + +def update_instance_permission_uids( + perm_instance, new_read_instance_uids, new_write_instance_uids, all_uids +): + """ + Given a list of all_uids, this methods checks if the permission instance + should be updated or not. + If an element exists in both the all_uids and in the + new_read/write_instance_uids, it should also exist in the read/write uids + of the permission instance. + On the other side, if an element exists in the all_uids but does not exist + in the new_read/write_instance_uids, it should not exist in the read/write + uids of the permission instance. + The read_/write uids of the permission instance that do not appear in + all_uids should not be removed + + Example: + if all_uids is [1, 2, 3, 4] + the read uids of the instance are [1, 2, 5, 6] + and the new read uids are [1, 3, 4] + We can see that the element 2 exists in all_uids and the read uids of the + permission instance, but not in the new read uids, so it should be removed + The elements 5 and 6 are in the read uid of the permission instance, but + are neither in the new read uids, nor in all_uids, so they should not be + removed. The final result of the red_uids are [1, 3, 4, 5, 6] + """ + read_instance_uids = deepcopy(perm_instance.read_instance_uids) + write_instance_uids = deepcopy(perm_instance.write_instance_uids) + read_permission_changed = False + write_permission_changed = False + for uid in all_uids: + uid_str = str(uid) + #: Handle read instance uids + read_permission_changed, read_instance_uids = add_or_remove_element_into_list( + elt=uid_str, + current_list=read_instance_uids, + check_list=new_read_instance_uids, + has_changes=read_permission_changed, + ) + + #: Handle write instance uids + write_permission_changed, write_instance_uids = add_or_remove_element_into_list( + elt=uid_str, + current_list=write_instance_uids, + check_list=new_write_instance_uids, + has_changes=write_permission_changed, + ) + + if read_permission_changed or write_permission_changed: + if read_permission_changed: + perm_instance.read_instance_uids = read_instance_uids + if write_permission_changed: + perm_instance.write_instance_uids = write_instance_uids + + logger.debug( + f'Updating permissions for {perm_instance.user.email} on ' + f'instance <{all_uids.model.__name__}>' + ) + perm_instance.save() + + +def create_or_update_instance_permission_per_user( + user, + instances_qs, + include_divider=True, + include_view_groups=True, + include_admin_groups=True, + include_view_users=True, + include_admin_users=True, +): + if instances_qs.exists() is False: + return + if user.is_at_least_admin: + return + model_name = instances_qs.model.__name__ + user_groups_pks = user.concrete_groups.values_list('pk', flat=True) + read_instances_uids, write_instances_uids = get_read_write_permission_users( + user=user, + instances_qs=instances_qs, + user_groups_pks=user_groups_pks, + include_divider=include_divider, + include_view_groups=include_view_groups, + include_admin_groups=include_admin_groups, + include_view_users=include_view_users, + include_admin_users=include_admin_users, + ) + perm_instance, _ = InstancePermission.objects.get_or_create( + user=user, model_name=model_name + ) + + update_instance_permission_uids( + perm_instance=perm_instance, + new_read_instance_uids=read_instances_uids, + new_write_instance_uids=write_instances_uids, + all_uids=instances_qs.values_list('pk', flat=True), + ) + + +def update_created_by_permissions(instance, user): + #: The creator of the instance has the read and write access + #: if the minimal levels on the datamodel allow it + model = instance._meta.model + model_name = model.__name__ + has_read_permission = check_minimum_level('GET', user, model) + has_write_permission = check_minimum_level('PATCH', user, model) + defaults = {} + if has_read_permission is True: + defaults['read_instance_uids'] = [str(instance.pk)] + if has_write_permission is True: + defaults['write_instance_uids'] = [str(instance.pk)] + if not defaults: + return + permission_instance, created = InstancePermission.objects.get_or_create( + user=user, model_name=model_name, defaults=defaults + ) + if created is False: + for field_name, field_value in defaults.items(): + getattr(permission_instance, field_name).extend(field_value) + permission_instance.save() diff --git a/concrete_datastore/api/v1/views.py b/concrete_datastore/api/v1/views.py index bd8f68db..453dee23 100644 --- a/concrete_datastore/api/v1/views.py +++ b/concrete_datastore/api/v1/views.py @@ -59,8 +59,7 @@ from concrete_datastore.api.v1.permissions import ( UserAtLeastAuthenticatedPermission, UserAccessPermission, - filter_queryset_by_permissions, - filter_queryset_by_divider, + filter_queryset_by_permissions_and_scope, ) from concrete_datastore.api.v1.responses import ConcreteBadResponse from concrete_datastore.api.v1.pagination import ExtendedPagination @@ -2122,17 +2121,12 @@ def get_queryset(self): user_filters.update(public=True) return UserModel.objects.filter(**user_filters) - queryset = filter_queryset_by_permissions( + queryset = filter_queryset_by_permissions_and_scope( queryset=self.model_class.objects.all(), user=self.request.user, divider=divider, ) - if divider is not None: - queryset = filter_queryset_by_divider( - queryset=queryset, user=self.request.user, divider=divider - ) - if self.request.method in permissions.SAFE_METHODS: rel_fields = self.rel_single_fields + self.rel_iterable_fields rel_fields += [ diff --git a/concrete_datastore/concrete/automation/signal_processor.py b/concrete_datastore/concrete/automation/signal_processor.py index d0397bdb..45e12687 100644 --- a/concrete_datastore/concrete/automation/signal_processor.py +++ b/concrete_datastore/concrete/automation/signal_processor.py @@ -1,21 +1,34 @@ # coding: utf-8 +import sys import os import logging from django.conf import settings from django.dispatch import receiver -from django.db.models.signals import pre_delete, post_save +from django.db.models.signals import pre_delete, post_save, m2m_changed from django.contrib.auth import get_user_model - import concrete_datastore.concrete.models from concrete_datastore.concrete.models import DIVIDER_MODEL +from concrete_datastore.concrete.automation.tasks import ( + on_update_divider_async, + on_update_group_members_async, + on_create_instance_async, + on_view_admin_groups_changed_async, + on_view_admin_users_changed_async, +) from concrete_datastore.api.v1.views import ( remove_instances_user_tracked_fields, ) +from concrete_datastore.concrete.meta import meta_models logger = logging.getLogger(__name__) +DIVIDER_MODELs = "{}s".format(DIVIDER_MODEL) +DIVIDER_MODELs_LOWER = DIVIDER_MODELs.lower() +DIVIDER_MODEL_LOWER = DIVIDER_MODEL.lower() + + @receiver(pre_delete) def on_pre_delete(sender, instance, **kwargs): model_name = instance.__class__.__name__ @@ -72,3 +85,170 @@ def on_post_save(sender, instance, **kwargs): remove_instances_user_tracked_fields(instance, user_dividers) divider_manager.clear() instance.concrete_groups.clear() + + +#: If user dividers changed +@receiver( + m2m_changed, + sender=getattr( + concrete_datastore.concrete.models.User, DIVIDER_MODELs_LOWER + ).through, +) +def on_update_divider(sender, instance, action, pk_set, **kwargs): + if action not in ('post_add', 'post_remove', 'pre_clear'): + return + + if action == 'pre_clear': + pk_set = getattr(instance, DIVIDER_MODELs_LOWER).values_list( + 'pk', flat=True + ) + if pk_set is None or len(pk_set) == 0: + #: Nothing to add/remove + return + on_update_divider_async.apply_async( + kwargs={ + 'include_divider': action != 'pre_clear', + 'pk_set': list(map(str, pk_set)), + 'instance_uid': str(instance.pk), + } + ) + + +#: If concrete groups changed members +@receiver( + m2m_changed, + sender=concrete_datastore.concrete.models.Group.members.through, +) +def on_update_group_members(sender, instance, action, pk_set, **kwargs): + #: Can be done either from the group.memebers fields, or from + #: user.concrete_groups reverse + if action not in ('post_add', 'post_remove', 'pre_clear'): + return + model_name = instance._meta.model.__name__ + if action == 'pre_clear': + if model_name == 'Group': + pk_set = instance.members.values_list('pk', flat=True) + else: + pk_set = instance.concrete_groups.values_list('pk', flat=True) + + if pk_set is None or len(pk_set) == 0: + #: Nothing to add/remove + return + on_update_group_members_async.apply_async( + kwargs={ + 'include_pks': action != 'pre_clear', + 'instance_model_name': model_name, + 'instance_uid': instance.pk, + 'pk_set': list(map(str, pk_set)), + } + ) + + +for meta_model in meta_models: + model_name = meta_model.get_dashed_case_class_name().replace('-', '_') + if model_name in ('user', 'group', 'email'): + continue + concrete_model = getattr( + concrete_datastore.concrete.models, meta_model.get_model_name() + ) + + @receiver(post_save, sender=concrete_model) + def on_create_instance(instance, created, *args, **kwargs): + #: Assign permissions to the creator + if created is False: + return + user = instance.created_by + on_create_instance_async.apply_async( + kwargs={ + 'user_pk': None if user is None else str(user.pk), + 'model_name': instance._meta.model.__name__, + 'instance_pk': str(instance.pk), + } + ) + + #: Signals for can_view/admin_groups + @receiver( + m2m_changed, sender=getattr(concrete_model, 'can_view_groups').through + ) + @receiver( + m2m_changed, sender=getattr(concrete_model, 'can_admin_groups').through + ) + def on_view_admin_groups_changed( + sender, instance, action, pk_set, *args, **kwargs + ): + model_name = instance._meta.model.__name__ + field_name = sender.__name__.replace(f'{model_name}_', '') + include_admin_groups = True + include_view_groups = True + if action == 'pre_clear': + pk_set = getattr(instance, field_name).values_list('pk', flat=True) + #: If the field name is 'can_view_groups' it means that we should + #: include the can_admin_groups and exclude the can_view_users + #: and vice versa + include_admin_groups = field_name == 'can_view_groups' + include_view_groups = field_name == 'can_admin_groups' + if pk_set is None or len(pk_set) == 0: + #: Nothing to add/remove + return + if action not in ('post_add', 'post_remove', 'pre_clear'): + return + on_view_admin_groups_changed_async.apply_async( + kwargs={ + 'include_admin_groups': include_admin_groups, + 'include_view_groups': include_view_groups, + 'pk_set': list(map(str, pk_set)), + 'model_name': model_name, + 'instance_pk': str(instance.pk), + } + ) + + #: Signals for can_view/admin_users + @receiver( + m2m_changed, sender=getattr(concrete_model, 'can_view_users').through + ) + @receiver( + m2m_changed, sender=getattr(concrete_model, 'can_admin_users').through + ) + def on_view_admin_users_changed( + sender, instance, action, pk_set, *args, **kwargs + ): + model_name = instance._meta.model.__name__ + field_name = sender.__name__.replace(f'{model_name}_', '') + include_admin_users = True + include_view_users = True + if action == 'pre_clear': + pk_set = getattr(instance, field_name).values_list('pk', flat=True) + #: If the field name is 'can_view_users' it means that we should + #: include the can_admin_users and exclude the can_view_users + #: and vice versa + include_admin_users = field_name == 'can_view_users' + include_view_users = field_name == 'can_admin_users' + if pk_set is None or len(pk_set) == 0: + #: Nothing to add/remove + return + if action not in ('post_add', 'post_remove', 'pre_clear'): + return + model_name = instance._meta.model.__name__ + on_view_admin_users_changed_async.apply_async( + kwargs={ + 'include_admin_users': include_admin_users, + 'include_view_users': include_view_users, + 'pk_set': list(map(str, pk_set)), + 'model_name': model_name, + 'instance_pk': str(instance.pk), + } + ) + + setattr( + sys.modules[__name__], f'on_create_{model_name}', on_create_instance + ) + setattr( + sys.modules[__name__], + f'on_view_admin_groups_changed_{model_name}', + on_view_admin_groups_changed, + ) + setattr( + sys.modules[__name__], + f'on_view_admin_users_changed_{model_name}', + on_view_admin_users_changed, + ) diff --git a/concrete_datastore/concrete/automation/tasks.py b/concrete_datastore/concrete/automation/tasks.py index 2fd8b743..a82590a1 100644 --- a/concrete_datastore/concrete/automation/tasks.py +++ b/concrete_datastore/concrete/automation/tasks.py @@ -7,16 +7,163 @@ retry_if_exception_type, stop_after_attempt, ) - +from django.db.models import Q from django.conf import settings +from django.contrib.auth import get_user_model +import concrete_datastore.concrete.models from concrete_datastore.settings.celery import app from concrete_mailer.preparers import prepare_email +from concrete_datastore.concrete.meta import meta_models +from concrete_datastore.concrete.models import ( # pylint:disable=E0611 + DIVIDER_MODEL, + UNDIVIDED_MODEL, + Email, +) +from concrete_datastore.api.v1.permissions import ( + update_created_by_permissions, + create_or_update_instance_permission_per_user, +) -from concrete_datastore.concrete.models import Email # pylint:disable=E0611 logger = logging.getLogger(__name__) +DIVIDER_MODELs = "{}s".format(DIVIDER_MODEL) +DIVIDER_MODELs_LOWER = DIVIDER_MODELs.lower() +DIVIDER_MODEL_LOWER = DIVIDER_MODEL.lower() + + +@app.task +def on_update_divider_async(pk_set, instance_uid, include_divider): + divider_model = getattr(concrete_datastore.concrete.models, DIVIDER_MODEL) + for meta_model in meta_models: + model_name = meta_model.get_model_name() + if model_name in ('User', 'Group', 'Email'): + continue + if model_name in UNDIVIDED_MODEL: + continue + + related_name = meta_model.get_dashed_case_class_name().replace('-', '') + related_divider_field_name = f'divider_{related_name}s' + for divider_pk in pk_set: + divider_instance = divider_model.objects.get(pk=divider_pk) + create_or_update_instance_permission_per_user( + user=get_user_model().objects.get(pk=instance_uid), + instances_qs=getattr( + divider_instance, related_divider_field_name + ).all(), + include_divider=include_divider, + ) + + +@app.task +def on_update_group_members_async( + instance_model_name, instance_uid, pk_set, include_pks +): + user_model = get_user_model() + instance_model = getattr( + concrete_datastore.concrete.models, instance_model_name + ) + instance = instance_model.objects.get(pk=instance_uid) + for meta_model in meta_models: + model_name = meta_model.get_model_name() + if model_name in ('User', 'Group', 'Email'): + continue + related_model_name = meta_model.get_slugified_name() + if instance_model_name == 'Group': + group_viewable_field_name = f'group_viewable_{related_model_name}s' + viewable_qs = getattr(instance, group_viewable_field_name).all() + group_admin_field_name = ( + f'group_administrable_{related_model_name}s' + ) + administrable_qs = getattr(instance, group_admin_field_name).all() + for user_pk in pk_set: + user = user_model.objects.get(pk=user_pk) + create_or_update_instance_permission_per_user( + user=user, + instances_qs=viewable_qs | administrable_qs, + include_admin_groups=include_pks, + include_view_groups=include_pks, + ) + else: + #: Instance is User + user_viewable_field_name = f'viewable_{related_model_name}s' + viewable_qs = getattr(instance, user_viewable_field_name).all() + user_admin_field_name = f'administrable_{related_model_name}s' + administrable_qs = getattr(instance, user_admin_field_name).all() + concrete_model = getattr( + concrete_datastore.concrete.models, model_name + ) + instances_qs = concrete_model.objects.filter( + Q(can_admin_groups__in=pk_set) | Q(can_view_groups__in=pk_set) + ) + create_or_update_instance_permission_per_user( + user=instance, + instances_qs=instances_qs, + include_view_users=include_pks, + include_admin_users=include_pks, + ) + + +@app.task +def on_create_instance_async(user_pk, model_name, instance_pk): + model = getattr(concrete_datastore.concrete.models, model_name) + instance = model.objects.get(pk=instance_pk) + if user_pk is not None: + user = get_user_model().objects.get(pk=user_pk) + if user.is_at_least_admin is False: + update_created_by_permissions(instance=instance, user=user) + + #: Assign permissions to users with scope + if model_name in UNDIVIDED_MODEL: + return + instance_divider_id = getattr(instance, f"{DIVIDER_MODEL_LOWER}_id") + if instance_divider_id is None: + return + for user in get_user_model().objects.filter( + **{DIVIDER_MODELs_LOWER: instance_divider_id} + ): + if user.is_at_least_admin: + continue + create_or_update_instance_permission_per_user( + user=user, instances_qs=model.objects.filter(pk=instance_pk) + ) + + +@app.task +def on_view_admin_groups_changed_async( + pk_set, model_name, instance_pk, include_view_groups, include_admin_groups +): + user_model = get_user_model() + users_ids = ( + concrete_datastore.concrete.models.Group.objects.filter(pk__in=pk_set) + .values_list('members', flat=True) + .distinct() + ) + model = getattr(concrete_datastore.concrete.models, model_name) + for user in user_model.objects.filter(pk__in=users_ids): + create_or_update_instance_permission_per_user( + user=user, + instances_qs=model.objects.filter(pk=instance_pk), + include_view_groups=include_view_groups, + include_admin_groups=include_admin_groups, + ) + + +@app.task +def on_view_admin_users_changed_async( + pk_set, model_name, instance_pk, include_admin_users, include_view_users +): + user_model = get_user_model() + model = getattr(concrete_datastore.concrete.models, model_name) + for user in user_model.objects.filter(pk__in=pk_set): + create_or_update_instance_permission_per_user( + user=user, + instances_qs=model.objects.filter(pk=instance_pk), + include_admin_users=include_admin_users, + include_view_users=include_view_users, + ) + @app.task def async_run_plugin_tasks(): diff --git a/concrete_datastore/concrete/models.py b/concrete_datastore/concrete/models.py index 7ec4ab9c..4378dc0f 100644 --- a/concrete_datastore/concrete/models.py +++ b/concrete_datastore/concrete/models.py @@ -745,6 +745,53 @@ class DeletedModel(models.Model): creation_date = models.DateTimeField(auto_now_add=True) +class InstancePermission(models.Model): + uid = models.UUIDField(primary_key=True, default=uuid.uuid4) + + model_name = models.CharField(max_length=255, default='', db_index=True) + + read_instance_uids = models.JSONField(default=list) + + write_instance_uids = models.JSONField(default=list) + + user = models.ForeignKey( + 'concrete.User', + related_name="instance_permissions", + on_delete=models.PROTECT, + ) + + modification_date = models.DateTimeField(auto_now=True) + + creation_date = models.DateTimeField(auto_now_add=True) + + +class SystemVersion(models.Model): + uid = models.UUIDField(primary_key=True, default=uuid.uuid4) + + app_name = models.CharField(max_length=255, db_index=True) + + is_latest = models.BooleanField(default=True) + + modification_date = models.DateTimeField(auto_now=True) + + creation_date = models.DateTimeField(auto_now_add=True) + + def save(self, *args, **kwargs): + #: If the field is_latest is True, update the other instacne with + #: the same app_name to is_latest = False + if self.is_latest is False: + return super().save(*args, **kwargs) + exclude_filters = {} + if self.uid: + #: The instance exists, and should be exluded from the queryset + exclude_filters = {'pk': self.uid} + + SystemVersion.objects.filter(app_name=self.app_name).exclude( + **exclude_filters + ).update(is_latest=False) + return super().save(*args, **kwargs) + + class UserManager(BaseUserManager): """Define a model manager for User model with no username field.""" diff --git a/development/settings.py b/development/settings.py index 0cf700a1..8b4fb7bb 100644 --- a/development/settings.py +++ b/development/settings.py @@ -2,6 +2,8 @@ import os from concrete_datastore.settings.base import * from concrete_datastore.settings.utils import load_datamodel +from plugin_concrete_olaf.celery_scheduling_settings import * + PROJECT_ROOT = os.path.dirname(os.path.realpath(__file__)) + '/' MEDIA_ROOT = os.path.join(PROJECT_ROOT, 'media/') @@ -82,3 +84,39 @@ API_REGISTER_EMAIL_FILTER = r'.*' + +USE_TWO_FACTOR_AUTH = False + +MFA_RULE_PER_USER = "plugin_concrete_olaf.mfa.mfa_rule.mfa_olaf_rule" + +ROOT_URLCONF = "plugin_concrete_olaf.urls" + +ENABLE_AUTHENTICATED_USER_THROTTLING = False +ENABLE_ANONYMOUS_USER_THROTTLING = False + +SAM_API_TOKEN = "4423ed09d86b754ae3a569b465fc6c6d1d126d73" +SAM_API_URL = "http://preprod-api-sam-database-occam.globecast.nms/api/v1.1/" + +POD_API_TOKEN = "21881c5bc361ca1f5e91efa268f73b4d2f2b36e0" +POD_API_URL = "http://localhost:8001/api/v1.1/" + +INSTALLED_PLUGINS = {"plugin_concrete_olaf": "1.0.0"} + +try: + import celery_once + + try: + CELERY_ONCE_TIMEOUT = CELERY_ONCE_TIMEOUT + except NameError: + CELERY_ONCE_TIMEOUT = 20 * 60 + ONCE = { + 'backend': 'celery_once.backends.Redis', + 'settings': { + 'url': BROKER_URL, + 'default_timeout': CELERY_ONCE_TIMEOUT, + }, + } +except ImportError: + pass + +# END of generated settings diff --git a/tests/migrations/0013_instancepermission_systemversion.py b/tests/migrations/0013_instancepermission_systemversion.py new file mode 100644 index 00000000..a908bb32 --- /dev/null +++ b/tests/migrations/0013_instancepermission_systemversion.py @@ -0,0 +1,38 @@ +# Generated by Django 3.2.16 on 2023-02-15 14:44 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('concrete', '0012_auto_20221115_0644'), + ] + + operations = [ + migrations.CreateModel( + name='SystemVersion', + fields=[ + ('uid', models.UUIDField(default=uuid.uuid4, primary_key=True, serialize=False)), + ('app_name', models.CharField(db_index=True, max_length=255)), + ('is_latest', models.BooleanField(default=True)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('creation_date', models.DateTimeField(auto_now_add=True)), + ], + ), + migrations.CreateModel( + name='InstancePermission', + fields=[ + ('uid', models.UUIDField(default=uuid.uuid4, primary_key=True, serialize=False)), + ('model_name', models.CharField(db_index=True, default='', max_length=255)), + ('read_instance_uids', models.JSONField(default=list)), + ('write_instance_uids', models.JSONField(default=list)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('creation_date', models.DateTimeField(auto_now_add=True)), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='instance_permissions', to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/tests/test_instance_permission.py b/tests/test_instance_permission.py new file mode 100644 index 00000000..667ceca1 --- /dev/null +++ b/tests/test_instance_permission.py @@ -0,0 +1,317 @@ +# coding: utf-8 +from django.test import TestCase +from concrete_datastore.concrete.models import ( + User, + UserConfirmation, + Group, + DefaultDivider, + Category, + InstancePermission, +) + + +class CommunPermissionInstanceTestCase(TestCase): + def setUp(self): + self.simple = User.objects.create_user( + email='simple@netsach.org', password='simple' + ) + + self.confirmation = UserConfirmation.objects.create(user=self.simple) + self.confirmation.confirmed = True + self.simple.set_level('simple', commit=True) + self.confirmation.save() + url = '/api/v1/auth/login/' + resp = self.client.post( + url, {"email": "simple@netsach.org", "password": "simple"} + ) + self.simple_token = resp.data['token'] + + self.manager = User.objects.create_user( + email='manager@netsach.org', password='manager' + ) + + self.confirmation = UserConfirmation.objects.create(user=self.manager) + self.confirmation.confirmed = True + self.manager.set_level('manager', commit=True) + self.confirmation.save() + url = '/api/v1/auth/login/' + resp = self.client.post( + url, {"email": "manager@netsach.org", "password": "manager"} + ) + self.manager_token = resp.data['token'] + + self.admin = User.objects.create_user( + email='admin@netsach.org', password='admin' + ) + + self.confirmation = UserConfirmation.objects.create(user=self.admin) + self.confirmation.confirmed = True + self.admin.set_level('admin', commit=True) + self.confirmation.save() + url = '/api/v1/auth/login/' + resp = self.client.post( + url, {"email": "admin@netsach.org", "password": "admin"} + ) + self.admin_token = resp.data['token'] + + self.group_can_admin = Group.objects.create( + name="Group for administration" + ) + self.group_can_admin.members.set([self.manager.uid, self.admin.uid]) + + self.divider = DefaultDivider.objects.create(name='divider') + + self.category1 = Category.objects.create( + name='v1', defaultdivider=self.divider + ) + self.category1.can_admin_groups.set([self.group_can_admin.uid]) + self.category1.can_view_users.set([self.simple.pk]) + self.category2 = Category.objects.create( + name='v2', defaultdivider=self.divider + ) + self.category2.can_admin_groups.set([self.group_can_admin.uid]) + self.category2.can_admin_users.set([self.simple.pk]) + + self.category3 = Category.objects.create( + name='v3', defaultdivider=self.divider + ) + self.category3.can_admin_groups.set([self.group_can_admin.uid]) + + self.simple.defaultdividers.add(self.divider) + self.manager.defaultdividers.add(self.divider) + self.admin.defaultdividers.add(self.divider) + + self.simple.save() + self.manager.save() + self.admin.save() + + def test_instance_permission(self): + #: We expect 2 InstancePermissions: one for the simple and one for the + #: manager + self.assertEqual(InstancePermission.objects.count(), 2) + self.assertTrue( + InstancePermission.objects.filter(user_id=self.simple.pk).exists() + ) + self.assertTrue( + InstancePermission.objects.filter(user_id=self.manager.pk).exists() + ) + simple_permissions = InstancePermission.objects.get( + user_id=self.simple.pk + ) + manager_permissions = InstancePermission.objects.get( + user_id=self.manager.pk + ) + + simple_read_permissions = simple_permissions.read_instance_uids + self.assertIn(str(self.category1.pk), simple_read_permissions) + self.assertIn(str(self.category2.pk), simple_read_permissions) + self.assertNotIn(str(self.category3.pk), simple_read_permissions) + + simple_write_permissions = simple_permissions.write_instance_uids + self.assertNotIn(str(self.category1.pk), simple_write_permissions) + self.assertIn(str(self.category2.pk), simple_write_permissions) + self.assertNotIn(str(self.category3.pk), simple_write_permissions) + + manager_read_permissions = manager_permissions.read_instance_uids + self.assertIn(str(self.category1.pk), manager_read_permissions) + self.assertIn(str(self.category2.pk), manager_read_permissions) + self.assertIn(str(self.category3.pk), manager_read_permissions) + + manager_write_permissions = manager_permissions.write_instance_uids + self.assertIn(str(self.category1.pk), manager_write_permissions) + self.assertIn(str(self.category2.pk), manager_write_permissions) + self.assertIn(str(self.category3.pk), manager_write_permissions) + + self.group_can_admin.members.add(self.simple) + simple_permissions.refresh_from_db() + + simple_read_permissions = simple_permissions.read_instance_uids + self.assertIn(str(self.category1.pk), simple_read_permissions) + self.assertIn(str(self.category2.pk), simple_read_permissions) + self.assertIn(str(self.category3.pk), simple_read_permissions) + + simple_write_permissions = simple_permissions.write_instance_uids + self.assertIn(str(self.category1.pk), simple_write_permissions) + self.assertIn(str(self.category2.pk), simple_write_permissions) + self.assertIn(str(self.category3.pk), simple_write_permissions) + + self.group_can_admin.members.remove(self.manager) + manager_permissions.refresh_from_db() + manager_read_permissions = manager_permissions.read_instance_uids + self.assertIn(str(self.category1.pk), manager_read_permissions) + self.assertIn(str(self.category2.pk), manager_read_permissions) + self.assertIn(str(self.category3.pk), manager_read_permissions) + + manager_write_permissions = manager_permissions.write_instance_uids + self.assertIn(str(self.category1.pk), manager_write_permissions) + self.assertIn(str(self.category2.pk), manager_write_permissions) + self.assertIn(str(self.category3.pk), manager_write_permissions) + + self.manager.defaultdividers.clear() + manager_permissions.refresh_from_db() + manager_read_permissions = manager_permissions.read_instance_uids + self.assertNotIn(str(self.category1.pk), manager_read_permissions) + self.assertNotIn(str(self.category2.pk), manager_read_permissions) + self.assertNotIn(str(self.category3.pk), manager_read_permissions) + + manager_write_permissions = manager_permissions.write_instance_uids + self.assertNotIn(str(self.category1.pk), manager_write_permissions) + self.assertNotIn(str(self.category2.pk), manager_write_permissions) + self.assertNotIn(str(self.category3.pk), manager_write_permissions) + + +class CanViewAdminUsersTestCase(TestCase): + def setUp(self): + self.u_1 = User.objects.create_user( + email='user1@netsach.org', password='simple' + ) + self.u_2 = User.objects.create_user( + email='user2@netsach.org', password='simple' + ) + self.u_3 = User.objects.create_user( + email='user3@netsach.org', password='simple' + ) + self.u_4 = User.objects.create_user( + email='user4@netsach.org', password='simple' + ) + self.cat_1 = Category.objects.create(name='Cat 1') + self.cat_2 = Category.objects.create(name='Cat 2') + self.cat_3 = Category.objects.create(name='Cat 3') + self.cat_4 = Category.objects.create(name='Cat 4') + + def test_set_can_view_admin_users(self): + self.assertEqual(InstancePermission.objects.count(), 0) + self.cat_1.can_view_users.set([self.u_1]) + self.assertEqual(InstancePermission.objects.count(), 1) + + permissions_instance_1 = InstancePermission.objects.first() + self.assertEqual(permissions_instance_1.user_id, self.u_1.pk) + self.assertEqual(permissions_instance_1.model_name, 'Category') + self.assertListEqual( + permissions_instance_1.read_instance_uids, [str(self.cat_1.pk)] + ) + self.assertListEqual(permissions_instance_1.write_instance_uids, []) + + self.cat_1.can_view_users.set([self.u_2]) + self.assertEqual(InstancePermission.objects.count(), 2) + permissions_instance_1.refresh_from_db() + permissions_instance_2 = InstancePermission.objects.get( + user_id=self.u_2.pk + ) + self.assertListEqual(permissions_instance_1.read_instance_uids, []) + self.assertListEqual(permissions_instance_1.write_instance_uids, []) + + self.assertListEqual( + permissions_instance_2.read_instance_uids, [str(self.cat_1.pk)] + ) + self.assertListEqual(permissions_instance_2.write_instance_uids, []) + + self.cat_1.can_view_users.set([self.u_3, self.u_4]) + self.assertEqual(InstancePermission.objects.count(), 4) + permissions_instance_1.refresh_from_db() + permissions_instance_2.refresh_from_db() + permissions_instance_3 = InstancePermission.objects.get( + user_id=self.u_3.pk + ) + permissions_instance_4 = InstancePermission.objects.get( + user_id=self.u_4.pk + ) + self.assertListEqual(permissions_instance_1.read_instance_uids, []) + self.assertListEqual(permissions_instance_1.write_instance_uids, []) + self.assertListEqual(permissions_instance_2.read_instance_uids, []) + self.assertListEqual(permissions_instance_2.write_instance_uids, []) + self.assertListEqual( + permissions_instance_3.read_instance_uids, [str(self.cat_1.pk)] + ) + self.assertListEqual(permissions_instance_3.write_instance_uids, []) + self.assertListEqual( + permissions_instance_4.read_instance_uids, [str(self.cat_1.pk)] + ) + self.assertListEqual(permissions_instance_4.write_instance_uids, []) + + self.cat_2.can_view_users.set([self.u_3]) + self.cat_3.can_admin_users.set([self.u_4]) + + permissions_instance_1.refresh_from_db() + permissions_instance_2.refresh_from_db() + permissions_instance_3.refresh_from_db() + permissions_instance_4.refresh_from_db() + self.assertListEqual(permissions_instance_1.read_instance_uids, []) + self.assertListEqual(permissions_instance_1.write_instance_uids, []) + self.assertListEqual(permissions_instance_2.read_instance_uids, []) + self.assertListEqual(permissions_instance_2.write_instance_uids, []) + self.assertSetEqual( + set(permissions_instance_3.read_instance_uids), + {str(self.cat_1.pk), str(self.cat_2.pk)}, + ) + self.assertListEqual(permissions_instance_3.write_instance_uids, []) + self.assertSetEqual( + set(permissions_instance_4.read_instance_uids), + {str(self.cat_1.pk), str(self.cat_3.pk)}, + ) + self.assertListEqual( + permissions_instance_4.write_instance_uids, [str(self.cat_3.pk)] + ) + + def test_set_can_view_admin_groups(self): + self.assertEqual(InstancePermission.objects.count(), 0) + g_v_1 = Group.objects.create(name='group viewer 1') + g_v_2 = Group.objects.create(name='group viewer 2') + g_a_1 = Group.objects.create(name='group admin 1') + g_a_2 = Group.objects.create(name='group admin 2') + + self.cat_1.can_view_groups.add(g_v_1) + self.cat_1.can_admin_groups.add(g_a_1) + g_v_1.members.add(self.u_1) + self.assertEqual(InstancePermission.objects.count(), 1) + perm_1 = InstancePermission.objects.get(user_id=self.u_1.pk) + self.assertListEqual([str(self.cat_1.pk)], perm_1.read_instance_uids) + self.assertListEqual([], perm_1.write_instance_uids) + g_a_1.members.add(self.u_2) + self.assertEqual(InstancePermission.objects.count(), 2) + perm_1.refresh_from_db() + self.assertIn(str(self.cat_1.pk), perm_1.read_instance_uids) + self.assertListEqual([], perm_1.write_instance_uids) + perm_2 = InstancePermission.objects.get(user_id=self.u_2.pk) + self.assertListEqual([str(self.cat_1.pk)], perm_2.read_instance_uids) + self.assertListEqual([str(self.cat_1.pk)], perm_2.write_instance_uids) + g_v_1.members.clear() + perm_1.refresh_from_db() + self.assertListEqual([], perm_1.read_instance_uids) + self.assertListEqual([], perm_1.write_instance_uids) + self.cat_1.can_admin_groups.clear() + perm_2.refresh_from_db() + self.assertListEqual([], perm_2.read_instance_uids) + self.assertListEqual([], perm_2.write_instance_uids) + self.cat_1.can_view_groups.add(g_v_1) + self.cat_1.can_view_groups.add(g_v_2) + g_v_1.members.set({self.u_1, self.u_2}) + self.u_3.concrete_groups.add(g_v_2) + + perm_1.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_1.read_instance_uids) + self.assertListEqual([], perm_1.write_instance_uids) + perm_2.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_2.read_instance_uids) + self.assertListEqual([], perm_2.write_instance_uids) + perm_3 = InstancePermission.objects.get(user_id=self.u_3.pk) + self.assertListEqual([str(self.cat_1.pk)], perm_3.read_instance_uids) + self.assertListEqual([], perm_3.write_instance_uids) + g_a_2.members.set({self.u_1, self.u_2}) + self.cat_1.can_admin_groups.add(g_a_2) + + perm_1.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_1.read_instance_uids) + self.assertListEqual([str(self.cat_1.pk)], perm_1.write_instance_uids) + perm_2.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_2.read_instance_uids) + self.assertListEqual([str(self.cat_1.pk)], perm_2.write_instance_uids) + + self.cat_1.can_admin_users.add(self.u_1) + self.cat_1.can_admin_groups.clear() + perm_1.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_1.read_instance_uids) + self.assertListEqual([str(self.cat_1.pk)], perm_1.write_instance_uids) + perm_2.refresh_from_db() + self.assertListEqual([str(self.cat_1.pk)], perm_2.read_instance_uids) + self.assertListEqual([], perm_2.write_instance_uids) diff --git a/tests/unittest_settings.py b/tests/unittest_settings.py index 7e198151..bce3947d 100644 --- a/tests/unittest_settings.py +++ b/tests/unittest_settings.py @@ -2,9 +2,7 @@ import os import warnings -from django.utils.deprecation import ( - RemovedInNextVersionWarning, -) +from django.utils.deprecation import RemovedInNextVersionWarning from concrete_datastore.settings.base import * from concrete_datastore.settings.utils import load_datamodel @@ -91,3 +89,5 @@ API_REGISTER_EMAIL_FILTER = '.*' ENABLE_USERS_SELF_REGISTER = True + +CELERY_ALWAYS_EAGER = True From eb72344c0fa18b5d3b338fd29d5f84081566fcdd Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Fri, 17 Feb 2023 18:05:32 +0100 Subject: [PATCH 2/7] Fix black --- concrete_datastore/admin/admin_form.py | 1 - concrete_datastore/api/v1/filters.py | 1 - concrete_datastore/api/v1/permissions.py | 15 ++++++++++++--- concrete_datastore/api/v1/serializers.py | 7 ------- concrete_datastore/api/v1/views.py | 6 ------ concrete_datastore/api/v1_1/serializers.py | 2 -- concrete_datastore/api/v1_1/views.py | 4 ---- .../concrete/management/commands/openapispec.py | 1 - concrete_datastore/concrete/models.py | 4 ---- .../interfaces/openapi_schema_generator.py | 4 ---- concrete_datastore/routes/forms.py | 1 - concrete_datastore/settings/utils.py | 1 - 12 files changed, 12 insertions(+), 35 deletions(-) diff --git a/concrete_datastore/admin/admin_form.py b/concrete_datastore/admin/admin_form.py index 06c31a9d..fc91f965 100644 --- a/concrete_datastore/admin/admin_form.py +++ b/concrete_datastore/admin/admin_form.py @@ -15,7 +15,6 @@ class MyAuthForm(forms.AuthenticationForm): class OTPAuthenticationForm(MyAuthForm, OTPAuthenticationFormMixin): - otp_error_messages = { 'token_required': _('Please enter your OTP token.'), 'challenge_exception': _('Error generating challenge: {0}'), diff --git a/concrete_datastore/api/v1/filters.py b/concrete_datastore/api/v1/filters.py index b63bd3aa..e91fc58b 100644 --- a/concrete_datastore/api/v1/filters.py +++ b/concrete_datastore/api/v1/filters.py @@ -440,7 +440,6 @@ class FilterSupportingOrBackend( BaseFilterBackend, CustomShemaOperationParameters ): def get_schema_operation_parameters(self, view): - params = [ { 'name': f'{field_name}__in{neg}', diff --git a/concrete_datastore/api/v1/permissions.py b/concrete_datastore/api/v1/permissions.py index 1a0516fd..3fc13c21 100644 --- a/concrete_datastore/api/v1/permissions.py +++ b/concrete_datastore/api/v1/permissions.py @@ -560,7 +560,10 @@ def update_instance_permission_uids( for uid in all_uids: uid_str = str(uid) #: Handle read instance uids - read_permission_changed, read_instance_uids = add_or_remove_element_into_list( + ( + read_permission_changed, + read_instance_uids, + ) = add_or_remove_element_into_list( elt=uid_str, current_list=read_instance_uids, check_list=new_read_instance_uids, @@ -568,7 +571,10 @@ def update_instance_permission_uids( ) #: Handle write instance uids - write_permission_changed, write_instance_uids = add_or_remove_element_into_list( + ( + write_permission_changed, + write_instance_uids, + ) = add_or_remove_element_into_list( elt=uid_str, current_list=write_instance_uids, check_list=new_write_instance_uids, @@ -603,7 +609,10 @@ def create_or_update_instance_permission_per_user( return model_name = instances_qs.model.__name__ user_groups_pks = user.concrete_groups.values_list('pk', flat=True) - read_instances_uids, write_instances_uids = get_read_write_permission_users( + ( + read_instances_uids, + write_instances_uids, + ) = get_read_write_permission_users( user=user, instances_qs=instances_qs, user_groups_pks=user_groups_pks, diff --git a/concrete_datastore/api/v1/serializers.py b/concrete_datastore/api/v1/serializers.py index 525869a4..960bc86c 100644 --- a/concrete_datastore/api/v1/serializers.py +++ b/concrete_datastore/api/v1/serializers.py @@ -236,7 +236,6 @@ def get_token(self, obj): def make_related_serializer_class( target_model_name, many, nested=False, api_namespace=DEFAULT_API_NAMESPACE ): - if target_model_name not in meta_registered: raise ValueError(f'Related to unknown model {target_model_name}') @@ -258,11 +257,9 @@ def make_custom_serializer_fields( for model_name, SERIALIZER_SETTINGS in SERIALIZERS_SETTINGS.items(): if meta_model.get_model_name() == model_name: - CUSTOM_FIELDS = SERIALIZER_SETTINGS.get('CUSTOM_FIELDS', {}) for field_name, field_args in CUSTOM_FIELDS.items(): - if 'type' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -270,7 +267,6 @@ def make_custom_serializer_fields( ) if field_args['type'] == 'RelatedModelSerializer': - if 'to' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -362,7 +358,6 @@ def make_serializer_class( # TODO: rajouter les _uid dans _fields fk_read_only_fields = [] for name, field in enum_fields: - if field.type.startswith("rel_"): _fields += ['{}_uid'.format(name)] fk_read_only_fields += [name] @@ -393,7 +388,6 @@ class Meta: # TODO : if field is relational, expose pk and url serialized for name, field in enum_fields: - if field.f_type == 'FileField': attrs.update( { @@ -418,7 +412,6 @@ class Meta: {name: serializers.JSONField(binary=False, required=False)} ) if field.type.startswith("rel_") and nested is True: - force_nested = getattr(field, 'force_nested', False) attrs.update( diff --git a/concrete_datastore/api/v1/views.py b/concrete_datastore/api/v1/views.py index 41175ead..1fa6714e 100644 --- a/concrete_datastore/api/v1/views.py +++ b/concrete_datastore/api/v1/views.py @@ -534,7 +534,6 @@ def post(self, request, *args, **kwargs): ) if not user.is_confirmed(): - log_request = ( base_message + f"Connection attempt to a not validated user {email}" @@ -820,7 +819,6 @@ def change_another_user_password(self, target_user, password): ): same_password = target_user.check_password(password) if same_password: - log_request = base_message + ( f"Change password attempt by {user.email}" f" to user {target_user.email}, " @@ -1505,7 +1503,6 @@ def post(self, request, *args, **kwargs): return Response(data={'email': user.email}, status=HTTP_200_OK) def send_email(self, link, user, created_by): - Email.objects.create( subject="Reset password", resource_status='to-send', @@ -1521,7 +1518,6 @@ def send_email(self, link, user, created_by): class AccountMeApiView( generics.RetrieveAPIView, generics.UpdateAPIView, generics.GenericAPIView ): - model_class = UserModel authentication_classes = ( authentication.SessionAuthentication, @@ -2107,7 +2103,6 @@ def get_queryset(self): ) if self.model_class is UserModel: - #: Anonymous user can only see public objects divider_name_plural = '{}s'.format(DIVIDER_MODEL.lower()) user_filters = {'is_active': True} @@ -2368,7 +2363,6 @@ def make_api_viewset_generic_attributes_class( model_filterset_fields += ('{}'.format(DIVIDER_MODEL.lower()),) class GenericAttributesViewsetClass: - permission_classes = model_permission_classes model_class = main_app.models[meta_model.get_model_name().lower()] diff --git a/concrete_datastore/api/v1_1/serializers.py b/concrete_datastore/api/v1_1/serializers.py index 9766223d..d5d387ad 100644 --- a/concrete_datastore/api/v1_1/serializers.py +++ b/concrete_datastore/api/v1_1/serializers.py @@ -108,7 +108,6 @@ def get_url(self, obj): class EmailDeviceSerializer(serializers.ModelSerializer): - url = serializers.SerializerMethodField() class Meta: @@ -143,7 +142,6 @@ def get_url(self, obj): class ConcretePermissionSerializer(serializers.ModelSerializer): - create_roles = serializers.StringRelatedField(many=True, read_only=True) update_roles = serializers.StringRelatedField(many=True, read_only=True) retrieve_roles = serializers.StringRelatedField(many=True, read_only=True) diff --git a/concrete_datastore/api/v1_1/views.py b/concrete_datastore/api/v1_1/views.py index ced1e75e..6885fbf5 100644 --- a/concrete_datastore/api/v1_1/views.py +++ b/concrete_datastore/api/v1_1/views.py @@ -146,7 +146,6 @@ class ProcessRegisterApiView(generics.GenericAPIView): serializer_class = ProcessRegisterSerializer def post(self, request, *args, **kwargs): - """ 1. Check if the token is not already created and that it is valid 2. Check if the user(process) is not already created with a different token @@ -450,13 +449,11 @@ def get_extra_informations(self, queryset): } def perform_create(self, serializer): - attrs = {'created_by': self.request.user, 'user': self.request.user} serializer.save(**attrs) class ConcreteRoleApiView(PaginatedViewSet, viewsets.ModelViewSet): - model_class = ConcreteRole permission_classes = (IsAuthenticated, ConcreteRolesPermission) api_namespace = DEFAULT_API_NAMESPACE @@ -502,7 +499,6 @@ class ConcretePermissionApiView( mixins.ListModelMixin, viewsets.GenericViewSet, ): - model_class = ConcretePermission permission_classes = (IsAuthenticated, ConcreteRolesPermission) serializer_class = ConcretePermissionSerializer diff --git a/concrete_datastore/concrete/management/commands/openapispec.py b/concrete_datastore/concrete/management/commands/openapispec.py index 4f84707e..71b7b210 100644 --- a/concrete_datastore/concrete/management/commands/openapispec.py +++ b/concrete_datastore/concrete/management/commands/openapispec.py @@ -71,7 +71,6 @@ def add_arguments(self, parser): ) def handle(self, *args, **options): - url_patterns_dict = { 'v1': (api_v1_urls,), 'v1.1': (api_v1_1_urls,), diff --git a/concrete_datastore/concrete/models.py b/concrete_datastore/concrete/models.py index 4378dc0f..5abd48fa 100644 --- a/concrete_datastore/concrete/models.py +++ b/concrete_datastore/concrete/models.py @@ -431,7 +431,6 @@ def send_mail(self): class SecureConnectToken(SecureConnectModelMixin): - value = models.UUIDField(default=uuid.uuid4, primary_key=True) user = models.ForeignKey( 'concrete.User', @@ -719,7 +718,6 @@ def totp_device(self): class PasswordChangeToken(models.Model): - uid = models.UUIDField(primary_key=True, default=uuid.uuid4) creation_date = models.DateTimeField(auto_now_add=True) expiry_date = models.DateTimeField(default=compute_pwd_change_token_expiry) @@ -1043,7 +1041,6 @@ class Meta: args.setdefault('max_digits', 20) args.setdefault('default', 0.00) elif field.f_type in ('ForeignKey',): - # Copy args to not alter the real field.f_args args = args.copy() # Force FK to null=True to avoid default value problems @@ -1157,7 +1154,6 @@ def get_divider_fn(meta_model): def get_divider(): - dividers_set = set(map(get_divider_fn, meta_models)) #: Remove None values from the set diff --git a/concrete_datastore/interfaces/openapi_schema_generator.py b/concrete_datastore/interfaces/openapi_schema_generator.py index d42822af..4a475412 100644 --- a/concrete_datastore/interfaces/openapi_schema_generator.py +++ b/concrete_datastore/interfaces/openapi_schema_generator.py @@ -354,7 +354,6 @@ def get_schema(self, request=None, public=False): class AutoSchema(AutoSchemaSuper): - components = {} def get_component_schemas(self): @@ -466,7 +465,6 @@ def get_custom_path_parameters(self, path, method): if model is not None: # Attempt to infer a field description if possible. try: - model_field = model._meta.get_field(variable) except Exception: model_field = None @@ -488,7 +486,6 @@ def get_custom_path_parameters(self, path, method): return parameters def custom_map_field(self, field): - # Related fields. if isinstance(field, serializers.ManyRelatedField): return { @@ -540,7 +537,6 @@ def custom_map_serializer(self, serializer, nested=True): properties = {} for field in serializer.fields.values(): - if ( isinstance(field, serializers.PrimaryKeyRelatedField) and nested is False diff --git a/concrete_datastore/routes/forms.py b/concrete_datastore/routes/forms.py index 9c8b54a9..e6e7b257 100644 --- a/concrete_datastore/routes/forms.py +++ b/concrete_datastore/routes/forms.py @@ -7,7 +7,6 @@ class ConfigureOTPLoginForm(forms.Form): - otp_error_messages = { 'token_required': _('Please enter your OTP token.'), 'challenge_exception': _('Error generating challenge: {0}'), diff --git a/concrete_datastore/settings/utils.py b/concrete_datastore/settings/utils.py index c42d1e76..c8eff7fe 100644 --- a/concrete_datastore/settings/utils.py +++ b/concrete_datastore/settings/utils.py @@ -9,7 +9,6 @@ def load_datamodel(datamodel_path='current-datamodel.meta'): - datamodel_path = os.getenv('DATAMODEL_FILE') or datamodel_path try: From 1998c8a97fe2a801e3dd53ec040e060f0f1ce481 Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Thu, 23 Feb 2023 11:39:48 +0100 Subject: [PATCH 3/7] Add mgmnt command to check user permissions in int_app --- concrete_datastore/admin/admin.py | 37 +++++++++ concrete_datastore/api/v1/permissions.py | 22 ++++++ .../concrete/automation/signal_processor.py | 28 ++++++- .../concrete/management/commands/init_app.py | 77 +++++++++++++++++++ concrete_datastore/concrete/models.py | 17 +--- development/settings.py | 3 +- tests/datamodel/unittest-datamodel.yaml | 2 +- .../migrations/0014_systemversion_version.py | 18 +++++ tests/test_init_app.py | 73 ++++++++++++++++++ tests/test_instance_permission.py | 10 --- 10 files changed, 257 insertions(+), 30 deletions(-) create mode 100644 tests/migrations/0014_systemversion_version.py create mode 100644 tests/test_init_app.py diff --git a/concrete_datastore/admin/admin.py b/concrete_datastore/admin/admin.py index 4128bf97..ecf2a38f 100644 --- a/concrete_datastore/admin/admin.py +++ b/concrete_datastore/admin/admin.py @@ -13,6 +13,7 @@ ConcretePermission, EmailDevice, InstancePermission, + SystemVersion, ) from concrete_datastore.concrete.models import ( divider, @@ -263,3 +264,39 @@ class InstancePermissionAdmin(SaveModelMixin, admin.ModelAdmin): 'modification_date', ] list_filter = ['model_name'] + + +@admin.register(SystemVersion, site=admin_site) +class SystemVersionAdmin(SaveModelMixin, admin.ModelAdmin): + @admin.action(description='Set selected versions as the latest versions') + def set_is_latest_to_true(self, request, queryset): + queryset.update(is_latest=True) + + @admin.action(description='Set selected versions as earlier versions') + def set_is_latest_to_false(self, request, queryset): + queryset.update(is_latest=False) + + list_display = [ + 'uid', + 'app_name', + 'version', + 'is_latest', + 'creation_date', + 'modification_date', + ] + search_fields = ['app_name'] + readonly_fields = ['uid', 'creation_date', 'modification_date'] + date_hierarchy = 'creation_date' + + fields = [ + 'uid', + 'user', + 'model_name', + 'read_instance_uids', + 'write_instance_uids', + 'creation_date', + 'modification_date', + ] + list_filter = ['app_name', 'is_latest'] + + actions = ['set_is_latest_to_true', 'set_is_latest_to_false'] diff --git a/concrete_datastore/api/v1/permissions.py b/concrete_datastore/api/v1/permissions.py index 3fc13c21..d2996bd8 100644 --- a/concrete_datastore/api/v1/permissions.py +++ b/concrete_datastore/api/v1/permissions.py @@ -9,6 +9,9 @@ from rest_framework.exceptions import APIException from rest_framework import permissions, status + +import concrete_datastore.concrete.models +from concrete_datastore.concrete.meta import meta_models from concrete_datastore.concrete.models import ( ConcretePermission, InstancePermission, @@ -655,3 +658,22 @@ def update_created_by_permissions(instance, user): for field_name, field_value in defaults.items(): getattr(permission_instance, field_name).extend(field_value) permission_instance.save() + + +def check_instance_permissions_per_user(user): + #: Checks the user permissions for all instances of the platform + for meta_model in meta_models: + model_name = meta_model.get_model_name() + if model_name in ('User', 'Group', 'Email'): + continue + concrete_model = getattr( + concrete_datastore.concrete.models, model_name + ) + queryset = concrete_model.objects.all() + logger.debug( + f'Checking permission for user {user} on {queryset.count()} ' + f'instances of model {model_name}' + ) + create_or_update_instance_permission_per_user( + user=user, instances_qs=queryset + ) diff --git a/concrete_datastore/concrete/automation/signal_processor.py b/concrete_datastore/concrete/automation/signal_processor.py index 45e12687..3ffc1d17 100644 --- a/concrete_datastore/concrete/automation/signal_processor.py +++ b/concrete_datastore/concrete/automation/signal_processor.py @@ -4,7 +4,12 @@ import logging from django.conf import settings from django.dispatch import receiver -from django.db.models.signals import pre_delete, post_save, m2m_changed +from django.db.models.signals import ( + pre_delete, + post_save, + m2m_changed, + pre_save, +) from django.contrib.auth import get_user_model import concrete_datastore.concrete.models from concrete_datastore.concrete.models import DIVIDER_MODEL @@ -15,6 +20,9 @@ on_view_admin_groups_changed_async, on_view_admin_users_changed_async, ) +from concrete_datastore.api.v1.permissions import ( + check_instance_permissions_per_user, +) from concrete_datastore.api.v1.views import ( remove_instances_user_tracked_fields, ) @@ -75,8 +83,24 @@ def on_pre_delete(sender, instance, **kwargs): continue +@receiver(pre_save, sender=get_user_model()) +def on_pre_save(sender, instance, *args, **kwargs): + if instance.level in ('blocked', 'superuser', 'admin'): + return + try: + prev_instance = get_user_model().objects.get(pk=instance.pk) + except get_user_model().DoesNotExist: + return + if prev_instance.level == instance.level: + return + check_instance_permissions_per_user(instance) + + @receiver(post_save, sender=get_user_model()) -def on_post_save(sender, instance, **kwargs): +def on_post_save(sender, instance, created, **kwargs): + if created is True and instance.level in ('simpleuser', 'manager'): + check_instance_permissions_per_user(instance) + if instance.level == 'blocked': divider_manager = getattr( instance, '{}s'.format(DIVIDER_MODEL.lower()) diff --git a/concrete_datastore/concrete/management/commands/init_app.py b/concrete_datastore/concrete/management/commands/init_app.py index 4307c9b2..2382df4e 100644 --- a/concrete_datastore/concrete/management/commands/init_app.py +++ b/concrete_datastore/concrete/management/commands/init_app.py @@ -1,18 +1,93 @@ # coding: utf-8 +import logging from django.apps import apps from django.core.management.base import BaseCommand from django.contrib.auth import get_user_model +from django.conf import settings +import concrete_datastore from concrete_datastore.api.v1.views import ( remove_instances_user_tracked_fields, ) +from concrete_datastore.api.v1.permissions import ( + check_instance_permissions_per_user, +) from concrete_datastore.concrete.models import ( DIVIDER_MODEL, ConcretePermission, + SystemVersion, ) from concrete_datastore.concrete.meta import meta_models +logger = logging.getLogger(__name__) + + +def check_for_latest_version(app_name, version): + #: The version has changed on one of these conditions + #: - created is True + #: - created is False and is_latest is False + instance, created = SystemVersion.objects.get_or_create( + app_name=app_name, version=version + ) + version_has_changed = created + if created is False and instance.is_latest is False: + version_has_changed = True + instance.is_latest = True + instance.save() + SystemVersion.objects.filter(app_name=app_name).exclude( + pk=instance.pk + ).update(is_latest=False) + return version_has_changed + + +def get_datamodel_version_v1(model_definitions): + return model_definitions['manifest']['data_modeling']['version'] + + +data_models_version = {'1.0.0': get_datamodel_version_v1} + + +def get_datamodel_version(): + model_definitions = settings.META_MODEL_DEFINITIONS + try: + version = model_definitions['manifest']['version'] + except KeyError: + logger.warn('Meta definition not supported') + return None + try: + version_func = data_models_version[version] + return version_func(model_definitions) + except KeyError: + logger.warn(f'Version {version} not supported') + return None + + +def setup_versions_and_permissions(): + #: Datastore version + concrete_version = concrete_datastore.__version__ + check_for_latest_version( + app_name='concrete_datastore', version=concrete_version + ) + + #: Plugins versions + for plugin_name, plugin_version in settings.INSTALLED_PLUGINS.items(): + check_for_latest_version(app_name=plugin_name, version=plugin_version) + + #: Datamodel version + datamodel_version = get_datamodel_version() + if datamodel_version is None: + return + datamodel_changed = check_for_latest_version( + app_name='datamodel', version=datamodel_version + ) + if datamodel_changed is True: + for user in get_user_model().objects.filter( + is_active=True, admin=False, is_superuser=False + ): + check_instance_permissions_per_user(user=user) + + class Command(BaseCommand): help = 'Init Applications (create groups, permissions, etc.)' @@ -49,3 +124,5 @@ def handle(self, *args, **options): model_name = meta_model.get_model_name() # pylint: disable=no-member ConcretePermission.objects.get_or_create(model_name=model_name) + + setup_versions_and_permissions() diff --git a/concrete_datastore/concrete/models.py b/concrete_datastore/concrete/models.py index 5abd48fa..8741faae 100644 --- a/concrete_datastore/concrete/models.py +++ b/concrete_datastore/concrete/models.py @@ -766,6 +766,8 @@ class InstancePermission(models.Model): class SystemVersion(models.Model): uid = models.UUIDField(primary_key=True, default=uuid.uuid4) + version = models.CharField(max_length=255, default='0.1.0') + app_name = models.CharField(max_length=255, db_index=True) is_latest = models.BooleanField(default=True) @@ -774,21 +776,6 @@ class SystemVersion(models.Model): creation_date = models.DateTimeField(auto_now_add=True) - def save(self, *args, **kwargs): - #: If the field is_latest is True, update the other instacne with - #: the same app_name to is_latest = False - if self.is_latest is False: - return super().save(*args, **kwargs) - exclude_filters = {} - if self.uid: - #: The instance exists, and should be exluded from the queryset - exclude_filters = {'pk': self.uid} - - SystemVersion.objects.filter(app_name=self.app_name).exclude( - **exclude_filters - ).update(is_latest=False) - return super().save(*args, **kwargs) - class UserManager(BaseUserManager): """Define a model manager for User model with no username field.""" diff --git a/development/settings.py b/development/settings.py index a803fb9f..f2abde51 100644 --- a/development/settings.py +++ b/development/settings.py @@ -2,7 +2,6 @@ import os from concrete_datastore.settings.base import * from concrete_datastore.settings.utils import load_datamodel -from plugin_concrete_olaf.celery_scheduling_settings import * PROJECT_ROOT = os.path.dirname(os.path.realpath(__file__)) + '/' @@ -85,4 +84,4 @@ API_REGISTER_EMAIL_FILTER = r'.*' -USE_TWO_FACTOR_AUTH = True +USE_TWO_FACTOR_AUTH = False diff --git a/tests/datamodel/unittest-datamodel.yaml b/tests/datamodel/unittest-datamodel.yaml index 10a73c25..3cd01929 100644 --- a/tests/datamodel/unittest-datamodel.yaml +++ b/tests/datamodel/unittest-datamodel.yaml @@ -1,7 +1,7 @@ manifest: data_modeling: application_id: 4a674cd4f14366b8248d46d739 - version: "1.0" + version: 1.0.0 models: - name: RoleModel description: "" diff --git a/tests/migrations/0014_systemversion_version.py b/tests/migrations/0014_systemversion_version.py new file mode 100644 index 00000000..5157824b --- /dev/null +++ b/tests/migrations/0014_systemversion_version.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.16 on 2023-02-21 18:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('concrete', '0013_instancepermission_systemversion'), + ] + + operations = [ + migrations.AddField( + model_name='systemversion', + name='version', + field=models.CharField(default='0.1.0', max_length=255), + ), + ] diff --git a/tests/test_init_app.py b/tests/test_init_app.py new file mode 100644 index 00000000..4aa0a378 --- /dev/null +++ b/tests/test_init_app.py @@ -0,0 +1,73 @@ +# coding: utf-8 +from django.core.management import call_command +from django.test import TestCase +import concrete_datastore +from concrete_datastore.concrete.models import ( + Project, + User, + InstancePermission, + SystemVersion, +) + + +class CommunPermissionInstanceTestCase(TestCase): + def test_empty_db(self): + self.assertEqual(SystemVersion.objects.count(), 0) + self.assertEqual(InstancePermission.objects.count(), 0) + call_command('init_app') + self.assertEqual(SystemVersion.objects.count(), 2) + self.assertEqual(InstancePermission.objects.count(), 0) + datastore_version = SystemVersion.objects.get( + app_name='concrete_datastore' + ) + self.assertEqual( + datastore_version.version, concrete_datastore.__version__ + ) + self.assertTrue(datastore_version.is_latest) + datamodel_version = SystemVersion.objects.get(app_name='datamodel') + self.assertEqual(datamodel_version.version, '1.0.0') + self.assertTrue(datamodel_version.is_latest) + + def test_with_users(self): + self.assertEqual(InstancePermission.objects.count(), 0) + simple = User.objects.create(email="simple@netsach.org") + manager = User.objects.create(email="manager@netsach.org") + manager.set_level('manager') + manager.save() + admin = User.objects.create(email="admin@netsach.org") + admin.set_level('admin') + admin.save() + superuser = User.objects.create(email="superuser@netsach.org") + superuser.set_level('superuser') + superuser.save() + #: No InstancePermission because the dabase is empty + self.assertEqual(InstancePermission.objects.count(), 0) + + #: Create a project + Project.objects.create() + self.assertEqual(InstancePermission.objects.count(), 0) + call_command('init_app') + #: We expect 2 InstancePermissions: simple + manager + self.assertEqual(InstancePermission.objects.count(), 2) + self.assertEqual( + InstancePermission.objects.filter(user_id=simple.pk).count(), 1 + ) + self.assertEqual( + InstancePermission.objects.filter(user_id=manager.pk).count(), 1 + ) + + #: If we update the admin level to manager, we expect a new + #: InstancePermission + admin.set_level('manager') + admin.save() + self.assertEqual(InstancePermission.objects.count(), 3) + self.assertEqual( + InstancePermission.objects.filter(user_id=admin.pk).count(), 1 + ) + + #: If we create a new simpleUser, we expect a new InstancePermission + simple2 = User.objects.create(email="simple2@netsach.org") + self.assertEqual(InstancePermission.objects.count(), 4) + self.assertEqual( + InstancePermission.objects.filter(user_id=simple2.pk).count(), 1 + ) diff --git a/tests/test_instance_permission.py b/tests/test_instance_permission.py index 667ceca1..775249e5 100644 --- a/tests/test_instance_permission.py +++ b/tests/test_instance_permission.py @@ -2,7 +2,6 @@ from django.test import TestCase from concrete_datastore.concrete.models import ( User, - UserConfirmation, Group, DefaultDivider, Category, @@ -16,10 +15,7 @@ def setUp(self): email='simple@netsach.org', password='simple' ) - self.confirmation = UserConfirmation.objects.create(user=self.simple) - self.confirmation.confirmed = True self.simple.set_level('simple', commit=True) - self.confirmation.save() url = '/api/v1/auth/login/' resp = self.client.post( url, {"email": "simple@netsach.org", "password": "simple"} @@ -30,10 +26,7 @@ def setUp(self): email='manager@netsach.org', password='manager' ) - self.confirmation = UserConfirmation.objects.create(user=self.manager) - self.confirmation.confirmed = True self.manager.set_level('manager', commit=True) - self.confirmation.save() url = '/api/v1/auth/login/' resp = self.client.post( url, {"email": "manager@netsach.org", "password": "manager"} @@ -44,10 +37,7 @@ def setUp(self): email='admin@netsach.org', password='admin' ) - self.confirmation = UserConfirmation.objects.create(user=self.admin) - self.confirmation.confirmed = True self.admin.set_level('admin', commit=True) - self.confirmation.save() url = '/api/v1/auth/login/' resp = self.client.post( url, {"email": "admin@netsach.org", "password": "admin"} From 26eb4ee13800fa38f15cf61a0d855eeb7158d4e6 Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Mon, 27 Feb 2023 12:17:55 +0100 Subject: [PATCH 4/7] Bulk create/update InstancePermissions --- CHANGELOG.md | 1 + concrete_datastore/api/v1/permissions.py | 71 +++++++++++++--- .../concrete/automation/signal_processor.py | 12 ++- .../concrete/automation/tasks.py | 81 +++++++++++++++++-- .../concrete/management/commands/init_app.py | 13 ++- concrete_datastore/settings/base.py | 2 + tests/test_init_app.py | 16 ++-- 7 files changed, 162 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fb55551..b07f3273 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - Add model InstancePermission to determine read and write permissions for a user +- InstancePermission are bulk created or bulk updated ### Changed diff --git a/concrete_datastore/api/v1/permissions.py b/concrete_datastore/api/v1/permissions.py index d2996bd8..c72ecfd0 100644 --- a/concrete_datastore/api/v1/permissions.py +++ b/concrete_datastore/api/v1/permissions.py @@ -428,7 +428,10 @@ def get_read_write_permission_users( include_admin_groups=True, include_view_users=True, include_admin_users=True, + user_level=None, ): + if user_level is None: + user_level = user.level user_id = user.pk created_by_qs = instances_qs.filter(created_by_id=user_id).values_list( 'pk', flat=True @@ -457,7 +460,7 @@ def get_read_write_permission_users( #: A manager has write permissions on his scope model_name = instances_qs.model.__name__ if ( - (user.is_at_least_staff is True) + (user_level == 'manager') and (model_name not in UNDIVIDED_MODEL) and (include_divider is True) ): @@ -594,7 +597,8 @@ def update_instance_permission_uids( f'Updating permissions for {perm_instance.user.email} on ' f'instance <{all_uids.model.__name__}>' ) - perm_instance.save() + return perm_instance + # perm_instance.save() def create_or_update_instance_permission_per_user( @@ -605,11 +609,14 @@ def create_or_update_instance_permission_per_user( include_admin_groups=True, include_view_users=True, include_admin_users=True, + user_level=None, ): if instances_qs.exists() is False: - return - if user.is_at_least_admin: - return + return None, False + if user_level is None: + user_level = user.level + if user_level in ('superuser', 'admin'): + return None, False model_name = instances_qs.model.__name__ user_groups_pks = user.concrete_groups.values_list('pk', flat=True) ( @@ -624,17 +631,24 @@ def create_or_update_instance_permission_per_user( include_admin_groups=include_admin_groups, include_view_users=include_view_users, include_admin_users=include_admin_users, + user_level=user_level, ) - perm_instance, _ = InstancePermission.objects.get_or_create( - user=user, model_name=model_name - ) + should_create = False + try: + perm_instance = InstancePermission.objects.get( + user=user, model_name=model_name + ) + except InstancePermission.DoesNotExist: + perm_instance = InstancePermission(user=user, model_name=model_name) + should_create = True - update_instance_permission_uids( + perm_instance = update_instance_permission_uids( perm_instance=perm_instance, new_read_instance_uids=read_instances_uids, new_write_instance_uids=write_instances_uids, all_uids=instances_qs.values_list('pk', flat=True), ) + return perm_instance, should_create def update_created_by_permissions(instance, user): @@ -660,8 +674,14 @@ def update_created_by_permissions(instance, user): permission_instance.save() -def check_instance_permissions_per_user(user): +def check_instance_permissions_per_user(user, user_level=None): + if user_level is None: + user_level = user.level + if user_level in ('superuser', 'admin'): + return [], [] #: Checks the user permissions for all instances of the platform + instances_to_create = [] + instances_to_update = [] for meta_model in meta_models: model_name = meta_model.get_model_name() if model_name in ('User', 'Group', 'Email'): @@ -674,6 +694,33 @@ def check_instance_permissions_per_user(user): f'Checking permission for user {user} on {queryset.count()} ' f'instances of model {model_name}' ) - create_or_update_instance_permission_per_user( - user=user, instances_qs=queryset + instance, should_create = create_or_update_instance_permission_per_user( + user=user, instances_qs=queryset, user_level=user_level ) + if instance is None: + continue + if should_create is True: + instances_to_create.append(instance) + else: + instances_to_update.append(instance) + return instances_to_create, instances_to_update + + +def bulk_create_permission_instances(instances): + if not instances: + return + logger.info(f'Creating {len(instances)} Permission objects') + InstancePermission.objects.bulk_create( + objs=instances, batch_size=settings.BATCH_SIZE_FOR_BULK_OPERATIONS + ) + + +def bulk_update_permission_instances(instances): + if not instances: + return + logger.debug(f'Updating {len(instances)} Permission objects') + InstancePermission.objects.bulk_update( + objs=instances, + fields=['read_instance_uids', 'write_instance_uids'], + batch_size=settings.BATCH_SIZE_FOR_BULK_OPERATIONS, + ) diff --git a/concrete_datastore/concrete/automation/signal_processor.py b/concrete_datastore/concrete/automation/signal_processor.py index 3ffc1d17..01173aae 100644 --- a/concrete_datastore/concrete/automation/signal_processor.py +++ b/concrete_datastore/concrete/automation/signal_processor.py @@ -19,9 +19,7 @@ on_create_instance_async, on_view_admin_groups_changed_async, on_view_admin_users_changed_async, -) -from concrete_datastore.api.v1.permissions import ( - check_instance_permissions_per_user, + check_all_user_permissions_async, ) from concrete_datastore.api.v1.views import ( remove_instances_user_tracked_fields, @@ -93,14 +91,13 @@ def on_pre_save(sender, instance, *args, **kwargs): return if prev_instance.level == instance.level: return - check_instance_permissions_per_user(instance) + check_all_user_permissions_async.apply_async( + kwargs={'user_pk': instance.pk, 'new_level': instance.level} + ) @receiver(post_save, sender=get_user_model()) def on_post_save(sender, instance, created, **kwargs): - if created is True and instance.level in ('simpleuser', 'manager'): - check_instance_permissions_per_user(instance) - if instance.level == 'blocked': divider_manager = getattr( instance, '{}s'.format(DIVIDER_MODEL.lower()) @@ -252,6 +249,7 @@ def on_view_admin_users_changed( return if action not in ('post_add', 'post_remove', 'pre_clear'): return + model_name = instance._meta.model.__name__ on_view_admin_users_changed_async.apply_async( kwargs={ diff --git a/concrete_datastore/concrete/automation/tasks.py b/concrete_datastore/concrete/automation/tasks.py index a82590a1..e2628615 100644 --- a/concrete_datastore/concrete/automation/tasks.py +++ b/concrete_datastore/concrete/automation/tasks.py @@ -23,6 +23,9 @@ from concrete_datastore.api.v1.permissions import ( update_created_by_permissions, create_or_update_instance_permission_per_user, + check_instance_permissions_per_user, + bulk_create_permission_instances, + bulk_update_permission_instances, ) @@ -33,9 +36,21 @@ DIVIDER_MODEL_LOWER = DIVIDER_MODEL.lower() +@app.task +def check_all_user_permissions_async(user_pk, new_level=None): + user = get_user_model().objects.get(pk=user_pk) + instances_to_create, instances_to_update = check_instance_permissions_per_user( + user=user, user_level=new_level + ) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) + + @app.task def on_update_divider_async(pk_set, instance_uid, include_divider): divider_model = getattr(concrete_datastore.concrete.models, DIVIDER_MODEL) + instances_to_create = [] + instances_to_update = [] for meta_model in meta_models: model_name = meta_model.get_model_name() if model_name in ('User', 'Group', 'Email'): @@ -47,13 +62,21 @@ def on_update_divider_async(pk_set, instance_uid, include_divider): related_divider_field_name = f'divider_{related_name}s' for divider_pk in pk_set: divider_instance = divider_model.objects.get(pk=divider_pk) - create_or_update_instance_permission_per_user( + instance, should_create = create_or_update_instance_permission_per_user( user=get_user_model().objects.get(pk=instance_uid), instances_qs=getattr( divider_instance, related_divider_field_name ).all(), include_divider=include_divider, ) + if instance is None: + continue + if should_create is True: + instances_to_create.append(instance) + else: + instances_to_update.append(instance) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) @app.task @@ -65,6 +88,8 @@ def on_update_group_members_async( concrete_datastore.concrete.models, instance_model_name ) instance = instance_model.objects.get(pk=instance_uid) + instances_to_create = [] + instances_to_update = [] for meta_model in meta_models: model_name = meta_model.get_model_name() if model_name in ('User', 'Group', 'Email'): @@ -79,12 +104,18 @@ def on_update_group_members_async( administrable_qs = getattr(instance, group_admin_field_name).all() for user_pk in pk_set: user = user_model.objects.get(pk=user_pk) - create_or_update_instance_permission_per_user( + perm_instance, should_create = create_or_update_instance_permission_per_user( user=user, instances_qs=viewable_qs | administrable_qs, include_admin_groups=include_pks, include_view_groups=include_pks, ) + if perm_instance is None: + continue + if should_create is True: + instances_to_create.append(perm_instance) + else: + instances_to_update.append(perm_instance) else: #: Instance is User user_viewable_field_name = f'viewable_{related_model_name}s' @@ -97,18 +128,28 @@ def on_update_group_members_async( instances_qs = concrete_model.objects.filter( Q(can_admin_groups__in=pk_set) | Q(can_view_groups__in=pk_set) ) - create_or_update_instance_permission_per_user( + perm_instance, should_create = create_or_update_instance_permission_per_user( user=instance, instances_qs=instances_qs, include_view_users=include_pks, include_admin_users=include_pks, ) + if perm_instance is None: + continue + if should_create is True: + instances_to_create.append(perm_instance) + else: + instances_to_update.append(perm_instance) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) @app.task def on_create_instance_async(user_pk, model_name, instance_pk): model = getattr(concrete_datastore.concrete.models, model_name) instance = model.objects.get(pk=instance_pk) + instances_to_create = [] + instances_to_update = [] if user_pk is not None: user = get_user_model().objects.get(pk=user_pk) if user.is_at_least_admin is False: @@ -125,9 +166,17 @@ def on_create_instance_async(user_pk, model_name, instance_pk): ): if user.is_at_least_admin: continue - create_or_update_instance_permission_per_user( + instance, should_create = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk) ) + if instance is None: + continue + if should_create is True: + instances_to_create.append(instance) + else: + instances_to_update.append(instance) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) @app.task @@ -141,13 +190,23 @@ def on_view_admin_groups_changed_async( .distinct() ) model = getattr(concrete_datastore.concrete.models, model_name) + instances_to_create = [] + instances_to_update = [] for user in user_model.objects.filter(pk__in=users_ids): - create_or_update_instance_permission_per_user( + instance, should_create = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk), include_view_groups=include_view_groups, include_admin_groups=include_admin_groups, ) + if instance is None: + continue + if should_create is True: + instances_to_create.append(instance) + else: + instances_to_update.append(instance) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) @app.task @@ -156,13 +215,23 @@ def on_view_admin_users_changed_async( ): user_model = get_user_model() model = getattr(concrete_datastore.concrete.models, model_name) + instances_to_create = [] + instances_to_update = [] for user in user_model.objects.filter(pk__in=pk_set): - create_or_update_instance_permission_per_user( + instance, should_create = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk), include_admin_users=include_admin_users, include_view_users=include_view_users, ) + if instance is None: + continue + if should_create is True: + instances_to_create.append(instance) + else: + instances_to_update.append(instance) + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) @app.task diff --git a/concrete_datastore/concrete/management/commands/init_app.py b/concrete_datastore/concrete/management/commands/init_app.py index 2382df4e..865fe8ac 100644 --- a/concrete_datastore/concrete/management/commands/init_app.py +++ b/concrete_datastore/concrete/management/commands/init_app.py @@ -11,6 +11,8 @@ ) from concrete_datastore.api.v1.permissions import ( check_instance_permissions_per_user, + bulk_create_permission_instances, + bulk_update_permission_instances, ) from concrete_datastore.concrete.models import ( DIVIDER_MODEL, @@ -82,10 +84,19 @@ def setup_versions_and_permissions(): app_name='datamodel', version=datamodel_version ) if datamodel_changed is True: + instances_to_create = [] + instances_to_update = [] for user in get_user_model().objects.filter( is_active=True, admin=False, is_superuser=False ): - check_instance_permissions_per_user(user=user) + partial_instances_to_create, partial_instances_to_update = check_instance_permissions_per_user( + user=user + ) + instances_to_create.extend(partial_instances_to_create) + instances_to_update.extend(partial_instances_to_update) + + bulk_create_permission_instances(instances_to_create) + bulk_update_permission_instances(instances_to_update) class Command(BaseCommand): diff --git a/concrete_datastore/settings/base.py b/concrete_datastore/settings/base.py index 1f4cc226..20f4218e 100644 --- a/concrete_datastore/settings/base.py +++ b/concrete_datastore/settings/base.py @@ -656,3 +656,5 @@ #: The TOTP issuer used for the QR-Code. Leave to None to use the same value #: as PLATFORM_NAME OTP_TOTP_ISSUER = None + +BATCH_SIZE_FOR_BULK_OPERATIONS = 1000 diff --git a/tests/test_init_app.py b/tests/test_init_app.py index 4aa0a378..54309bf0 100644 --- a/tests/test_init_app.py +++ b/tests/test_init_app.py @@ -29,6 +29,10 @@ def test_empty_db(self): self.assertTrue(datamodel_version.is_latest) def test_with_users(self): + #: in this test, we will create users and a Project. We are gonna + #: set the can_view_users of this project and the delete the + #: InstancePermissions. We are going to check that the init_app + #: command creates the right instance permissions self.assertEqual(InstancePermission.objects.count(), 0) simple = User.objects.create(email="simple@netsach.org") manager = User.objects.create(email="manager@netsach.org") @@ -44,7 +48,10 @@ def test_with_users(self): self.assertEqual(InstancePermission.objects.count(), 0) #: Create a project - Project.objects.create() + p = Project.objects.create() + p.can_view_users.set({simple.pk, manager.pk, admin.pk}) + self.assertEqual(InstancePermission.objects.count(), 2) + InstancePermission.objects.all().delete() self.assertEqual(InstancePermission.objects.count(), 0) call_command('init_app') #: We expect 2 InstancePermissions: simple + manager @@ -64,10 +71,3 @@ def test_with_users(self): self.assertEqual( InstancePermission.objects.filter(user_id=admin.pk).count(), 1 ) - - #: If we create a new simpleUser, we expect a new InstancePermission - simple2 = User.objects.create(email="simple2@netsach.org") - self.assertEqual(InstancePermission.objects.count(), 4) - self.assertEqual( - InstancePermission.objects.filter(user_id=simple2.pk).count(), 1 - ) From b09de3cc3f937ea0cb09f2196ba465f9c6be195d Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Mon, 27 Feb 2023 12:20:42 +0100 Subject: [PATCH 5/7] Update black --- concrete_datastore/api/v1/permissions.py | 5 ++- .../concrete/automation/tasks.py | 37 ++++++++++++++----- .../concrete/management/commands/init_app.py | 7 ++-- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/concrete_datastore/api/v1/permissions.py b/concrete_datastore/api/v1/permissions.py index c72ecfd0..9a6abe1a 100644 --- a/concrete_datastore/api/v1/permissions.py +++ b/concrete_datastore/api/v1/permissions.py @@ -694,7 +694,10 @@ def check_instance_permissions_per_user(user, user_level=None): f'Checking permission for user {user} on {queryset.count()} ' f'instances of model {model_name}' ) - instance, should_create = create_or_update_instance_permission_per_user( + ( + instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=user, instances_qs=queryset, user_level=user_level ) if instance is None: diff --git a/concrete_datastore/concrete/automation/tasks.py b/concrete_datastore/concrete/automation/tasks.py index e2628615..21cf3011 100644 --- a/concrete_datastore/concrete/automation/tasks.py +++ b/concrete_datastore/concrete/automation/tasks.py @@ -39,9 +39,10 @@ @app.task def check_all_user_permissions_async(user_pk, new_level=None): user = get_user_model().objects.get(pk=user_pk) - instances_to_create, instances_to_update = check_instance_permissions_per_user( - user=user, user_level=new_level - ) + ( + instances_to_create, + instances_to_update, + ) = check_instance_permissions_per_user(user=user, user_level=new_level) bulk_create_permission_instances(instances_to_create) bulk_update_permission_instances(instances_to_update) @@ -62,7 +63,10 @@ def on_update_divider_async(pk_set, instance_uid, include_divider): related_divider_field_name = f'divider_{related_name}s' for divider_pk in pk_set: divider_instance = divider_model.objects.get(pk=divider_pk) - instance, should_create = create_or_update_instance_permission_per_user( + ( + instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=get_user_model().objects.get(pk=instance_uid), instances_qs=getattr( divider_instance, related_divider_field_name @@ -104,7 +108,10 @@ def on_update_group_members_async( administrable_qs = getattr(instance, group_admin_field_name).all() for user_pk in pk_set: user = user_model.objects.get(pk=user_pk) - perm_instance, should_create = create_or_update_instance_permission_per_user( + ( + perm_instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=user, instances_qs=viewable_qs | administrable_qs, include_admin_groups=include_pks, @@ -128,7 +135,10 @@ def on_update_group_members_async( instances_qs = concrete_model.objects.filter( Q(can_admin_groups__in=pk_set) | Q(can_view_groups__in=pk_set) ) - perm_instance, should_create = create_or_update_instance_permission_per_user( + ( + perm_instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=instance, instances_qs=instances_qs, include_view_users=include_pks, @@ -166,7 +176,10 @@ def on_create_instance_async(user_pk, model_name, instance_pk): ): if user.is_at_least_admin: continue - instance, should_create = create_or_update_instance_permission_per_user( + ( + instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk) ) if instance is None: @@ -193,7 +206,10 @@ def on_view_admin_groups_changed_async( instances_to_create = [] instances_to_update = [] for user in user_model.objects.filter(pk__in=users_ids): - instance, should_create = create_or_update_instance_permission_per_user( + ( + instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk), include_view_groups=include_view_groups, @@ -218,7 +234,10 @@ def on_view_admin_users_changed_async( instances_to_create = [] instances_to_update = [] for user in user_model.objects.filter(pk__in=pk_set): - instance, should_create = create_or_update_instance_permission_per_user( + ( + instance, + should_create, + ) = create_or_update_instance_permission_per_user( user=user, instances_qs=model.objects.filter(pk=instance_pk), include_admin_users=include_admin_users, diff --git a/concrete_datastore/concrete/management/commands/init_app.py b/concrete_datastore/concrete/management/commands/init_app.py index 865fe8ac..b07097b8 100644 --- a/concrete_datastore/concrete/management/commands/init_app.py +++ b/concrete_datastore/concrete/management/commands/init_app.py @@ -89,9 +89,10 @@ def setup_versions_and_permissions(): for user in get_user_model().objects.filter( is_active=True, admin=False, is_superuser=False ): - partial_instances_to_create, partial_instances_to_update = check_instance_permissions_per_user( - user=user - ) + ( + partial_instances_to_create, + partial_instances_to_update, + ) = check_instance_permissions_per_user(user=user) instances_to_create.extend(partial_instances_to_create) instances_to_update.extend(partial_instances_to_update) From 95fcb31fd06c009fff61cc13c16a223cb261695a Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Mon, 27 Feb 2023 12:25:09 +0100 Subject: [PATCH 6/7] Restore not needed changes --- concrete_datastore/api/v1/serializers.py | 7 +++++++ concrete_datastore/api/v1_1/serializers.py | 2 ++ concrete_datastore/api/v1_1/views.py | 4 ++++ .../concrete/management/commands/openapispec.py | 1 + concrete_datastore/interfaces/openapi_schema_generator.py | 4 ++++ concrete_datastore/routes/forms.py | 1 + concrete_datastore/settings/utils.py | 1 + 7 files changed, 20 insertions(+) diff --git a/concrete_datastore/api/v1/serializers.py b/concrete_datastore/api/v1/serializers.py index 960bc86c..525869a4 100644 --- a/concrete_datastore/api/v1/serializers.py +++ b/concrete_datastore/api/v1/serializers.py @@ -236,6 +236,7 @@ def get_token(self, obj): def make_related_serializer_class( target_model_name, many, nested=False, api_namespace=DEFAULT_API_NAMESPACE ): + if target_model_name not in meta_registered: raise ValueError(f'Related to unknown model {target_model_name}') @@ -257,9 +258,11 @@ def make_custom_serializer_fields( for model_name, SERIALIZER_SETTINGS in SERIALIZERS_SETTINGS.items(): if meta_model.get_model_name() == model_name: + CUSTOM_FIELDS = SERIALIZER_SETTINGS.get('CUSTOM_FIELDS', {}) for field_name, field_args in CUSTOM_FIELDS.items(): + if 'type' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -267,6 +270,7 @@ def make_custom_serializer_fields( ) if field_args['type'] == 'RelatedModelSerializer': + if 'to' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -358,6 +362,7 @@ def make_serializer_class( # TODO: rajouter les _uid dans _fields fk_read_only_fields = [] for name, field in enum_fields: + if field.type.startswith("rel_"): _fields += ['{}_uid'.format(name)] fk_read_only_fields += [name] @@ -388,6 +393,7 @@ class Meta: # TODO : if field is relational, expose pk and url serialized for name, field in enum_fields: + if field.f_type == 'FileField': attrs.update( { @@ -412,6 +418,7 @@ class Meta: {name: serializers.JSONField(binary=False, required=False)} ) if field.type.startswith("rel_") and nested is True: + force_nested = getattr(field, 'force_nested', False) attrs.update( diff --git a/concrete_datastore/api/v1_1/serializers.py b/concrete_datastore/api/v1_1/serializers.py index d5d387ad..9766223d 100644 --- a/concrete_datastore/api/v1_1/serializers.py +++ b/concrete_datastore/api/v1_1/serializers.py @@ -108,6 +108,7 @@ def get_url(self, obj): class EmailDeviceSerializer(serializers.ModelSerializer): + url = serializers.SerializerMethodField() class Meta: @@ -142,6 +143,7 @@ def get_url(self, obj): class ConcretePermissionSerializer(serializers.ModelSerializer): + create_roles = serializers.StringRelatedField(many=True, read_only=True) update_roles = serializers.StringRelatedField(many=True, read_only=True) retrieve_roles = serializers.StringRelatedField(many=True, read_only=True) diff --git a/concrete_datastore/api/v1_1/views.py b/concrete_datastore/api/v1_1/views.py index 6885fbf5..ced1e75e 100644 --- a/concrete_datastore/api/v1_1/views.py +++ b/concrete_datastore/api/v1_1/views.py @@ -146,6 +146,7 @@ class ProcessRegisterApiView(generics.GenericAPIView): serializer_class = ProcessRegisterSerializer def post(self, request, *args, **kwargs): + """ 1. Check if the token is not already created and that it is valid 2. Check if the user(process) is not already created with a different token @@ -449,11 +450,13 @@ def get_extra_informations(self, queryset): } def perform_create(self, serializer): + attrs = {'created_by': self.request.user, 'user': self.request.user} serializer.save(**attrs) class ConcreteRoleApiView(PaginatedViewSet, viewsets.ModelViewSet): + model_class = ConcreteRole permission_classes = (IsAuthenticated, ConcreteRolesPermission) api_namespace = DEFAULT_API_NAMESPACE @@ -499,6 +502,7 @@ class ConcretePermissionApiView( mixins.ListModelMixin, viewsets.GenericViewSet, ): + model_class = ConcretePermission permission_classes = (IsAuthenticated, ConcreteRolesPermission) serializer_class = ConcretePermissionSerializer diff --git a/concrete_datastore/concrete/management/commands/openapispec.py b/concrete_datastore/concrete/management/commands/openapispec.py index 71b7b210..4f84707e 100644 --- a/concrete_datastore/concrete/management/commands/openapispec.py +++ b/concrete_datastore/concrete/management/commands/openapispec.py @@ -71,6 +71,7 @@ def add_arguments(self, parser): ) def handle(self, *args, **options): + url_patterns_dict = { 'v1': (api_v1_urls,), 'v1.1': (api_v1_1_urls,), diff --git a/concrete_datastore/interfaces/openapi_schema_generator.py b/concrete_datastore/interfaces/openapi_schema_generator.py index 4a475412..d42822af 100644 --- a/concrete_datastore/interfaces/openapi_schema_generator.py +++ b/concrete_datastore/interfaces/openapi_schema_generator.py @@ -354,6 +354,7 @@ def get_schema(self, request=None, public=False): class AutoSchema(AutoSchemaSuper): + components = {} def get_component_schemas(self): @@ -465,6 +466,7 @@ def get_custom_path_parameters(self, path, method): if model is not None: # Attempt to infer a field description if possible. try: + model_field = model._meta.get_field(variable) except Exception: model_field = None @@ -486,6 +488,7 @@ def get_custom_path_parameters(self, path, method): return parameters def custom_map_field(self, field): + # Related fields. if isinstance(field, serializers.ManyRelatedField): return { @@ -537,6 +540,7 @@ def custom_map_serializer(self, serializer, nested=True): properties = {} for field in serializer.fields.values(): + if ( isinstance(field, serializers.PrimaryKeyRelatedField) and nested is False diff --git a/concrete_datastore/routes/forms.py b/concrete_datastore/routes/forms.py index e6e7b257..9c8b54a9 100644 --- a/concrete_datastore/routes/forms.py +++ b/concrete_datastore/routes/forms.py @@ -7,6 +7,7 @@ class ConfigureOTPLoginForm(forms.Form): + otp_error_messages = { 'token_required': _('Please enter your OTP token.'), 'challenge_exception': _('Error generating challenge: {0}'), diff --git a/concrete_datastore/settings/utils.py b/concrete_datastore/settings/utils.py index c8eff7fe..c42d1e76 100644 --- a/concrete_datastore/settings/utils.py +++ b/concrete_datastore/settings/utils.py @@ -9,6 +9,7 @@ def load_datamodel(datamodel_path='current-datamodel.meta'): + datamodel_path = os.getenv('DATAMODEL_FILE') or datamodel_path try: From 1e5423f79fd1f46b00e05835929c423a9b203615 Mon Sep 17 00:00:00 2001 From: Khaled Bousrih Date: Mon, 27 Feb 2023 12:32:23 +0100 Subject: [PATCH 7/7] Update black version --- CHANGELOG.md | 3 ++- concrete_datastore/admin/admin.py | 10 +++++----- concrete_datastore/api/v1/serializers.py | 7 ------- concrete_datastore/api/v1_1/serializers.py | 2 -- concrete_datastore/api/v1_1/views.py | 4 ---- .../concrete/automation/signal_processor.py | 2 +- .../concrete/management/commands/openapispec.py | 1 - .../interfaces/openapi_schema_generator.py | 4 ---- concrete_datastore/routes/forms.py | 1 - concrete_datastore/settings/utils.py | 1 - development/settings.py | 3 +-- 11 files changed, 9 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b07f3273..9d089c1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ ### Changed -- Changed the logic of read/write permissions by user: A user that has read/write access to an instance of the model Model, will have an instance of InstancePermission (with the model_name="Model") and the uid of the instance will be n the read/write_instance_uids +- Changed the logic of read/write permissions by user: A user that has read/write access to an instance of the model Model, will have an instance of InstancePermission (with the model_name="Model") and the uid of the instance will be in the read/write_instance_uids +- Update black version ### Removed diff --git a/concrete_datastore/admin/admin.py b/concrete_datastore/admin/admin.py index ecf2a38f..1e75f98c 100644 --- a/concrete_datastore/admin/admin.py +++ b/concrete_datastore/admin/admin.py @@ -268,12 +268,12 @@ class InstancePermissionAdmin(SaveModelMixin, admin.ModelAdmin): @admin.register(SystemVersion, site=admin_site) class SystemVersionAdmin(SaveModelMixin, admin.ModelAdmin): - @admin.action(description='Set selected versions as the latest versions') - def set_is_latest_to_true(self, request, queryset): + @admin.action(description='Tag as latest') + def tag_as_latest(self, request, queryset): queryset.update(is_latest=True) - @admin.action(description='Set selected versions as earlier versions') - def set_is_latest_to_false(self, request, queryset): + @admin.action(description='Untag as latest') + def untag_as_latest(self, request, queryset): queryset.update(is_latest=False) list_display = [ @@ -299,4 +299,4 @@ def set_is_latest_to_false(self, request, queryset): ] list_filter = ['app_name', 'is_latest'] - actions = ['set_is_latest_to_true', 'set_is_latest_to_false'] + actions = ['tag_as_latest', 'untag_as_latest'] diff --git a/concrete_datastore/api/v1/serializers.py b/concrete_datastore/api/v1/serializers.py index 525869a4..960bc86c 100644 --- a/concrete_datastore/api/v1/serializers.py +++ b/concrete_datastore/api/v1/serializers.py @@ -236,7 +236,6 @@ def get_token(self, obj): def make_related_serializer_class( target_model_name, many, nested=False, api_namespace=DEFAULT_API_NAMESPACE ): - if target_model_name not in meta_registered: raise ValueError(f'Related to unknown model {target_model_name}') @@ -258,11 +257,9 @@ def make_custom_serializer_fields( for model_name, SERIALIZER_SETTINGS in SERIALIZERS_SETTINGS.items(): if meta_model.get_model_name() == model_name: - CUSTOM_FIELDS = SERIALIZER_SETTINGS.get('CUSTOM_FIELDS', {}) for field_name, field_args in CUSTOM_FIELDS.items(): - if 'type' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -270,7 +267,6 @@ def make_custom_serializer_fields( ) if field_args['type'] == 'RelatedModelSerializer': - if 'to' not in field_args: raise ValueError( 'CONCRETE improperly configured : custom field ' @@ -362,7 +358,6 @@ def make_serializer_class( # TODO: rajouter les _uid dans _fields fk_read_only_fields = [] for name, field in enum_fields: - if field.type.startswith("rel_"): _fields += ['{}_uid'.format(name)] fk_read_only_fields += [name] @@ -393,7 +388,6 @@ class Meta: # TODO : if field is relational, expose pk and url serialized for name, field in enum_fields: - if field.f_type == 'FileField': attrs.update( { @@ -418,7 +412,6 @@ class Meta: {name: serializers.JSONField(binary=False, required=False)} ) if field.type.startswith("rel_") and nested is True: - force_nested = getattr(field, 'force_nested', False) attrs.update( diff --git a/concrete_datastore/api/v1_1/serializers.py b/concrete_datastore/api/v1_1/serializers.py index 9766223d..d5d387ad 100644 --- a/concrete_datastore/api/v1_1/serializers.py +++ b/concrete_datastore/api/v1_1/serializers.py @@ -108,7 +108,6 @@ def get_url(self, obj): class EmailDeviceSerializer(serializers.ModelSerializer): - url = serializers.SerializerMethodField() class Meta: @@ -143,7 +142,6 @@ def get_url(self, obj): class ConcretePermissionSerializer(serializers.ModelSerializer): - create_roles = serializers.StringRelatedField(many=True, read_only=True) update_roles = serializers.StringRelatedField(many=True, read_only=True) retrieve_roles = serializers.StringRelatedField(many=True, read_only=True) diff --git a/concrete_datastore/api/v1_1/views.py b/concrete_datastore/api/v1_1/views.py index ced1e75e..6885fbf5 100644 --- a/concrete_datastore/api/v1_1/views.py +++ b/concrete_datastore/api/v1_1/views.py @@ -146,7 +146,6 @@ class ProcessRegisterApiView(generics.GenericAPIView): serializer_class = ProcessRegisterSerializer def post(self, request, *args, **kwargs): - """ 1. Check if the token is not already created and that it is valid 2. Check if the user(process) is not already created with a different token @@ -450,13 +449,11 @@ def get_extra_informations(self, queryset): } def perform_create(self, serializer): - attrs = {'created_by': self.request.user, 'user': self.request.user} serializer.save(**attrs) class ConcreteRoleApiView(PaginatedViewSet, viewsets.ModelViewSet): - model_class = ConcreteRole permission_classes = (IsAuthenticated, ConcreteRolesPermission) api_namespace = DEFAULT_API_NAMESPACE @@ -502,7 +499,6 @@ class ConcretePermissionApiView( mixins.ListModelMixin, viewsets.GenericViewSet, ): - model_class = ConcretePermission permission_classes = (IsAuthenticated, ConcreteRolesPermission) serializer_class = ConcretePermissionSerializer diff --git a/concrete_datastore/concrete/automation/signal_processor.py b/concrete_datastore/concrete/automation/signal_processor.py index 01173aae..cc6d696d 100644 --- a/concrete_datastore/concrete/automation/signal_processor.py +++ b/concrete_datastore/concrete/automation/signal_processor.py @@ -97,7 +97,7 @@ def on_pre_save(sender, instance, *args, **kwargs): @receiver(post_save, sender=get_user_model()) -def on_post_save(sender, instance, created, **kwargs): +def on_post_save(sender, instance, **kwargs): if instance.level == 'blocked': divider_manager = getattr( instance, '{}s'.format(DIVIDER_MODEL.lower()) diff --git a/concrete_datastore/concrete/management/commands/openapispec.py b/concrete_datastore/concrete/management/commands/openapispec.py index 4f84707e..71b7b210 100644 --- a/concrete_datastore/concrete/management/commands/openapispec.py +++ b/concrete_datastore/concrete/management/commands/openapispec.py @@ -71,7 +71,6 @@ def add_arguments(self, parser): ) def handle(self, *args, **options): - url_patterns_dict = { 'v1': (api_v1_urls,), 'v1.1': (api_v1_1_urls,), diff --git a/concrete_datastore/interfaces/openapi_schema_generator.py b/concrete_datastore/interfaces/openapi_schema_generator.py index d42822af..4a475412 100644 --- a/concrete_datastore/interfaces/openapi_schema_generator.py +++ b/concrete_datastore/interfaces/openapi_schema_generator.py @@ -354,7 +354,6 @@ def get_schema(self, request=None, public=False): class AutoSchema(AutoSchemaSuper): - components = {} def get_component_schemas(self): @@ -466,7 +465,6 @@ def get_custom_path_parameters(self, path, method): if model is not None: # Attempt to infer a field description if possible. try: - model_field = model._meta.get_field(variable) except Exception: model_field = None @@ -488,7 +486,6 @@ def get_custom_path_parameters(self, path, method): return parameters def custom_map_field(self, field): - # Related fields. if isinstance(field, serializers.ManyRelatedField): return { @@ -540,7 +537,6 @@ def custom_map_serializer(self, serializer, nested=True): properties = {} for field in serializer.fields.values(): - if ( isinstance(field, serializers.PrimaryKeyRelatedField) and nested is False diff --git a/concrete_datastore/routes/forms.py b/concrete_datastore/routes/forms.py index 9c8b54a9..e6e7b257 100644 --- a/concrete_datastore/routes/forms.py +++ b/concrete_datastore/routes/forms.py @@ -7,7 +7,6 @@ class ConfigureOTPLoginForm(forms.Form): - otp_error_messages = { 'token_required': _('Please enter your OTP token.'), 'challenge_exception': _('Error generating challenge: {0}'), diff --git a/concrete_datastore/settings/utils.py b/concrete_datastore/settings/utils.py index c42d1e76..c8eff7fe 100644 --- a/concrete_datastore/settings/utils.py +++ b/concrete_datastore/settings/utils.py @@ -9,7 +9,6 @@ def load_datamodel(datamodel_path='current-datamodel.meta'): - datamodel_path = os.getenv('DATAMODEL_FILE') or datamodel_path try: diff --git a/development/settings.py b/development/settings.py index f2abde51..14b907ca 100644 --- a/development/settings.py +++ b/development/settings.py @@ -3,7 +3,6 @@ from concrete_datastore.settings.base import * from concrete_datastore.settings.utils import load_datamodel - PROJECT_ROOT = os.path.dirname(os.path.realpath(__file__)) + '/' MEDIA_ROOT = os.path.join(PROJECT_ROOT, 'media/') STATIC_ROOT = os.path.join(PROJECT_ROOT, 'static/') @@ -84,4 +83,4 @@ API_REGISTER_EMAIL_FILTER = r'.*' -USE_TWO_FACTOR_AUTH = False +USE_TWO_FACTOR_AUTH = True