diff --git a/cookbook/admin.py b/cookbook/admin.py index 41687f9cb..b5fa6640b 100644 --- a/cookbook/admin.py +++ b/cookbook/admin.py @@ -81,8 +81,8 @@ admin.site.register(Space, SpaceAdmin) class UserPreferenceAdmin(admin.ModelAdmin): - list_display = ('name', 'space', 'theme', 'nav_color', 'default_page', 'search_style',) # TODO add new fields - search_fields = ('user__username', 'space__name') + list_display = ('name', 'theme', 'nav_color', 'default_page', 'search_style',) # TODO add new fields + search_fields = ('user__username',) list_filter = ('theme', 'nav_color', 'default_page', 'search_style') date_hierarchy = 'created_at' diff --git a/cookbook/helper/permission_helper.py b/cookbook/helper/permission_helper.py index 0331a5993..13de26bd6 100644 --- a/cookbook/helper/permission_helper.py +++ b/cookbook/helper/permission_helper.py @@ -40,8 +40,11 @@ def has_group_permission(user, groups): return False groups_allowed = get_allowed_groups(groups) if user.is_authenticated: - if bool(user.groups.filter(name__in=groups_allowed)): - return True + if user_space := user.userspace_set.filter(active=True): + if len(user_space) != 1: + return False # do not allow any group permission if more than one space is active, needs to be changed when simultaneous multi-space-tenancy is added + if bool(user_space.first().groups.filter(name__in=groups_allowed)): + return True return False diff --git a/cookbook/helper/scope_middleware.py b/cookbook/helper/scope_middleware.py index f5179bcba..df63d5df0 100644 --- a/cookbook/helper/scope_middleware.py +++ b/cookbook/helper/scope_middleware.py @@ -27,13 +27,19 @@ class ScopeMiddleware: return self.get_response(request) with scopes_disabled(): - if request.user.userpreference.space is None and not reverse('account_logout') in request.path: + if request.user.userspace_set.count() == 0 and not reverse('account_logout') in request.path: return views.no_space(request) - if request.user.groups.count() == 0 and not reverse('account_logout') in request.path: + # get active user space, if for some reason more than one space is active select first (group permission checks will fail, this is not intended at this point) + user_space = request.user.userspace_set.filter(active=True).first() + + if not user_space: + pass # TODO show space selection page (maybe include in no space page) + + if user_space.groups.count() == 0 and not reverse('account_logout') in request.path: return views.no_groups(request) - request.space = request.user.userpreference.space + request.space = user_space.space # with scopes_disabled(): with scope(space=request.space): return self.get_response(request) @@ -41,9 +47,11 @@ class ScopeMiddleware: if request.path.startswith(prefix + '/api/'): try: if auth := TokenAuthentication().authenticate(request): - request.space = auth[0].userpreference.space - with scope(space=request.space): - return self.get_response(request) + user_space = auth[0].userspace_set.filter(active=True).first() + if user_space: + request.space = user_space.space + with scope(space=request.space): + return self.get_response(request) except AuthenticationFailed: pass diff --git a/cookbook/migrations/0174_alter_food_substitute_userspace.py b/cookbook/migrations/0174_alter_food_substitute_userspace.py new file mode 100644 index 000000000..9fddb151e --- /dev/null +++ b/cookbook/migrations/0174_alter_food_substitute_userspace.py @@ -0,0 +1,45 @@ +# Generated by Django 4.0.4 on 2022-05-31 14:10 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +from django_scopes import scopes_disabled + + +def migrate_space_permissions(apps, schema_editor): + with scopes_disabled(): + UserPreference = apps.get_model('cookbook', 'UserPreference') + UserSpace = apps.get_model('cookbook', 'UserSpace') + + for up in UserPreference.objects.exclude(space=None).all(): + us = UserSpace.objects.create(user=up.user, space=up.space, active=True) + us.groups.set(up.user.groups.all()) + + +class Migration(migrations.Migration): + dependencies = [ + ('auth', '0012_alter_user_first_name_max_length'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('cookbook', '0173_recipe_source_url'), + ] + + operations = [ + migrations.AlterField( + model_name='food', + name='substitute', + field=models.ManyToManyField(blank=True, to='cookbook.food'), + ), + migrations.CreateModel( + name='UserSpace', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('active', models.BooleanField(default=False)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('groups', models.ManyToManyField(to='auth.group')), + ('space', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='cookbook.space')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.RunPython(migrate_space_permissions) + ] diff --git a/cookbook/migrations/0175_remove_userpreference_space.py b/cookbook/migrations/0175_remove_userpreference_space.py new file mode 100644 index 000000000..e0455aa2d --- /dev/null +++ b/cookbook/migrations/0175_remove_userpreference_space.py @@ -0,0 +1,17 @@ +# Generated by Django 4.0.4 on 2022-05-31 14:56 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('cookbook', '0174_alter_food_substitute_userspace'), + ] + + operations = [ + migrations.RemoveField( + model_name='userpreference', + name='space', + ), + ] diff --git a/cookbook/models.py b/cookbook/models.py index 2ed79cc51..5b8251dd6 100644 --- a/cookbook/models.py +++ b/cookbook/models.py @@ -2,9 +2,7 @@ import operator import pathlib import re import uuid -from collections import OrderedDict from datetime import date, timedelta -from decimal import Decimal from annoying.fields import AutoOneToOneField from django.contrib import auth @@ -14,10 +12,9 @@ from django.contrib.postgres.search import SearchVectorField from django.core.files.uploadedfile import InMemoryUploadedFile, UploadedFile from django.core.validators import MinLengthValidator from django.db import IntegrityError, models -from django.db.models import Index, ProtectedError, Q, Subquery +from django.db.models import Index, ProtectedError, Q from django.db.models.fields.related import ManyToManyField from django.db.models.functions import Substr -from django.db.transaction import atomic from django.utils import timezone from django.utils.translation import gettext as _ from django_prometheus.models import ExportModelOperationsMixin @@ -340,13 +337,25 @@ class UserPreference(models.Model, PermissionModelMixin): csv_prefix = models.CharField(max_length=10, blank=True, ) created_at = models.DateTimeField(auto_now_add=True) - space = models.ForeignKey(Space, on_delete=models.CASCADE, null=True) objects = ScopedManager(space='space') def __str__(self): return str(self.user) +class UserSpace(models.Model): + user = models.ForeignKey(User, on_delete=models.CASCADE) + space = models.ForeignKey(Space, on_delete=models.CASCADE) + groups = models.ManyToManyField(Group) + + # there should always only be one active space although permission methods are written in such a way + # that having more than one active space should just break certain parts of the application and not leak any data + active = models.BooleanField(default=False) + + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + + class Storage(models.Model, PermissionModelMixin): DROPBOX = 'DB' NEXTCLOUD = 'NEXTCLOUD' diff --git a/cookbook/serializer.py b/cookbook/serializer.py index 6679f8d26..7867587b1 100644 --- a/cookbook/serializer.py +++ b/cookbook/serializer.py @@ -117,7 +117,7 @@ class SpaceFilterSerializer(serializers.ListSerializer): # if query is sliced it came from api request not nested serializer return super().to_representation(data) if self.child.Meta.model == User: - data = data.filter(userpreference__space=self.context['request'].space) + data = User.objects.filter(userspace__space=self.context['request'].space).all() else: data = data.filter(**{'__'.join(data.model.get_space_key()): self.context['request'].space}) return super().to_representation(data) diff --git a/cookbook/templatetags/custom_tags.py b/cookbook/templatetags/custom_tags.py index 7f09613b2..f0fedc051 100644 --- a/cookbook/templatetags/custom_tags.py +++ b/cookbook/templatetags/custom_tags.py @@ -163,8 +163,7 @@ def base_path(request, path_type): @register.simple_tag def user_prefs(request): - from cookbook.serializer import \ - UserPreferenceSerializer # putting it with imports caused circular execution + from cookbook.serializer import UserPreferenceSerializer # putting it with imports caused circular execution try: return UserPreferenceSerializer(request.user.userpreference, context={'request': request}).data except AttributeError: diff --git a/cookbook/tests/factories/__init__.py b/cookbook/tests/factories/__init__.py index 77b59199a..9562db722 100644 --- a/cookbook/tests/factories/__init__.py +++ b/cookbook/tests/factories/__init__.py @@ -10,7 +10,7 @@ from django_scopes import scopes_disabled from faker import Factory as FakerFactory from pytest_factoryboy import register -from cookbook.models import Recipe, Step +from cookbook.models import Recipe, Step, UserSpace # this code will run immediately prior to creating the model object useful when you want a reverse relationship # log = factory.RelatedFactory( @@ -65,7 +65,8 @@ class UserFactory(factory.django.DjangoModelFactory): return if extracted: - self.groups.add(Group.objects.get(name=extracted)) + us = UserSpace.objects.create(space=self.space, user=self, active=True) + us.groups.add(Group.objects.get(name=extracted)) @factory.post_generation def userpreference(self, create, extracted, **kwargs): diff --git a/cookbook/tests/other/test_permission_helper.py b/cookbook/tests/other/test_permission_helper.py new file mode 100644 index 000000000..e12ec2017 --- /dev/null +++ b/cookbook/tests/other/test_permission_helper.py @@ -0,0 +1,38 @@ +import pytest +from django.contrib import auth +from django.contrib.auth.models import Group +from django.urls import reverse +from django_scopes import scopes_disabled + +from cookbook.forms import ImportExportBase +from cookbook.helper.ingredient_parser import IngredientParser +from cookbook.helper.permission_helper import has_group_permission +from cookbook.models import ExportLog, UserSpace + + +def test_has_group_permission(u1_s1, a_u, space_2): + with scopes_disabled(): + # test that a normal user has user permissions + assert has_group_permission(auth.get_user(u1_s1), ('guest',)) + assert has_group_permission(auth.get_user(u1_s1), ('user',)) + assert not has_group_permission(auth.get_user(u1_s1), ('admin',)) + + # test that permissions are not taken from non active spaces + us = UserSpace.objects.create(user=auth.get_user(u1_s1), space=space_2, active=False) + us.groups.add(Group.objects.get(name='admin')) + assert not has_group_permission(auth.get_user(u1_s1), ('admin',)) + + # disable all spaces and enable space 2 permission to check if permission is now valid + auth.get_user(u1_s1).userspace_set.update(active=False) + us.active = True + us.save() + assert has_group_permission(auth.get_user(u1_s1), ('admin',)) + + # test that group permission checks fail if more than one userspace is active + auth.get_user(u1_s1).userspace_set.update(active=True) + assert not has_group_permission(auth.get_user(u1_s1), ('user',)) + + # test that anonymous users don't have any permissions + assert not has_group_permission(auth.get_user(a_u), ('guest',)) + assert not has_group_permission(auth.get_user(a_u), ('user',)) + assert not has_group_permission(auth.get_user(a_u), ('admin',)) diff --git a/cookbook/views/views.py b/cookbook/views/views.py index f22030c1a..e27dcae03 100644 --- a/cookbook/views/views.py +++ b/cookbook/views/views.py @@ -29,7 +29,7 @@ from cookbook.forms import (CommentForm, Recipe, SearchPreferenceForm, ShoppingP from cookbook.helper.permission_helper import group_required, has_group_permission, share_link_valid from cookbook.models import (Comment, CookLog, Food, InviteLink, Keyword, MealPlan, RecipeImport, SearchFields, SearchPreference, ShareLink, - Space, Unit, ViewLog) + Space, Unit, ViewLog, UserSpace) from cookbook.tables import (CookLogTable, InviteLinkTable, RecipeTable, RecipeTableSmall, ViewLogTable) from cookbook.views.data import Object @@ -104,7 +104,7 @@ def no_groups(request): @login_required def no_space(request): - if request.user.userpreference.space: + if request.user.userspace_set.count() > 0: return HttpResponseRedirect(reverse('index')) if request.POST: @@ -120,9 +120,8 @@ def no_space(request): allow_sharing=settings.SPACE_DEFAULT_ALLOW_SHARING, ) - request.user.userpreference.space = created_space - request.user.userpreference.save() - request.user.groups.add(Group.objects.filter(name='admin').get()) + user_space = UserSpace.objects.create(space=created_space, user=request.user, active=True) + user_space.groups.add(Group.objects.filter(name='admin').get()) messages.add_message(request, messages.SUCCESS, _('You have successfully created your own recipe space. Start by adding some recipes or invite other people to join you.'))