multi space membership basics

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'
 

cookbook/helper/permission_helper.py

@@ -40,7 +40,10 @@ 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)):
+        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
 

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,7 +47,9 @@ class ScopeMiddleware:
             if request.path.startswith(prefix + '/api/'):
                 try:
                     if auth := TokenAuthentication().authenticate(request):
-                        request.space = auth[0].userpreference.space
+                        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:

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)
+    ]

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',
+        ),
+    ]

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'

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)

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:

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):

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',))

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.'))