Merge pull request #23 from neuroforgede/2.1.0

fix behaviour of user permissions and user endpoints

skipper/skipper/core/components/user/serializers.py

@@ -20,43 +20,29 @@
 
 
 class UserSerializer(serializers.HyperlinkedModelSerializer):
+    """
+    Tenant unspecific User serializer
+    """
     id = serializers.IntegerField(read_only=True)
     url = serializers.HyperlinkedIdentityField(view_name=constants.core_user_view_set_name + '-detail')
     password = serializers.CharField(write_only=True, max_length=128)
-    username = serializers.CharField(max_length=40)
+    username = serializers.CharField(max_length=150)
     permissions = serializers.HyperlinkedIdentityField(view_name=constants.core_user_permission_view_set_name)
 
-    def to_representation(self, instance: Any) -> Any:
-        representation = super().to_representation(instance)
-        split = representation['username'].split('@@')
-        _name: str
-        if len(split) == 1:
-            _name = split[0]
-        else:
-            _name = representation['username'][len(split[0]) + len('@@'):]
-        representation['username'] = _name
-        return representation
-
     def validate_username(self, username: str) -> str:
-        if '@@' in username:
-            raise ValidationError("may not contain '@@'")
-
-        tenant = get_current_tenant()
-        name_with_tenant = f'{tenant.name}@@{username}'
-
         kwargs = self.context.get('view').kwargs  # type: ignore
 
         if 'pk' not in kwargs:
-            if User.objects.all().filter(username=name_with_tenant).exists():
+            if User.objects.all().filter(username=username).exists():
                 raise ValidationError(
                     f'username \'{username}\' is already in use by another User')
         else:
             _id = kwargs['pk']
             _user: User = get_object_or_404(
                 User.objects.filter(id=_id))
-            if name_with_tenant != _user.username:
+            if username != _user.username:
                 raise ValidationError('changing of usernames is not supported!')
-        return name_with_tenant
+        return username
 
     def validate_password(self, password: str) -> str:
         password_validation.validate_password(password)

skipper/skipper/core/components/user/user_permissions.py

@@ -28,12 +28,21 @@
     get_directly_assigned_user_permissions
 )
 from skipper.core.renderers import CustomizableBrowsableAPIRendererObjectMixin
-
-from skipper.core.models.permissions import get_permissions_class
+from skipper.core.models.permissions import get_permissions_class, get_permission_string_for_action_and_http_verb
 
 
 class CoreUserPermissionsViewMixin:
-    permission_classes: Any = (get_permissions_class('user', 'user-permission'),)
+    permission_classes: Any = (get_permissions_class('user', 'user_permission'),)
+
+
+"""
+includes also all permissions that should not be settable on tenant users
+"""
+all_generally_assignable_permissions = set([
+    get_permission_string_for_action_and_http_verb("user", action, method) 
+    for method in ["GET", "OPTIONS", "HEAD", "POST", "PUT", "PATCH", "DELETE"]
+    for action in ["user", "user_permission"]
+]).union(generally_assignable_permissions)
 
 
 class UserPermissionsView(
@@ -59,7 +68,7 @@ def put(self, request: Request, *args: Any, **kwargs: Any) -> Response:
 
     def get_serializer_class(self) -> Any:
 
-        _assignable_permissions = get_assignable_permissions(self.request.user, generally_assignable_permissions)
+        _assignable_permissions = get_assignable_permissions(self.request.user, all_generally_assignable_permissions)
 
         class GenericUserPermissionSerializer(BaseSerializer):
             user_permissions = MultipleChoiceField(
@@ -71,7 +80,7 @@ def to_representation(self, obj: User) -> Any:
                 # only the directly assigned ones
                 representation = {
                     'user_permissions': sorted(
-                        list(get_directly_assigned_user_permissions(obj, generally_assignable_permissions))
+                        list(get_directly_assigned_user_permissions(obj, all_generally_assignable_permissions))
                     )
                 }
                 return representation
@@ -84,7 +93,7 @@ def update(self, instance: Model, validated_data: Dict[str, Any]) -> Model:
                     new_permissions = set(validated_data['user_permissions'])
 
                     _directly_assigned = get_directly_assigned_user_permissions(
-                        cast(User, instance), generally_assignable_permissions
+                        cast(User, instance), all_generally_assignable_permissions
                     )
 
                     to_remove = (_directly_assigned - new_permissions)

skipper/skipper/core/constants.py

@@ -22,4 +22,4 @@ def component_root(component: str) -> str:
 core_tenant_view_set_name = skipper_base_name('tenant')
 core_tenant_user_view_set_name = skipper_base_name('tenant_user')
 core_user_view_set_name = skipper_base_name('user')
-core_user_permission_view_set_name = skipper_base_name('user-permission')
+core_user_permission_view_set_name = skipper_base_name('user_permission')

skipper/skipper/core/migrations/0035_alter_coreuserpermissionspermissions_options.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.1.9 on 2023-06-09 19:34
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0034_alter_tenant_user_options'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='coreuserpermissionspermissions',
+            options={'default_permissions': [], 'managed': False, 'permissions': [('user_get_user_permission', 'Can run GET on entity user and action user_permission'), ('user_options_user_permission', 'Can run OPTIONS on entity user on action user_permission'), ('user_head_user_permission', 'Can run HEAD on entity user on action user_permission'), ('user_post_user_permission', 'Can run POST on entity user on action user_permission'), ('user_put_user_permission', 'Can run PUT on entity user on action user_permission'), ('user_patch_user_permission', 'Can run PATCH on entity user on action user_permission'), ('user_delete_user_permission', 'Can run DELETE on entity user on action user_permission')]},
+        ),
+    ]

skipper/skipper/core/models/permissions.py

@@ -64,7 +64,7 @@ def get_permissions_class(entity: str, action: str) -> Type[permissions.DjangoMo
     Example:
     
         * changing core/user: entity=user, action=user
-        * changing core/user's permission field: entity=user, action=user-permission
+        * changing core/user's permission field: entity=user, action=user_permission
         
     """
     class Permission(permissions.DjangoModelPermissions):
@@ -84,7 +84,7 @@ class Permission(permissions.DjangoModelPermissions):
 # TODO This is only a hack-solution until we have a custom user model
 class CoreUserPermissions(Model):
     """
-    global flow permissions, not really a model
+    global core permissions, not really a model
     that stores any real data
     """
     class Meta:
@@ -99,13 +99,13 @@ class Meta:
 # TODO This is only a hack-solution until we have a custom user model
 class CoreUserPermissionsPermissions(Model):
     """
-    global flow permissions, not really a model
+    global core permissions, not really a model
     that stores any real data
     """
     class Meta:
         managed = False  # No database table creation or deletion  \
         # operations will be performed for this model
         default_permissions: List[str] = []
         permissions = (
-            gen_permissions(entity='user', action='user-permission')
+            gen_permissions(entity='user', action='user_permission')
         )

skipper/skipper/core/tests/components/user/test_user_permissions_detail.py

@@ -39,9 +39,9 @@ def base_add_bare_user(self) -> None:
         self.user_dict = self.create_payload(
             url=USER_LIST_URL,
             payload={
-                'username': 'test_user',
+                'username': 'some_test_user',
                 'password': 'lkshdfjkghjashgoi4ewrpbn',
-                'email': 'test_user@neurofoege.de',
+                'email': 'some_test_user@neurofoege.de',
                 'groups': [],
                 'is_active': True
             },

skipper/skipper/core/tests/components/user/test_user_permissions_list.py

@@ -124,9 +124,9 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
         return self.user_client.post(
             path=self.url_under_test,
             data={
-                'username': 'test_user',
+                'username': 'some_test_user',
                 'password': 'lkshdfjkghjashgoi4ewrpbn',
-                'email': 'test_user@neurofoege.de',
+                'email': 'some_test_user@neurofoege.de',
                 'groups': [],
                 'is_active': True
             },

skipper/skipper/core/tests/components/user/test_userpermissions_permissions.py

@@ -39,9 +39,9 @@ def base_add_bare_user(self) -> None:
         self.user_dict = self.create_payload(
             url=USER_LIST_URL,
             payload={
-                'username': 'test_user',
+                'username': 'some_test_user',
                 'password': 'lkshdfjkghjashgoi4ewrpbn',
-                'email': 'test_user@neurofoege.de',
+                'email': 'some_test_user@neurofoege.de',
                 'is_active': True
             },
             simulate_tenant=False
@@ -51,7 +51,7 @@ def base_add_bare_user(self) -> None:
 class CoreUserPermissionPermissionGETTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'GET', 'user-permission')
+        return core_permission_for_rest_method('user', 'GET', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None
@@ -71,7 +71,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionHEADTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'HEAD', 'user-permission')
+        return core_permission_for_rest_method('user', 'HEAD', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None
@@ -92,7 +92,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionPOSTTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'POST', 'user-permission')
+        return core_permission_for_rest_method('user', 'POST', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None
@@ -116,7 +116,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionOPTIONSTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'OPTIONS', 'user-permission')
+        return core_permission_for_rest_method('user', 'OPTIONS', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None
@@ -136,7 +136,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionPUTTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'PUT', 'user-permission')
+        return core_permission_for_rest_method('user', 'PUT', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return self.user_client.put(
@@ -166,7 +166,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionPATCHTest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'PATCH', 'user-permission')
+        return core_permission_for_rest_method('user', 'PATCH', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None
@@ -190,7 +190,7 @@ def method_under_test_proper(self) -> Union[HttpResponse, TestHttpResponse]:
 class CoreUserPermissionPermissionDELETETest(CoreUserPermissionPermissionBaseTest):
 
     def permission_code_name(self) -> str:
-        return core_permission_for_rest_method('user', 'DELETE', 'user-permission')
+        return core_permission_for_rest_method('user', 'DELETE', 'user_permission')
 
     def method_under_test_malformed(self) -> Optional[Union[HttpResponse, TestHttpResponse]]:
         return None

skipper/skipper/flow/models/permissions.py

@@ -81,7 +81,7 @@ def get_permissions_class(entity: str, action: str) -> Type[permissions.DjangoMo
     Example:
     
         * changing user: entity=user, action=user
-        * changing user's permission field: entity=user, action=user-permission
+        * changing user's permission field: entity=user, action=some-action
         
     """
     class Permission(permissions.DjangoModelPermissions):